Skip to content

fix(rtc): mark room queue task done on publish and unpublish track errors - #902

Open
NabiBukhsh-AI wants to merge 1 commit into
livekit:mainfrom
NabiBukhsh-AI:fix/rtc-publish-track-error-deadlock
Open

NabiBukhsh-AI wants to merge 1 commit into
livekit:mainfrom
NabiBukhsh-AI:fix/rtc-publish-track-error-deadlock

Conversation

@NabiBukhsh-AI

Copy link
Copy Markdown
Contributor

Summary

In LocalParticipant.publish_track and LocalParticipant.unpublish_track, queue.task_done() was placed at the end of the try: block, after checking for FFI errors (if cb.publish_track.error: / if cb.unpublish_track.error:).

When the FFI reports an error, the exception (PublishTrackError / UnpublishTrackError) is raised before queue.task_done() is called. This leaves the room queue event unacknowledged during active subscription and keeps Room._listen_task's await self._room_queue.join() waiting on that event until unsubscription teardown.

This resolves the sibling bug to #565 (as noted by Devin review on #752) and ensures both methods adhere to Queue.wait_for's contract (task_done must be manually called for the returned item).

Changes

  • Move queue.task_done() immediately after await queue.wait_for(...) in both LocalParticipant.publish_track and LocalParticipant.unpublish_track.
  • Add regression test suite livekit-rtc/tests/test_publish_track_deadlock.py verifying that:
    • task_done() is invoked while the subscriber is still active when an FFI error occurs.
    • Room._listen_task's room_queue.join() unblocks immediately upon error event receipt.

Testing

  • Ran pytest livekit-rtc/tests/test_publish_track_deadlock.py (all tests pass).
  • Ran ruff check and ruff format --check (passes).
  • Ran mypy (no type issues found).

Closes #565

In LocalParticipant.publish_track and LocalParticipant.unpublish_track,
queue.task_done() was placed at the bottom of the try: block, after
checking for FFI errors. If an error occurred or an exception was raised,
queue.task_done() was skipped during active subscription.

Move queue.task_done() immediately after await queue.wait_for(...) in
both methods, satisfying Queue.wait_for's contract and unblocking
Room._listen_task's queue join immediately upon event receipt.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Devin Review: 1 flag

Not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)

Devin Review

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unpublish track failure causes deadlock

1 participant