-
Notifications
You must be signed in to change notification settings - Fork 141
Description
Describe the bug
Room.disconnect sometimes fails. In one degenerate case, disconnect failed for another user and myself, and we could still hear each other!
SDK Version
2.6.1
iOS/macOS Version
iOS 18
Xcode Version
Xcode 16.4, Swift 5
Steps to Reproduce
Call Room.disconnect
when leaving a room. It may occur more frequently with a poor connection.
Actual behavior
Room can remain connected with an active server session even after disconnect()
returns.
Expected behavior
disconnect()
should always succeed in cleaning up local resources and, when possible, notify the server. The room should never remain connected after disconnect()
completes.
Here's one possible flow that could result in failure to disconnect:
Room.connectionState = .connected
- Network issue (e.g. failed to receive message from websocket) →
SignalClient.cleanUp
- Quick reconnect is started, calling
SignalClient.connect
SignalClient.cleanUp
_state.socket
is set tonil
- Application calls
Room.disconnect()
public func disconnect() async {
if case .disconnected = connectionState { return }
do {
try await signalClient.sendLeave()
} catch {
log("Failed to send leave with error: \(error)")
}
await cleanUp()
}
- The room state is
.connected
(not yet updated), so we attempt to send leave sendLeave() → _sendRequest() → _requestQueue.processIfResumed()
SignalClient.requireWebSocket
throws becausesocket
was set tonil
duringSignalClient.cleanUp()
, and the new socket hasn't been created yet- The error propagates,
sendLeave()
fails, andRoom.cleanUp()
is called - The reconnection completes and the room is resurrected
I'm not completely sure about this. There's definitely the possibility that Room.disconnect
can be called in the middle of SignalClient.cleanUp
, resulting in some weird combinations of states.
For now, we'll work around it by repeatedly calling disconnect
until it succeeds. But I think this has been reproducing for a while - maybe the same as #729, #256, #583, #642