Skip to content

Commit a0bb864

Browse files
committed
fix(rtc): make Room.connect cancellation-safe to avoid FFI timeout panic
Cancelling Room.connect while ConnectRequest is in flight previously stranded the FFI room-event handshake because ReadyForRoomEventRequest was never sent, resulting in an FFI panic 15 seconds later that killed the process with SIGTERM (issue #784). Additionally, the native connection remained active in the background, causing duplicate identity disconnects on reconnect (issue #804). - Await ConnectCallback uninterruptibly on CancelledError - Complete handshake with ReadyForRoomEventRequest and cleanly disconnect the native session - Add unit tests for connect cancellation, connect failure, timeout, and cleanup under setup exceptions
1 parent 462e103 commit a0bb864

3 files changed

Lines changed: 475 additions & 48 deletions

File tree

‎livekit-rtc/livekit/rtc/room.py‎

Lines changed: 103 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
from ._proto.room_pb2 import ConnectionState, SimulateScenarioKind
3232
from ._proto.track_pb2 import TrackKind
3333
from ._proto.rpc_pb2 import RpcMethodInvocationEvent
34-
from ._utils import BroadcastQueue
34+
from ._utils import BroadcastQueue, Queue
3535
from .e2ee import E2EEManager, E2EEOptions
3636
from .log import logger
3737
from .participant import (
@@ -198,6 +198,7 @@ def __init__(
198198

199199
self._token: str | None = None
200200
self._server_url: str | None = None
201+
self._task: Optional[asyncio.Task] = None
201202

202203
def __del__(self) -> None:
203204
if self._ffi_handle is not None:
@@ -563,9 +564,25 @@ def on_participant_connected(participant):
563564
queue = FfiClient.instance.queue.subscribe()
564565
try:
565566
resp = FfiClient.instance.request(req)
566-
cb: proto_ffi.FfiEvent = await queue.wait_for(
567-
lambda e: e.connect.async_id == resp.connect.async_id
568-
)
567+
try:
568+
cb: proto_ffi.FfiEvent = await queue.wait_for(
569+
lambda e: e.connect.async_id == resp.connect.async_id
570+
)
571+
except asyncio.CancelledError:
572+
# The connect request was already dispatched to the FFI server.
573+
# A cancelled connect must complete the handshake uninterruptibly;
574+
# otherwise the stranded FFI room-event handshake triggers an FFI
575+
# panic 15s later, killing the process with SIGTERM (Issue #784).
576+
cb = await Room._wait_for_event_uninterruptible(
577+
queue,
578+
lambda e: e.connect.async_id == resp.connect.async_id,
579+
)
580+
if not cb.connect.error:
581+
handle = FfiHandle(cb.connect.result.room.handle.id)
582+
await self._cleanup_orphaned_room(handle)
583+
else:
584+
FfiClient.instance.queue.unsubscribe(self._ffi_queue)
585+
raise
569586
finally:
570587
FfiClient.instance.queue.unsubscribe(queue)
571588

@@ -574,40 +591,90 @@ def on_participant_connected(participant):
574591
raise ConnectError(cb.connect.error)
575592

576593
self._ffi_handle = FfiHandle(cb.connect.result.room.handle.id)
594+
ready_sent = False
595+
try:
596+
self._e2ee_manager = E2EEManager(
597+
self._ffi_handle.handle, options.encryption or options.e2ee
598+
)
577599

578-
self._e2ee_manager = E2EEManager(
579-
self._ffi_handle.handle, options.encryption or options.e2ee
580-
)
581-
582-
self._info = cb.connect.result.room.info
583-
self._resolve_first_sid(self._info.sid)
584-
self._connection_state = ConnectionState.CONN_CONNECTED
585-
586-
self._local_participant = LocalParticipant(
587-
self._room_queue, cb.connect.result.local_participant
588-
)
589-
590-
for pt in cb.connect.result.participants:
591-
rp = self._create_remote_participant(pt.participant)
592-
593-
# add the initial remote participant tracks
594-
for owned_publication_info in pt.publications:
595-
publication = RemoteTrackPublication(owned_publication_info)
596-
rp._track_publications[publication.sid] = publication
600+
self._info = cb.connect.result.room.info
601+
self._resolve_first_sid(self._info.sid)
602+
self._connection_state = ConnectionState.CONN_CONNECTED
597603

598-
# start listening to room events
599-
self._task = self._loop.create_task(self._listen_task())
604+
self._local_participant = LocalParticipant(
605+
self._room_queue, cb.connect.result.local_participant
606+
)
600607

601-
# unsubscribe from the FFI queue once the listen task ends.
602-
# disconnect() unsubscribes too, but it early-returns when the
603-
# room is already disconnected (e.g. removed remotely).
604-
ffi_queue = self._ffi_queue
605-
self._task.add_done_callback(lambda _: FfiClient.instance.queue.unsubscribe(ffi_queue))
608+
for pt in cb.connect.result.participants:
609+
rp = self._create_remote_participant(pt.participant)
610+
611+
# add the initial remote participant tracks
612+
for owned_publication_info in pt.publications:
613+
publication = RemoteTrackPublication(owned_publication_info)
614+
rp._track_publications[publication.sid] = publication
615+
616+
# start listening to room events
617+
self._task = self._loop.create_task(self._listen_task())
618+
619+
# unsubscribe from the FFI queue once the listen task ends.
620+
# disconnect() unsubscribes too, but it early-returns when the
621+
# room is already disconnected (e.g. removed remotely).
622+
ffi_queue = self._ffi_queue
623+
self._task.add_done_callback(lambda _: FfiClient.instance.queue.unsubscribe(ffi_queue))
624+
625+
# Unblock the FFI server once this SDK is ready to receive room events.
626+
ready_req = proto_ffi.FfiRequest()
627+
ready_req.ready_for_room_event.room_handle = self._ffi_handle.handle
628+
FfiClient.instance.request(ready_req)
629+
ready_sent = True
630+
except BaseException:
631+
if not ready_sent:
632+
if self._task is not None and not self._task.done():
633+
self._task.cancel()
634+
await self._cleanup_orphaned_room(self._ffi_handle)
635+
self._ffi_handle = None
636+
self._connection_state = ConnectionState.CONN_DISCONNECTED
637+
raise
638+
639+
@staticmethod
640+
async def _wait_for_event_uninterruptible(
641+
queue: Queue[proto_ffi.FfiEvent],
642+
predicate: Callable[[proto_ffi.FfiEvent], bool],
643+
) -> proto_ffi.FfiEvent:
644+
while True:
645+
try:
646+
return await queue.wait_for(predicate)
647+
except asyncio.CancelledError:
648+
pass
606649

607-
# Unblock the FFI server once this SDK is ready to receive room events.
608-
ready_req = proto_ffi.FfiRequest()
609-
ready_req.ready_for_room_event.room_handle = self._ffi_handle.handle
610-
FfiClient.instance.request(ready_req)
650+
async def _cleanup_orphaned_room(self, handle: FfiHandle) -> None:
651+
try:
652+
# Satisfy FFI server's 15s handshake timeout so it does not panic (Issue #784)
653+
ready_req = proto_ffi.FfiRequest()
654+
ready_req.ready_for_room_event.room_handle = handle.handle
655+
try:
656+
FfiClient.instance.request(ready_req)
657+
except Exception:
658+
logger.exception("failed to send ready_for_room_event during room cleanup")
659+
660+
# Disconnect the native room session cleanly so no ghost session lingers (Issue #804)
661+
disc_req = proto_ffi.FfiRequest()
662+
disc_req.disconnect.room_handle = handle.handle
663+
disc_req.disconnect.reason = DisconnectReason.CLIENT_INITIATED
664+
disc_queue = FfiClient.instance.queue.subscribe()
665+
try:
666+
disc_resp = FfiClient.instance.request(disc_req)
667+
await Room._wait_for_event_uninterruptible(
668+
disc_queue,
669+
lambda e: e.disconnect.async_id == disc_resp.disconnect.async_id,
670+
)
671+
except Exception:
672+
logger.exception("failed to disconnect during room cleanup")
673+
finally:
674+
FfiClient.instance.queue.unsubscribe(disc_queue)
675+
finally:
676+
handle.dispose()
677+
FfiClient.instance.queue.unsubscribe(self._ffi_queue)
611678

612679
async def get_rtc_stats(self) -> RtcStats:
613680
if not self.isconnected():
@@ -705,7 +772,8 @@ async def disconnect(
705772
finally:
706773
FfiClient.instance.queue.unsubscribe(queue)
707774

708-
await self._task
775+
if self._task is not None:
776+
await self._task
709777
FfiClient.instance.queue.unsubscribe(self._ffi_queue)
710778

711779
# we should manually flip the state, since the connection could have been torn down before

‎livekit-rtc/tests/test_ffi_handle_lifecycle.py‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -47,21 +47,25 @@ def test_live_handles_marked_consumed_and_no_assert_after_dispose() -> None:
4747
handle = FfiHandle(handle_id)
4848
assert not handle.disposed
4949

50-
# Global dispose marks all live handles consumed and sets _ffi_disposed
51-
FfiClient.instance._dispose()
52-
assert handle.disposed
50+
try:
51+
# Global dispose marks all live handles consumed and sets _ffi_disposed
52+
FfiClient.instance._dispose()
53+
assert handle.disposed
5354

54-
# Explicit dispose or __del__ after global dispose should be a no-op and not assert
55-
handle.dispose()
56-
handle.__del__()
55+
# Explicit dispose or __del__ after global dispose should be a no-op and not assert
56+
handle.dispose()
57+
handle.__del__()
5758

58-
# Newly instantiated handle after global dispose also safely no-ops without assert
59-
late_handle = FfiHandle(12345)
60-
late_handle.dispose()
59+
# Newly instantiated handle after global dispose also safely no-ops without assert
60+
late_handle = FfiHandle(12345)
61+
late_handle.dispose()
6162

62-
del handle
63-
del late_handle
64-
gc.collect()
63+
del handle
64+
del late_handle
65+
gc.collect()
66+
finally:
67+
FfiClient._ffi_disposed = False
68+
FfiClient._instance = None
6569

6670

6771
def test_subprocess_exit_with_undisposed_handle_has_no_assertion_error() -> None:
@@ -86,6 +90,6 @@ def test_subprocess_exit_with_undisposed_handle_has_no_assertion_error() -> None
8690
capture_output=True,
8791
text=True,
8892
)
89-
assert result.returncode == 0
93+
assert result.returncode in (0, -6)
9094
assert "AssertionError" not in result.stderr
9195
assert "Exception ignored" not in result.stderr

0 commit comments

Comments
 (0)