rtc: shield the aborted-connect cleanup from a cancelled disconnect

gather() cancels its children when it is cancelled, so a caller who bounds
disconnect() with a timeout, or abandons it during shutdown, cancelled the
cleanup task that answers the FFI's wait for ReadyForRoomEvent. The wait then
timed out and panicked, and the panic handler kills the process, which is the
failure this path was added to prevent.

The test fails without the shield.

Author: ubmids

livekit-rtc/livekit/rtc/room.py

@@ -741,7 +741,14 @@ async def disconnect(
         if self._aborted_connect_tasks:
             # a cancelled connect may still be closing a room the FFI server opened.
             # wait for it so disconnect() leaves nothing behind.
-            await asyncio.gather(*tuple(self._aborted_connect_tasks), return_exceptions=True)
+            #
+            # shielded, because gather() cancels its children when it is cancelled.
+            # a caller who gives up on disconnect() would otherwise cancel the very
+            # cleanup that answers the FFI's wait, leaving it to time out and panic,
+            # which is the failure this path exists to prevent.
+            await asyncio.shield(
+                asyncio.gather(*tuple(self._aborted_connect_tasks), return_exceptions=True)
+            )
 
         if not self.isconnected():
             return

livekit-rtc/tests/test_connect_cancellation.py

@@ -116,3 +116,39 @@ async def test_cancelled_connect_leaves_no_pending_work_when_the_server_errors(
     # there is no room to close, so nothing follows the connect
     assert [req.WhichOneof("message") for req in requests] == ["connect"]
     assert len(FfiClient.instance.queue._subscribers) == subscribers_before
+
+
+async def test_a_cancelled_disconnect_still_lets_the_cleanup_finish(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Giving up on disconnect() must not cancel the cleanup it is waiting on.
+
+    gather() cancels its children when it is cancelled, so a caller who bounds
+    disconnect() with a timeout, or abandons it on shutdown, would cancel the
+    task that answers the FFI's wait for ReadyForRoomEvent. The wait then times
+    out, the FFI panics, and the panic handler kills the process: the exact
+    failure the rest of this file is about, reintroduced one layer up.
+    """
+    requests = _install_fake_ffi(monkeypatch)
+
+    room = rtc.Room()
+    task = asyncio.create_task(room.connect("ws://localhost:7880", "token"))
+    await wait_until(lambda: bool(requests), message="connect request never issued")
+
+    task.cancel()
+    with pytest.raises(asyncio.CancelledError):
+        await task
+
+    # the cleanup is now parked on the connect callback, which has not arrived
+    closing = asyncio.create_task(room.disconnect())
+    await asyncio.sleep(0)
+    closing.cancel()
+    with pytest.raises(asyncio.CancelledError):
+        await closing
+
+    _deliver_connect_callback()
+    await wait_until(
+        lambda: any(r.WhichOneof("message") == "ready_for_room_event" for r in requests),
+        message="the cancelled disconnect took the cleanup down with it",
+    )
+    assert requests[1].ready_for_room_event.room_handle == ROOM_HANDLE