Skip to content

Serialize the room lifecycle so a stale reconnect can't tear down the next session - #1147

Draft
pblazej wants to merge 24 commits into
mainfrom
blaze/reconnect-handoff
Draft

Serialize the room lifecycle so a stale reconnect can't tear down the next session#1147
pblazej wants to merge 24 commits into
mainfrom
blaze/reconnect-handoff

Conversation

@pblazej

@pblazej pblazej commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Problem

An app can move a Room to a new session while an automatic reconnect of the old session is still running, typically when it returns to the foreground after suspension. When that stale reconnect fails, its failure path cleans up the new session. connect() returns successfully, and then the room drops to .disconnected.

Fix

  • connect(), disconnect() and the server's disconnect are serialized on the existing SerialRunnerActor, each running to completion.
  • The reconnect is one registered task. The next lifecycle operation cancels it and waits for it to finish before cleaning up, and no reconnect starts during that handoff.
  • disconnect() still interrupts an in-flight connect() instead of queuing behind it. That includes a Cloud region-settings fetch and a microphone start waiting on its permission prompt.
  • A server leave is tied to the session its socket was opened for, so an old session's leave can't end the next one.
  • connect() only stops tracks it created itself.

Behaviour changes

  • A disconnect() that interrupts a connect() after it connected reports .cancelled to room(_:didDisconnectWithError:) instead of nil.
  • A failed or interrupted connect() no longer stops the pre-connect audio track. The pre-connect buffer owns that track (withPreConnectAudio stops it on failure), and a buffer started manually must be stopped by its owner.

Known limitation

A server leave that arrives while connect() is still joining still cleans up alongside the connect's own clean-up, as on main. It is out of scope here.

Testing

  • New tests are barrier-based, with no sleeps. They live in ReconnectThenConnectTests, LifecycleHandoffTests, OfferWithJoinGateTests and RegionManagerTests.
  • The stale-reconnect test (staleReconnectDoesNotTearDownNextSession) and the held-disconnect test (connectFromDidDisconnectSurvivesAHeldDisconnect) fail on main.
  • The full LiveKitCoreTests suite passes locally against livekit-server --dev: 471 tests in 98 suites.
  • No public API or Obj-C surface change: LiveKit-Swift.h is identical to main.
  • Swift 6.1 / Xcode 16.4 is left to CI.

pblazej and others added 24 commits October 7, 2026 10:49
@pblazej @claude omnigent
Test that a stale reconnect or teardown cannot destroy the next session
3bc7e87 
Two failing tests against main:

- ReconnectThenConnectTests.staleReconnectDoesNotTearDownNextSession: a
  reconnect held in its socket handshake (a local listener that never
  answers) is still unwinding when the app connects the Room to a new
  session, with or without disconnect() first. Once released, the stale
  reconnect cleans up over the new session: the room ends .disconnected.
- LifecycleHandoffTests.connectFromDidDisconnectSurvivesAHeldDisconnect:
  disconnect() is held inside cleanUp() while the server's leave arrives;
  the app connects from didDisconnectWithError, and the old disconnect()
  then tears that session down.

Both hold their ordering on barriers, not sleeps.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Serialize the room lifecycle so a stale reconnect cannot tear down a …
c560937 
…new session

When the app moves a Room to a new session while an automatic reconnect of
the old one is still running (socket died while suspended; on foreground the
app calls connect(), with or without disconnect() first), the reconnect's
failure path ran cleanUp() over the new session: connect() returned, then the
room dropped to .disconnected(cancelled).

- The `if !Task.isCancelled` guard read the outer, never-cancelled task.
  The whole reconnect (retry loop, success write, failure clean-up) is now
  one stored task that forwards cancellation into Task.retrying and checks
  its own cancellation before touching state.
- Reconnect admission and registration happen in one StateSync mutation, so
  a connect()/disconnect() either finds it to cancel or refuses it.
