fix: complete disconnect() when the server closes the socket without echoing the leave - #1208
Draft
hiroshihorie wants to merge 1 commit into
Draft
hiroshihorie wants to merge 1 commit into
hiroshihorie wants to merge 1 commit into
Conversation
…echoing the leave engine.disconnect() sends the Leave and relies on the server's echo to run cleanup and emit EngineDisconnectedEvent. Media nodes drop queued leave messages when they close the signal sink, so the echo can be lost. The socket close was then ignored because _isClosed was set, and Room.disconnect() waited out its 10 s timeout for an event that never came, leaving the room half torn down. The engine now remembers a disconnect that is waiting for its echo and, on a socket close in that window, runs cleanup and emits the event itself. A close that follows the echo is still ignored, so there is no second event.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
engine.disconnect()sends the Leave and relies on the server's echo to run cleanup and emitEngineDisconnectedEvent. Media nodes drop queued leave messages when they close the signal sink (pkg/routing/signal.go,signalMessageSink.Close, comment in place), so the echo can be lost. The socket close was then ignored because_isClosedwas already set, andRoom.disconnect()waited out its 10 s timeout for an event that never came, then threw without running its own cleanup.Found by the e2e-flutter suite (livekit/e2e-flutter#7), where one agent per run hit it against a local livekit-server 1.13.6. Reproduced every run with
LOG=fine:SignalClient did disconnectwith noLeave receivedbefore it, while the server loggedsendLeave: true.Fix
The engine remembers a disconnect that has sent its Leave and is waiting for the echo. A socket close in that window runs cleanup and emits
EngineDisconnectedEventwith the original reason. A close that arrives after the echo finds nothing pending and is ignored as before, so there is no second event.One field and one branch in
engine.dart. Independent of #1204.Tests
Two in
test/core/disconnect_event_test.dart: socket close without echo completesdisconnect()in well under the timeout with oneclientInitiatedevent, and echo followed by close still emits exactly one. The first fails on main.🤖 Generated with Claude Code