Repository navigation
fix(rtc): avoid Room.sid hang on disconnect and deduplicate active speakers - #875
NabiBukhsh-AI wants to merge 2 commits into
Conversation
…peakers - Fix Room.sid indefinitely hanging when room disconnects before receiving SID or when called on a disconnected room (fixes livekit#651) - Add Room.get_sid(timeout=...) to allow bounding the wait for room SID - Fail _first_sid_future cleanly with RuntimeError/ConnectError when disconnect or connect error occurs - Safely retrieve participant identity in _retrieve_participant without requiring local_participant before connecting - Safely read sid in Room.__repr__ without raising if _first_sid_future has an exception - Deduplicate active_speakers_changed events to avoid emitting duplicate or blank events when active speakers have not changed (fixes livekit#148) - Expose Room.active_speakers property to inspect currently active speakers - Clear active_speakers and emit empty list when room disconnects or speaking participant disconnects - Isolate FfiClient._dispose in subprocess in test_ffi_handle_lifecycle to avoid invalidating the global FFI client during test runs
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| except Exception as e: | ||
| self._fail_first_sid(e) | ||
| raise | ||
| finally: | ||
| self._is_connecting = False |
There was a problem hiding this comment.
🔴 Cancelled connect strands SID waiters
When connect() is cancelled before its response, existing SID waiters remain blocked because asyncio.CancelledError bypasses the failure handler. The room's FFI subscription also stays open.
Learn more
connect() subscribes the room to FFI events before awaiting its response, and sid allows callers to wait while _is_connecting is true. Cancelling connect() during that await runs the inner queue cleanup, but asyncio.CancelledError does not enter except Exception. The SID future stays pending for callers already awaiting it. The outer room subscription also remains registered; isconnected is false without a room handle, so disconnect() cannot clean it up.
Example: Start connect() against an unresponsive server and start awaiting room.sid. Cancel the connect task with asyncio.wait_for; the SID task never finishes, and the room remains subscribed to FFI events.
Recommended fix: Handle cancellation separately in connect(), fail the pending SID future, and unsubscribe the room FFI queue when a connection does not complete. Preserve cancellation propagation and ensure cleanup applies to other early failures.
Was this helpful? React with 👍 or 👎 to provide feedback.
- Catch BaseException in connect to properly fail pending SID future and unsubscribe FFI queue on cancellation - Emit defensive list(speakers) copy to prevent listener mutation from corrupting cached speakers - Add regression tests for connect cancellation and speaker list immutability
This branch has not been deployed
1 participant
Description
This PR resolves two issues in
livekit-rtc:Room.sidhanging indefinitely when the room disconnects before receiving a SID, when connect fails, or when accessed on a disconnected room.active_speakers_changedevents being emitted when there are no changes to the active speakers list.Root Cause Analysis & Key Changes
1. Issue #651 (
Room.sidindefinite hang)Room.sidawaitedself._first_sid_future. If a room disconnected before receiving a SID (e.g. SIP ringback hangup before server sends SID), or ifconnect()threw aConnectError, or ifsidwas queried on a disconnected room,_first_sid_futureremained pending indefinitely, causing caller tasks to suspend forever.sidproperty: ifconnection_state == ConnectionState.CONN_DISCONNECTEDand not currently connecting, immediately raisesRuntimeError("cannot get sid of a disconnected room")._fail_first_sid(exc): if_first_sid_futureis pending, sets the exception (and marks it retrieved to prevent unretrieved future warnings if unawaited).disconnect(),disconnectedFFI event, or_listen_tasktermination (EOS/error): marks_first_sid_futurefailed withRuntimeError("room disconnected before receiving sid").connect()failure: marks_first_sid_futurefailed with the connect exception.async def get_sid(self, timeout: float | None = None) -> strfor callers to easily bound the wait time for a SID.Room.__repr__against unhandled exceptions on_first_sid_future.2. Issue #148 (Blank and redundant
active_speakers_changedevents)_on_room_eventemittedactive_speakers_changedunconditionally, even when the active speaker list remained identical or empty.self._active_speakers: list[Participant] = []and exposed propertyRoom.active_speakers.active_speakers_changedwhen the identities list differs fromself._active_speakers.self._active_speakersand emit an updated list.3. Test Runner FFI Isolation
livekit-rtc/tests/test_ffi_handle_lifecycle.py,test_live_handles_marked_consumed_and_no_assert_after_disposeis isolated in a subprocess to preventFfiClient._dispose()from tearing down the C/Rust FFI runtime in the main pytest process.Verification
uv run pytest tests/ livekit-rtc/tests/: 203 passed, 74 skipped, 0 failures.uv run mypy livekit-protocol livekit-api livekit-rtc: Success (no issues found in 105 files).uv run ruff check&uv run ruff format --check: Passed cleanly.