livekit / python-sdks

LiveKit real-time and server SDKs for Python
https://docs.livekit.io
Apache License 2.0
123 stars 37 forks source link

Room._listen_task is not cancelled when server closes room #199

Open gigaverse-oz opened 5 months ago

gigaverse-oz commented 5 months ago

Issue Description

Overview

When the Livekit Server terminates a room and removes a Python client, the task responsible for listening (self._listen_task) does not properly conclude or cancel.

Background

We have set up a Python participant to join a specific room and listen to a particular track. If the track is no longer published, the Python participant leaves the room, completes all asynchronous tasks, and then the process exits. However, if the room is deleted, the Python participant struggles to disconnect and the _listen_task remains active.

Diagnostics

The Room.disconnect() method is designed to clear the task queue and wait for self._task to finish. But, if the Livekit Server deletes the room, the Room.disconnect() cannot complete as it fails the self.isconnected() check, resulting in self._task remaining in a running state.

if not self.isconnected():
            return

View the code segment here: Room.py Line 221

Requested Solution

We suggest introducing an aclose() function to manage disconnections and clean up any remaining asynchronous tasks. Currently, we have implemented the following in the object that utilizes the Room object:

    async def aclose(self):
        if self.is_closed():
            return
        try:
            for task in self._tasks:
                task.cancel()
            await asyncio.gather(*self._tasks, return_exceptions=True)
            await self._room.disconnect()
        except Exception as err:
            logger.error(f"Error shutting down transcript agent: {str(err)}")
        finally:
            if self._room._task:
                self._room._task.cancel()
                logger.warning("Room disconnect method failed to close _listen_task")
            self._closed = True
theomonnom commented 5 months ago

The correct implementation would be to always call self._room.disconnect like you do, the difference is that we're now awaiting the main task in https://github.com/livekit/python-sdks/pull/200. There is no need to cancel the task because the Rust SDK should send EOS which will break the infinite loop

theomonnom commented 5 months ago

I'm going to verify that this line is correctly ran if the disconnect comes from the server before merging the above PR

gigaverse-oz commented 5 months ago

Thanks for the quick reply and fix. Will also verify once merged :)