Skip to content

Commit 09ee0a2

Browse files
Willyfrogclaude
andauthored
[MM-67999] fix websocket crash (#163)
* fix(ios): synchronize WebSocket dictionary access and prevent delegate use-after-free Add a serial dispatch queue to protect the `webSockets` dictionary in WebSocketManager from concurrent access across URLSession delegate callbacks, main queue, and the RN bridge. All reads/writes are now wrapped in `queue.sync {}` with atomic remove-and-return in `invalidateClient` and atomic snapshot+removeAll in `invalidateContext` to eliminate TOCTOU races. In WebSocketWrapper.didReceive, capture the weak `self.delegate` into a strong local at method entry so the delegate cannot deallocate mid-callback, preventing EXC_BAD_ACCESS. Addresses Sentry issues MATTERMOST-MOBILE-IOS-XHW and MATTERMOST-MOBILE-IOS-YQG. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(ios): address PR review feedback on WebSocket synchronization - Restore synchronized early-return in createWebSocket to skip expensive option parsing and keychain work when a socket already exists - Move dictionary removal in invalidateClient back to after disconnect and cleanup, so the entry is only removed on success - Change didReceive delegate capture from required guard to optional strong local so internal side effects (errorCounter cleanup, client.disconnect on cancel) still execute when delegate is nil Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent e25cbaf commit 09ee0a2

2 files changed

Lines changed: 39 additions & 28 deletions

File tree

ios/WebSocket/WebSocketManager.swift

Lines changed: 38 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,23 @@ import SwiftyJSON
55
class WebSocketManager: NSObject {
66
static let `default` = WebSocketManager()
77
private override init() {}
8-
internal var webSockets: [URL: WebSocket] = [:]
9-
8+
private var webSockets: [URL: WebSocket] = [:]
9+
private let queue = DispatchQueue(label: "com.mattermost.react-native-network-client.websocket-manager")
10+
1011
func webSocketCount() -> Int {
11-
return webSockets.count
12+
return queue.sync { webSockets.count }
1213
}
13-
14+
1415
func createWebSocket(for url:URL, withOptions options: Dictionary<String, Any>, withDelegate delegate: WebSocketWrapper) throws -> Void {
15-
let existingWebSocket = getWebSocket(for: url)
16-
if (existingWebSocket != nil) {
17-
existingWebSocket!.delegate = delegate
18-
return
16+
let reused: Bool = queue.sync {
17+
if let existing = webSockets[url] {
18+
existing.delegate = delegate
19+
return true
20+
}
21+
return false
1922
}
20-
23+
if reused { return }
24+
2125
var request = URLRequest(url: url)
2226
var compressionHandler: CompressionHandler? = nil
2327
var clientCredential: URLCredential? = nil
@@ -30,15 +34,15 @@ class WebSocketManager: NSObject {
3034
request.addValue(value.stringValue, forHTTPHeaderField: key)
3135
}
3236
}
33-
37+
3438
if let timeoutInterval = options["timeoutInterval"].double {
3539
request.timeoutInterval = timeoutInterval / 1000
3640
}
37-
41+
3842
if let clientP12Configuration = options["clientP12Configuration"].dictionaryObject as? [String:String] {
3943
let path = clientP12Configuration["path"]
4044
let password = clientP12Configuration["password"]
41-
45+
4246
do {
4347
try Keychain.importClientP12(withPath: path!, withPassword: password, forHost: url.host!)
4448
} catch KeychainError.DuplicateIdentity {
@@ -50,33 +54,34 @@ class WebSocketManager: NSObject {
5054
let (identity, certificate) = try Keychain.getClientIdentityAndCertificate(for: url.host!)!
5155
clientCredential = URLCredential(identity: identity, certificates: [certificate], persistence: URLCredential.Persistence.permanent)
5256
}
53-
57+
5458
if options["trustSelfSignedServerCertificate"].boolValue {
5559
allowSelfSign = true
5660
}
5761
}
5862

5963
let webSocket = WebSocket(request: request, engine: WebSocketEngine(forHost: url.host, clientCredential: clientCredential, allowSelfSignCertificate: allowSelfSign))
6064
webSocket.delegate = delegate
61-
62-
webSockets[url] = webSocket
65+
66+
queue.sync {
67+
if let existing = webSockets[url] {
68+
existing.delegate = delegate
69+
webSocket.delegate = nil
70+
} else {
71+
webSockets[url] = webSocket
72+
}
73+
}
6374
}
64-
75+
6576
func getWebSocket(for url:URL) -> WebSocket? {
66-
return webSockets[url]
67-
}
68-
69-
func disconnectAll() -> Void {
70-
for ws in webSockets {
71-
ws.value.disconnect()
72-
}
77+
return queue.sync { webSockets[url] }
7378
}
7479

7580
func invalidateClient(for url:URL) -> Void {
7681
guard let webSocket = getWebSocket(for: url) else {
7782
return
7883
}
79-
84+
8085
if let _ = try? Keychain.getClientIdentityAndCertificate(for: url.host!) {
8186
do {
8287
try Keychain.deleteClientP12(for: url.host!)
@@ -88,11 +93,17 @@ class WebSocketManager: NSObject {
8893
}
8994
webSocket.forceDisconnect()
9095
webSocket.delegate = nil
91-
webSockets.removeValue(forKey: url)
96+
queue.sync { webSockets.removeValue(forKey: url) }
9297
}
9398

9499
func invalidateContext() -> Void {
95-
disconnectAll()
96-
webSockets.removeAll()
100+
let snapshot: [WebSocket] = queue.sync {
101+
let values = Array(webSockets.values)
102+
webSockets.removeAll()
103+
return values
104+
}
105+
for ws in snapshot {
106+
ws.disconnect()
107+
}
97108
}
98109
}

ios/WebSocket/WebSocketWrapper.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,8 @@ var READY_STATE = [
171171
// MARK: WebSocketDelegate methods
172172

173173
public func didReceive(event: WebSocketEvent, client: Starscream.WebSocketClient) {
174-
175174
guard let url = client.url() else { return }
175+
let delegate = self.delegate
176176

177177
switch event {
178178
case .connected(let headers):

0 commit comments

Comments
 (0)