Skip to content

Commit 3effff4

Browse files
committed
fix(rtc): mark room queue task done on publish and unpublish track error
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.
1 parent 1448e0e commit 3effff4

2 files changed

Lines changed: 235 additions & 2 deletions

File tree

‎livekit-rtc/livekit/rtc/participant.py‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,6 +1089,10 @@ async def publish_track(
10891089
cb: proto_ffi.FfiEvent = await queue.wait_for(
10901090
lambda e: e.publish_track.async_id == resp.publish_track.async_id
10911091
)
1092+
# wait_for hands back one event the caller owns; mark it done before the
1093+
# error check, otherwise raising here leaves the room queue's join()
1094+
# waiting on it forever and the room event loop stalls.
1095+
queue.task_done()
10921096

10931097
if cb.publish_track.error:
10941098
raise PublishTrackError(cb.publish_track.error)
@@ -1098,7 +1102,6 @@ async def publish_track(
10981102
track._info.sid = track_publication.sid
10991103
self._track_publications[track_publication.sid] = track_publication
11001104

1101-
queue.task_done()
11021105
return track_publication
11031106
finally:
11041107
self._room_queue.unsubscribe(queue)
@@ -1131,6 +1134,10 @@ async def unpublish_track(self, track_sid: str) -> None:
11311134
cb: proto_ffi.FfiEvent = await queue.wait_for(
11321135
lambda e: e.unpublish_track.async_id == resp.unpublish_track.async_id
11331136
)
1137+
# wait_for hands back one event the caller owns; mark it done before the
1138+
# error check, otherwise raising here leaves the room queue's join()
1139+
# waiting on it forever and the room event loop stalls.
1140+
queue.task_done()
11341141

11351142
if cb.unpublish_track.error:
11361143
raise UnpublishTrackError(cb.unpublish_track.error)
@@ -1149,7 +1156,6 @@ async def unpublish_track(self, track_sid: str) -> None:
11491156
if publication._track is not None:
11501157
publication._track._set_room(None)
11511158
publication._track = None
1152-
queue.task_done()
11531159
finally:
11541160
self._room_queue.unsubscribe(queue)
11551161

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
1+
# Copyright 2026 LiveKit, Inc.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
"""Regression tests for publish_track and unpublish_track room queue lifecycle.
16+
17+
When the FFI reports a publish or unpublish error, publish_track/unpublish_track raises.
18+
The event returned by queue.wait_for() must be marked done immediately so that
19+
Room._listen_task's join() does not remain blocked on the unconsumed event.
20+
"""
21+
22+
from __future__ import annotations
23+
24+
import asyncio
25+
from unittest.mock import MagicMock, patch
26+
27+
import pytest
28+
29+
from livekit import rtc
30+
from livekit.rtc._ffi_client import FfiClient
31+
from livekit.rtc._proto import ffi_pb2 as proto_ffi
32+
from livekit.rtc._utils import BroadcastQueue, Queue
33+
from livekit.rtc.participant import PublishTrackError, UnpublishTrackError
34+
35+
_ASYNC_ID = 54321
36+
37+
38+
@pytest.mark.asyncio
39+
async def test_publish_track_marks_event_done_before_raising_error() -> None:
40+
"""Verify that publish_track marks the event done immediately upon receipt,
41+
42+
even when the FFI event reports an error. It must not rely solely on
43+
unsubscription release to account for the event.
44+
"""
45+
room_queue: BroadcastQueue[proto_ffi.FfiEvent] = BroadcastQueue()
46+
47+
participant = rtc.LocalParticipant.__new__(rtc.LocalParticipant)
48+
participant._room_queue = room_queue
49+
participant._ffi_handle = MagicMock(handle=1)
50+
participant._track_publications = {}
51+
52+
track = MagicMock()
53+
track._ffi_handle = MagicMock(handle=2)
54+
55+
resp = proto_ffi.FfiResponse()
56+
resp.publish_track.async_id = _ASYNC_ID
57+
58+
error_event = proto_ffi.FfiEvent()
59+
error_event.publish_track.async_id = _ASYNC_ID
60+
error_event.publish_track.error = "publish track failed"
61+
62+
called_while_subscribed = False
63+
64+
orig_subscribe = room_queue.subscribe
65+
66+
def spy_subscribe() -> Queue[proto_ffi.FfiEvent]:
67+
q = orig_subscribe()
68+
orig_task_done = q.task_done
69+
70+
def spy_task_done() -> None:
71+
nonlocal called_while_subscribed
72+
if q in room_queue._subscribers:
73+
called_while_subscribed = True
74+
orig_task_done()
75+
76+
q.task_done = spy_task_done # type: ignore[method-assign]
77+
return q
78+
79+
room_queue.subscribe = spy_subscribe # type: ignore[method-assign]
80+
81+
with patch.object(FfiClient.instance, "request", return_value=resp):
82+
pub_task = asyncio.create_task(participant.publish_track(track))
83+
await asyncio.sleep(0) # let publish_track subscribe
84+
85+
room_queue.put_nowait(error_event)
86+
87+
with pytest.raises(PublishTrackError, match="publish track failed"):
88+
await pub_task
89+
90+
assert called_while_subscribed, (
91+
"queue.task_done() was not called for the event while the subscriber was active"
92+
)
93+
94+
95+
@pytest.mark.asyncio
96+
async def test_publish_track_error_releases_room_queue_join() -> None:
97+
"""Verify that Room._listen_task's join() finishes cleanly when publish_track fails."""
98+
room_queue: BroadcastQueue[proto_ffi.FfiEvent] = BroadcastQueue()
99+
100+
participant = rtc.LocalParticipant.__new__(rtc.LocalParticipant)
101+
participant._room_queue = room_queue
102+
participant._ffi_handle = MagicMock(handle=1)
103+
participant._track_publications = {}
104+
105+
track = MagicMock()
106+
track._ffi_handle = MagicMock(handle=2)
107+
108+
resp = proto_ffi.FfiResponse()
109+
resp.publish_track.async_id = _ASYNC_ID
110+
111+
error_event = proto_ffi.FfiEvent()
112+
error_event.publish_track.async_id = _ASYNC_ID
113+
error_event.publish_track.error = "publish track failed"
114+
115+
async def deliver_event_like_listen_task() -> None:
116+
room_queue.put_nowait(error_event)
117+
await room_queue.join()
118+
119+
with patch.object(FfiClient.instance, "request", return_value=resp):
120+
pub_task = asyncio.create_task(participant.publish_track(track))
121+
await asyncio.sleep(0)
122+
listen_task = asyncio.create_task(deliver_event_like_listen_task())
123+
124+
try:
125+
with pytest.raises(PublishTrackError):
126+
await pub_task
127+
128+
try:
129+
await asyncio.wait_for(listen_task, timeout=2.0)
130+
except asyncio.TimeoutError:
131+
pytest.fail("room queue join() deadlocked after publish_track error")
132+
finally:
133+
if not listen_task.done():
134+
listen_task.cancel()
135+
136+
137+
@pytest.mark.asyncio
138+
async def test_unpublish_track_marks_event_done_before_raising_error() -> None:
139+
"""Verify that unpublish_track marks the event done immediately upon receipt,
140+
141+
even when the FFI event reports an error. It must not rely solely on
142+
unsubscription release to account for the event.
143+
"""
144+
room_queue: BroadcastQueue[proto_ffi.FfiEvent] = BroadcastQueue()
145+
146+
participant = rtc.LocalParticipant.__new__(rtc.LocalParticipant)
147+
participant._room_queue = room_queue
148+
participant._ffi_handle = MagicMock(handle=1)
149+
participant._track_publications = {}
150+
151+
resp = proto_ffi.FfiResponse()
152+
resp.unpublish_track.async_id = _ASYNC_ID
153+
154+
error_event = proto_ffi.FfiEvent()
155+
error_event.unpublish_track.async_id = _ASYNC_ID
156+
error_event.unpublish_track.error = "unpublish track failed"
157+
158+
called_while_subscribed = False
159+
160+
orig_subscribe = room_queue.subscribe
161+
162+
def spy_subscribe() -> Queue[proto_ffi.FfiEvent]:
163+
q = orig_subscribe()
164+
orig_task_done = q.task_done
165+
166+
def spy_task_done() -> None:
167+
nonlocal called_while_subscribed
168+
if q in room_queue._subscribers:
169+
called_while_subscribed = True
170+
orig_task_done()
171+
172+
q.task_done = spy_task_done # type: ignore[method-assign]
173+
return q
174+
175+
room_queue.subscribe = spy_subscribe # type: ignore[method-assign]
176+
177+
with patch.object(FfiClient.instance, "request", return_value=resp):
178+
unpub_task = asyncio.create_task(participant.unpublish_track("TR_test"))
179+
await asyncio.sleep(0)
180+
181+
room_queue.put_nowait(error_event)
182+
183+
with pytest.raises(UnpublishTrackError, match="unpublish track failed"):
184+
await unpub_task
185+
186+
assert called_while_subscribed, (
187+
"queue.task_done() was not called for the event while the subscriber was active"
188+
)
189+
190+
191+
@pytest.mark.asyncio
192+
async def test_unpublish_track_error_releases_room_queue_join() -> None:
193+
"""Verify that Room._listen_task's join() finishes cleanly when unpublish_track fails."""
194+
room_queue: BroadcastQueue[proto_ffi.FfiEvent] = BroadcastQueue()
195+
196+
participant = rtc.LocalParticipant.__new__(rtc.LocalParticipant)
197+
participant._room_queue = room_queue
198+
participant._ffi_handle = MagicMock(handle=1)
199+
participant._track_publications = {}
200+
201+
resp = proto_ffi.FfiResponse()
202+
resp.unpublish_track.async_id = _ASYNC_ID
203+
204+
error_event = proto_ffi.FfiEvent()
205+
error_event.unpublish_track.async_id = _ASYNC_ID
206+
error_event.unpublish_track.error = "unpublish track failed"
207+
208+
async def deliver_event_like_listen_task() -> None:
209+
room_queue.put_nowait(error_event)
210+
await room_queue.join()
211+
212+
with patch.object(FfiClient.instance, "request", return_value=resp):
213+
unpub_task = asyncio.create_task(participant.unpublish_track("TR_test"))
214+
await asyncio.sleep(0)
215+
listen_task = asyncio.create_task(deliver_event_like_listen_task())
216+
217+
try:
218+
with pytest.raises(UnpublishTrackError):
219+
await unpub_task
220+
221+
try:
222+
await asyncio.wait_for(listen_task, timeout=2.0)
223+
except asyncio.TimeoutError:
224+
pytest.fail("room queue join() deadlocked after unpublish_track error")
225+
finally:
226+
if not listen_task.done():
227+
listen_task.cancel()

0 commit comments

Comments
 (0)