- connect(), disconnect() and the server's disconnect run one at a time on a
  SerialRunnerActor (as Track and LocalParticipant do). Each cancels the
  in-flight reconnect explicitly, waits for it to finish, then cleans up
  once; no reconnect starts while they hand off.
- SignalClient drops a leave whose socket is no longer current at delivery,
  so the old session's leave cannot fail the next connect().
- fullConnectSequence is @nonobjc: as an implicitly @objc async method its
  calls dropped the caller's cancellation, so a cancelled reconnect waited
  out its socket connect.
- AnyTaskCancellable gains wait().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Test the handoff: registration, refusal, cancellation, queuing, deleg…
bc1db1a 
…ate reconnect

- reconnectIsRegisteredWhenAdmitted: no mutation admits a reconnect without
  registering it.
- reconnectIsRefusedDuringHandoff: no reconnect starts while connect() waits
  for a stale one.
- cancelledConnectEndsItsHandoff / connectQueuedBehindAHandoffRunsAfterIt.
- connectFromDidDisconnectSurvivesTheDrain: a connect() started from
  didDisconnectWithError while disconnect() waits for a stale reconnect
  keeps its session.

Each handoff test asserts the handoff marker is cleared. Ordering is held on
barriers (a test-held reconnect, delegate-queue flush), not timeouts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Test that disconnect() interrupts a held connect and a late leave is …
563d92c 
…dropped

Two failing tests for the serialized lifecycle:

- ReconnectThenConnectTests.disconnectInterruptsAHeldConnect: connect() is
  held in its socket handshake; disconnect(), without cancelling the connect
  task, must return promptly and the connect must throw .cancelled. Today
  disconnect() queues behind the connect, which throws .network once the
  handshake times out.
- OfferWithJoinGateTests.leaveEnqueuedAfterItsSocketWasReplacedIsDropped: a
  leave that arrived on a socket since replaced is enqueued late (as the
  detached enqueue can be); it must not be delivered. Today the leave
  guard captures the socket current at processing time, so it is.

The detached enqueue moves into enqueue(_:encoded:from:) so the test can
run it late; no behaviour change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Let disconnect() interrupt an in-flight connect instead of queuing be…
bf2849c 
…hind it

Serializing the lifecycle queued disconnect() behind a connect() in
progress, so a stalled join held disconnect() until it timed out; on main
disconnect() aborted it at once.

- connect() registers its runner task (SerialRunnerActor.enqueue, split
  out of run()) while it is queued or running; disconnect() cancels the
  ones registered before it, then queues its teardown as before.
- SignalClient.connect() skips server validation once cancelled, so the
  interrupted connect throws .cancelled instead of a validation error.
- A connect() failing after it joined (e.g. interrupted while publishing
  the microphone) sends a leave before cleaning up, as disconnect() would.
- connectQueuedBehindAHandoffRunsAfterIt waits until the second connect is
  queued on the runner before releasing the first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Drop a leave by the socket it arrived on, not the one current at proc…
eedddc1 
…essing

The leave guard captured the socket current when the response was
processed. The detached enqueue can run after cleanUp() and a new connect
replaced the socket; the old session's leave then captured the new socket,
passed the guard and could end the new session.

The response queue now carries the socket each message arrived on, and the
leave is delivered only while that socket is still current.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Add a changeset for the reconnect handoff fix
2506c1f 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Test connect registration, a cancelled region waiter, the post-JOIN l…
6dbf83f 
…eave, and a drained leave oracle

- connectIsRegisteredBeforeItRuns: _connect must already be in
  _connectTasks when it starts, or a disconnect() in between misses it.
- RegionManagerTests.cancelledWaiterStopsWaitingForTheSharedFetch: a
  cancelled resolveBest() must stop waiting for a held settings fetch,
  which then still serves the next waiter.
