Repository navigation
fix(rtc): avoid Room.sid hang when disconnected or cancelled (#651) #875
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
86afe63
2bf92fb
2762cbe
f1e0e00
d567e4a
500b900
ced3c84
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Duplicate departures produce false error logs When a departed participant appears in another disconnect event, (Refers to this code) Learn moreThe room tracks remote participants by identity and removes an entry on a disconnect event. A duplicate or late disconnect event has no corresponding entry. The bare pop raises Example: Alice disconnects and the first event removes Alice from the map. A second disconnect event for Alice raises Recommended fix: Use Was this helpful? React with 👍 or 👎 to provide feedback. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| # Copyright 2026 LiveKit, Inc. | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| import asyncio | ||
|
|
||
| import pytest | ||
|
|
||
| from livekit import rtc | ||
| from livekit.rtc import ConnectionState | ||
| from livekit.rtc._proto import ffi_pb2 as proto_ffi | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_room_sid_disconnected_raises_runtime_error() -> None: | ||
| room = rtc.Room() | ||
| with pytest.raises(RuntimeError, match="cannot get sid of a disconnected room"): | ||
| await room.sid | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_room_sid_resolution() -> None: | ||
| room = rtc.Room() | ||
| room._connection_state = ConnectionState.CONN_CONNECTED | ||
|
|
||
| async def resolve_soon() -> None: | ||
| await asyncio.sleep(0.01) | ||
| room._resolve_first_sid("RM_test123") | ||
|
|
||
| task = asyncio.create_task(resolve_soon()) | ||
| sid = await room.sid | ||
| await task | ||
| assert sid == "RM_test123" | ||
| assert "RM_test123" in repr(room) | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_room_sid_failed_on_disconnect() -> None: | ||
| room = rtc.Room() | ||
| room._connection_state = ConnectionState.CONN_CONNECTED | ||
|
|
||
| async def wait_sid() -> None: | ||
| await room.sid | ||
|
|
||
| wait_task = asyncio.create_task(wait_sid()) | ||
| await asyncio.sleep(0.01) | ||
|
|
||
| # Room disconnects before receiving SID | ||
| room._fail_first_sid(RuntimeError("room disconnected before receiving sid")) | ||
|
|
||
| with pytest.raises(RuntimeError, match="room disconnected before receiving sid"): | ||
| await wait_task | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_connect_cancellation_fails_sid_future_and_cleans_up( | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| ) -> None: | ||
| room = rtc.Room() | ||
|
|
||
| # Mock FfiClient request to never answer connect response | ||
| def mock_request(req: proto_ffi.FfiRequest) -> proto_ffi.FfiResponse: | ||
| resp = proto_ffi.FfiResponse() | ||
| resp.connect.async_id = 9999 | ||
| return resp | ||
|
|
||
| monkeypatch.setattr(rtc._ffi_client.FfiClient.instance, "request", mock_request) | ||
|
|
||
| connect_task = asyncio.create_task(room.connect("ws://dummy", "dummy_token")) | ||
| await asyncio.sleep(0.01) | ||
|
|
||
| assert room._is_connecting | ||
|
|
||
| async def wait_sid() -> None: | ||
| await room.sid | ||
|
|
||
| sid_task = asyncio.create_task(wait_sid()) | ||
| await asyncio.sleep(0.01) | ||
|
|
||
| connect_task.cancel() | ||
| with pytest.raises(asyncio.CancelledError): | ||
| await connect_task | ||
|
|
||
| assert not room._is_connecting | ||
| with pytest.raises(RuntimeError, match="room connection was cancelled"): | ||
| await sid_task |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Disconnected speakers remain in listener state
When a speaking participant or room disconnects,
active_speakers_changedno longer reports the reduced or empty list. Listeners retain departed speakers until another speaker update arrives.(Refers to this code)
Learn more
Speaker listeners receive a complete speaker list, not incremental removal events. On a participant disconnect, the room removes the participant and emits
participant_disconnected, but no longer tells speaker listeners that their list changed. The same omission occurs on both room disconnect paths in disconnect and the disconnected event.Example: The last speaker is Alice. Alice disconnects, and a UI that updates speakers only from
active_speakers_changedcontinues to show Alice until another speaker update arrives.Recommended fix: Maintain current speakers and emit a reduced list when a speaking participant disconnects; emit an empty list on room disconnect when any speakers remain.
Was this helpful? React with 👍 or 👎 to provide feedback.