Skip to content

fix: ignore callbacks from a signal socket that was already replaced - #1211

Draft
hiroshihorie wants to merge 1 commit into
hiroshi/complete-disconnect-on-socket-closefrom
hiroshi/signal-socket-generation
Draft

hiroshihorie wants to merge 1 commit into
hiroshi/complete-disconnect-on-socket-closefrom
hiroshi/signal-socket-generation

Conversation

@hiroshihorie

Copy link
Copy Markdown
Member

Stacked on #1208.

Problem

A real socket reports its close a few milliseconds after the SDK disposes it (websocket/io.dart onDone, websocket/web.dart onClose). SignalClient routed that report through the same handler as a live server close. If it landed while the next connection was being established, it set the signal state back to disconnected and the engine, no longer closed, started a reconnect against a healthy socket. The window is small, a disconnect() followed immediately by connect(), or any full reconnect, but it is real. #1208 makes disconnect() return sooner, so this is the right time to close it.

Fix

  • Each socket's onData, onDispose and onError callbacks carry the generation of the socket they were created for. cleanUp() and connect() bump the generation. A callback from an older generation is dropped with a log line.
  • The signalReconnect and fullReconnect debug scenarios used to call signalClient.cleanUp() and rely on the late close to make the engine recover. That path is now dropped by design, so they call a new internal simulateSignalDrop() which disposes the socket and raises the disconnect explicitly. Same observable behaviour.
  • The mock socket now reports its close after dispose() with a configurable latency, like the real implementations, so the race is reproducible.

Tests

test/core/stale_socket_close_test.dart:

  • disconnect, connect again before the old socket reports its close, the stale close arrives during the new session: the room stays connected and no reconnect or disconnect event is emitted. Fails without the guard.
  • the signalReconnect debug scenario still re-opens the signal connection as a resume.

Full suite 430 passing.

🤖 Generated with Claude Code

A real socket reports its close a few milliseconds after dispose()
returns. That report went through the same handler as a live close, so
when it landed during the next connect it reset the signal state to
disconnected and started a reconnect against a socket that was fine.
Each socket's callbacks now carry a generation, bumped on cleanUp() and
on every connect, and a callback from an older generation is dropped.

The signalReconnect and fullReconnect debug scenarios relied on that
late close to trigger the engine's recovery. They now call
simulateSignalDrop(), which disposes the socket and raises the
disconnect explicitly.

The mock socket reports its close after dispose() like the real ones, so
the race is reproducible in tests.
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.

1 participant