- disconnectDuringTheTransportWaitSendsTheLeave: interrupted after the
  JOIN while the transport cannot connect (relay-only, no TURN), the
  observer must see the participant leave with CLIENT_INITIATED.
- leaveEnqueuedAfterItsSocketWasReplacedIsDropped flushes the delegate
  runner after each enqueue instead of racing a control leave.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Register a connect atomically with its creation; leave once the JOIN …
ad2b1d3 
…arrived

- connect() creates its task inside the _connectTasks mutation, so a
  disconnect() either finds it or was requested before it existed.
  Previously the runner could start _connect before the handle was
  inserted, and a disconnect() in that window queued behind a stalled join.
- An interrupted or failed connect sends a leave when the signal
  connection has its JOIN, not only once the room reads .connected:
  between JOIN and transport-connected the room is still .connecting,
  where main's disconnect() did send the leave.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Let a cancelled waiter stop waiting for the shared region-settings fetch
50fe020 
A connect interrupted by disconnect() while resolving a Cloud region kept
awaiting the shared fetch's value, which ignores the waiter's cancellation,
so disconnect() stayed queued for up to URLSession's 60 s timeout. The
waiter now waits on an AsyncCompleter fed by the fetch: it throws
.cancelled at once, and the fetch carries on for other waiters.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Keep a leave to the session it arrived for, delivered in processing o…
db81ae1 
…rder

- The leave handler captures the room's session before the region update
  can suspend and drops the leave if the session changed meanwhile; the
  queued server-disconnect clean-up checks that same session. Before, an
  old leave resuming after a replacement cleaned up the new connecting
  session, or captured it for the queued clean-up.
- SignalClient queues the leave's delegate delivery in processing order
  (AsyncSerialDelegate.notifyQueued, on SerialRunnerActor.enqueue) instead
  of racing it in a detached task; the response queue awaits only the
  enqueue, never delegate code. This also makes the stale-leave test's
  delegate flush a real barrier.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Test a leave crossing into the room after its socket was replaced, an…
ba1a122 
…d disconnect during microphone setup

