Skip to content

Fix crash in RCTWebSocketModule when delegate callbacks fire after module invalidation#55858

Closed
lukeharvey wants to merge 1 commit intofacebook:mainfrom
lukeharvey:fix/websocket-nil-socketid-crash
Closed

Fix crash in RCTWebSocketModule when delegate callbacks fire after module invalidation#55858
lukeharvey wants to merge 1 commit intofacebook:mainfrom
lukeharvey:fix/websocket-nil-socketid-crash

Conversation

@lukeharvey
Copy link
Contributor

Summary:

Guard against nil socketID in all SRWebSocketDelegate callbacks in RCTWebSocketModule.

When the module is invalidated during teardown, invalidate nils the delegate and closes each socket. However, SocketRocket's network thread may have already dispatched a delegate callback to the main queue via setDelegateDispatchQueue:. By the time the block executes, the SRWebSocket has been deallocated and reactTag (an associated object) returns nil, causing either an objc_retain crash on a dangling pointer or an NSInvalidArgumentException when inserting nil into an NSDictionary literal.

The didFailWithError: callback already had a partial nil guard (socketID ?: @(-1)), but didCloseWithCode:, webSocketDidOpen:, and didReceiveMessage: did not. This adds consistent early-return nil checks to all four callbacks.
Previously reported in #28278, #29525, and #31048 — all closed by the stale bot without a fix being merged.

Changelog:

[IOS] [FIXED] - Fix crash in RCTWebSocketModule when delegate callbacks fire after module invalidation

Test Plan:

This is a race condition during module teardown that is difficult to reproduce deterministically — it surfaces in production Crashlytics reports (< 0.1% of sessions). The fix is a nil guard matching the defensive pattern already present in didFailWithError:.

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 2, 2026
@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Mar 2, 2026
Copy link
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

@meta-codesync
Copy link

meta-codesync bot commented Mar 3, 2026

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this in D95056045.

@meta-codesync meta-codesync bot closed this in 587ef05 Mar 3, 2026
@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @lukeharvey in 587ef05

When will my fix make it into a release? | How to file a pick request?

@react-native-bot react-native-bot added the Merged This PR has been merged. label Mar 3, 2026
@meta-codesync
Copy link

meta-codesync bot commented Mar 3, 2026

@cipolleschi merged this pull request in 587ef05.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants