Skip to content

Stay closed when disconnect() lands during a full reconnect - #1264

Open
hiroshihorie wants to merge 2 commits into
mainfrom
hiroshi/disconnect-during-full-reconnect
Open

hiroshihorie wants to merge 2 commits into
mainfrom
hiroshi/disconnect-during-full-reconnect

Conversation

@hiroshihorie

@hiroshihorie hiroshihorie commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

Fixes #1245.

Problem

restartConnection() checks _isClosed only on entry. It then awaits the old peer connections' dispose, the signal cleanup and the listener cancel before calling connect(), and connect() sets _isClosed = false unconditionally.

A disconnect() that runs in that window is undone. The engine joins again with the old token and identity, and if that join fails it keeps retrying up to the attempt limit. A join that lands after the app has reconnected the same identity from a new Room makes the server remove the new, live session with DUPLICATE_IDENTITY.

Fix

  • connect() still reopens the engine for a connect the caller asks for. The body moves into a private _connect() that leaves _isClosed alone, and restartConnection() uses that. A disconnect that lands while the reconnect is joining therefore stays in effect, and the retry loop stops at its existing _isClosed checks.
  • restartConnection() checks _isClosed again once its teardown is done, and returns before opening a new signal connection.
  • connect() cancels and recreates the signal listeners before joining. disconnect() cancels them when it stops a pending reconnect, and only restartConnection() recreated them, so a room that disconnected during a reconnect delay could not connect again. Its next connect() opened a socket, but nothing handled the join response and it timed out. The early return above made the same state reachable from a full reconnect.

Verification

  • New unit tests in test/core/disconnect_during_reconnect_test.dart, over the mock transport:
    • disconnect() from RoomReconnectingEvent during a full reconnect opens no new signal socket, and the same room then connects again. The first part fails on main.
    • disconnect() from RoomAttemptReconnectEvent, while the retry is still pending, and the same room then connects again. This fails on main, where the connect times out waiting for the join response.
  • New e2e scenario in livekit/e2e-flutter, "Disconnect during a full reconnect stays disconnected", against a local livekit-server. It triggers a full reconnect with sendSimulateScenario(fullReconnect: true), disconnects from RoomReconnectingEvent, and watches the server's participant list for 15 s. Only a session with a new participant SID counts as a rejoin.
    • main: fails 3 of 3 runs. The target rejoins with a new participant SID within a second.
    • this PR: passes 3 of 3 runs.
  • Existing e2e scenarios on macOS: connection basics and ping pong pass on the PR head, and migration and node failure passed on the first commit, before the listener change. FullReconnectTest and the duplicate identity scenario sometimes time out waiting for video stats on a machine with a camera, on main as well, which their headers already document.
  • flutter analyze, flutter test, format and import order are clean.

restartConnection() checked _isClosed only on entry, then awaited the old
peer connections' dispose and the signal cleanup before calling connect(),
which cleared _isClosed. A disconnect() in that window was undone: the
engine joined again with the old token and kept retrying up to the attempt
limit, and a join that landed after the app reconnected the same identity
removed the new session with DUPLICATE_IDENTITY.

Check _isClosed again once the teardown is done, and connect through a
helper that leaves the flag alone, so only a caller's connect() reopens the
engine.

Fixes #1245
devin-ai-integration[bot]

This comment was marked as resolved.

disconnect() cancels the signal listeners when it stops a pending
reconnect, and only restartConnection() recreated them. A room that
disconnected during a reconnect delay, or now during a full reconnect's
teardown, could not connect again: the next connect() opened a socket but
nothing turned the join response into EngineJoinResponseEvent, so it timed
out. connect() now cancels and recreates them before joining.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

A pending full restart can outlive disconnect and interfere with an immediate reconnect, canceling the new signal listeners and starting another join.

1 open issue
What changed in this PR

This PR addresses #1245 by preventing full reconnect teardown from reopening a disconnected engine, while restoring signal listeners for later explicit connections.

Changes:

  • Separates explicit connection setup from the shared connection logic and adds a closed-state check after reconnect teardown.
  • Adds regression tests for disconnecting during full reconnects and pending retry delays.
  • Adds a patch changeset documenting the fix.
File Description
test/​core/​disconnect_during_reconnect_test.dart Tests disconnect behavior during reconnect and subsequent room reuse.
lib/​src/​core/​engine.dart Separates connection paths, restores listeners, and guards reconnect after teardown.
.changes/​disconnect-during-full-reconnect Documents the reconnect fix for release.

Review effort: Balanced

Comment thread lib/src/core/engine.dart
_setUpSignalListeners();
// disconnect() can run during the teardown above. It closes the engine
// for good, so do not connect again.
if (_isClosed) {

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A full reconnect that is in progress when disconnect() is called reopens the closed engine and retries up to reconnectAttempts

2 participants