- LifecycleHandoffTests.leaveFromAReplacedSocketDoesNotFailTheNextConnect:
  the old session's leave reaches the room's handler only after a new
  connect replaced the socket (the delegate call can suspend after the
  signal client's check, SE-0338). The room must not clean up the new
  connect, which today fails it.
- LifecycleHandoffTests.disconnectDoesNotWaitForTheMicrophone: with
  enableMicrophone and the microphone track held (an unanswered permission
  prompt), disconnect() must return while it is still held. Today it waits
  for it. Once connected, the interruption is reported to
  didDisconnectWithError as .cancelled.

Seams, no behaviour change: the leave delegate call carries the socket it
arrived on (the room ignores it for now); the microphone track comes from
an overridable makeMicrophoneTrack(). HeldSocketServer moves to its own
test file. disconnectDuringTheTransportWaitSendsTheLeave now chains the
room's onDidMutate instead of replacing it (which silenced its delegates).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Check a leave's socket where the room acts on it, not only before the…
fa4ac7a 
… call

SignalClient checked the leave's socket and then called into the room; that
call can suspend (SE-0338), so a socket replaced in between let the old leave
capture the new session. The delegate call now carries the socket the leave
arrived on, and the room checks it and captures the session in one
synchronous step on entry.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Don't let disconnect() wait for the microphone track of the connect i…
fd43e90 
…t interrupts

With enableMicrophone the microphone track can wait on a permission prompt,
which ignores cancellation. The interrupted connect awaited it, either for
publishing or to stop it in its catch, and disconnect() waited behind it.

- The connect waits for the track via Task.valueUnlessCancelled(), which
  throws .cancelled once the connect is cancelled; the track task keeps
  running.
- On failure the connect cancels the track task and stops its track when it
  finishes, without waiting for it.
- AsyncCompleter accepts an infinite timeout (no timer scheduled), which the
  helper uses: a permission prompt may legitimately take minutes.
- The region-settings waiter uses the same helper instead of a 120 s bound.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Release-note the .cancelled didDisconnectWithError of an interrupted …
2fbba09 
…connect

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Test leave/session association, a permission granted after disconnect…
e1c5539 
…, and the borrowed pre-connect track

- leaveForAReplacedSessionIsIgnoredOnTheSameSocket: the session is
  replaced while the leave's socket still reads current, as when a
  replacement lands between the room's separate socket and session reads.
  The old session's leave must not end the new one; today it does.
- disconnectDoesNotWaitForTheMicrophone now also grants the held
  permission after the disconnect: the abandoned microphone task must stop
  before starting recording. Today it goes on to start it.
- interruptedConnectDoesNotStopThePreConnectTrack: an interrupted connect
  must not stop the pre-connect buffer's track it only borrowed. Today its
  deferred clean-up stops it, which can land on the next session's publish.

Seams, no behaviour change: SignalClient records the room session with
the socket it installs and passes the leave's session to the delegate
(the room ignores it for now); LocalAudioTrack's permission request is an
overridable requestMicrophonePermission().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Tie a leave to the session recorded with its socket, decided in one read
cb179c3 
The room checked the leave's socket and read its session under separate
locks, so a session replaced between the two reads let the old leave act
on the new one. SignalClient now records the room session in the same
mutation that installs the socket, and reads both together when the leave
is delivered; the room only compares that session with its current one,
in single reads, before acting and in the queued clean-up. The leave's
delegate call no longer carries the socket.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Stop an abandoned microphone start before it starts recording
e7a5c66 
The permission prompt ignores cancellation, so a microphone task abandoned
by an interrupted connect() went on, once permission was granted, to
configure and start shared audio recording, possibly under the next
session. LocalAudioTrack.startCapture() now checks cancellation after the
permission request and before any recording side effect; the connect's
catch already cancels the task, and Track.start() forwards that
cancellation into startCapture().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Never stop the pre-connect track an interrupted connect only borrowed
4f74a43 
The connect took the pre-connect buffer's track through the same task as a
microphone track it makes itself, so its deferred clean-up stopped the
borrowed track too, possibly after the next session had published it
(resetting that publish's frame watcher). The pre-connect track is now
published directly and never stopped by connect(); only a microphone
track the connect made itself is stopped on failure.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Test a microphone start cancelled while queued on the RTC executor
00e798e 
With the executor held, the start is granted permission, queues its
recording step, and is cancelled before that step runs; it must end
cancelled without starting recording.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Check a microphone start's cancellation on the RTC executor, right be…
7cf0aaf 
…fore recording

The check ran before the hop to @rtc; a cancel landing while the start
waited for that executor still configured and started shared recording.
It now runs inside the RTC closure, immediately before
startLocalRecording, which covers every cancel up to that point, so the
pre-hop check is dropped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Keep the new internal helpers out of the Objective-C runtime
a519d5e 
LocalAudioTrack and Room are @objcMembers, so requestMicrophonePermission()
and cleanUpForServerDisconnect(withError:session:) were exported as
Objective-C selectors. Both are @nonobjc now; the generated header and the
built object's Objective-C method names match main.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Release-note that connect() no longer stops the borrowed pre-connect …
b6c42da 
…track

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
@pblazej @claude omnigent
Describe what the RTC-hop test's barrier actually proves
c2fceef 
Comment only: `requested` proves the permission request was entered, not
that the recording step is already queued on the executor.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: omnigent <noreply@omnigent.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hiroshihorie hiroshihorie Awaiting requested review from hiroshihorie hiroshihorie will be requested when the pull request is marked ready for review hiroshihorie is a code owner
@xianshijing-lk xianshijing-lk Awaiting requested review from xianshijing-lk xianshijing-lk will be requested when the pull request is marked ready for review xianshijing-lk is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@pblazej