Skip to content

Commit 1448e0e

Browse files
fix(rtc): synchronize stream writer aclose with in-flight writes (#879)
1 parent ab2be1a commit 1448e0e

2 files changed

Lines changed: 97 additions & 12 deletions

File tree

‎livekit-rtc/livekit/rtc/data_stream.py‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,7 @@ def __init__(
389389
self,
390390
local_participant: LocalParticipant,
391391
topic: str = "",
392-
attributes: Optional[Dict[str, str]] = {},
392+
attributes: Optional[Dict[str, str]] = None,
393393
stream_id: str | None = None,
394394
total_size: int | None = None,
395395
mime_type: str = "",
@@ -456,16 +456,17 @@ async def _wait_for_callback(
456456
async def aclose(
457457
self, *, reason: str = "", attributes: Optional[Dict[str, str]] = None
458458
) -> None:
459-
if self._closed:
460-
raise RuntimeError("Stream already closed")
461-
if self._writer_handle is None:
462-
raise RuntimeError("Stream is not open")
463-
self._closed = True
464-
# unregister before sending: the request consumes the handle natively,
465-
# so it must not be dropped again by the disconnect cleanup even if the
466-
# close itself reports an error
467-
self._unregister_open()
468-
await self._send_close(reason=reason, attributes=attributes)
459+
async with self._write_lock:
460+
if self._closed:
461+
raise RuntimeError("Stream already closed")
462+
if self._writer_handle is None:
463+
raise RuntimeError("Stream is not open")
464+
self._closed = True
465+
# unregister before sending: the request consumes the handle natively,
466+
# so it must not be dropped again by the disconnect cleanup even if the
467+
# close itself reports an error
468+
self._unregister_open()
469+
await self._send_close(reason=reason, attributes=attributes)
469470

470471
async def _send_close(self, *, reason: str, attributes: Optional[Dict[str, str]]) -> None:
471472
raise NotImplementedError
@@ -477,7 +478,7 @@ def __init__(
477478
local_participant: LocalParticipant,
478479
*,
479480
topic: str = "",
480-
attributes: Optional[Dict[str, str]] = {},
481+
attributes: Optional[Dict[str, str]] = None,
481482
stream_id: str | None = None,
482483
total_size: int | None = None,
483484
reply_to_id: str | None = None,
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
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+
from __future__ import annotations
16+
17+
import asyncio
18+
from unittest.mock import MagicMock
19+
20+
import pytest
21+
22+
from livekit.rtc.data_stream import TextStreamWriter
23+
from livekit.rtc.participant import LocalParticipant
24+
25+
26+
def test_stream_writer_attributes_default_is_none() -> None:
27+
participant = MagicMock(spec=LocalParticipant)
28+
participant.identity = "test-user"
29+
participant._open_stream_writers = {}
30+
31+
writer1 = TextStreamWriter(participant, topic="t1")
32+
writer2 = TextStreamWriter(participant, topic="t2")
33+
34+
# attributes default should be None rather than a shared mutable dict
35+
assert writer1._options.attributes == {}
36+
assert writer2._options.attributes == {}
37+
38+
39+
async def test_aclose_waits_for_inflight_write() -> None:
40+
participant = MagicMock(spec=LocalParticipant)
41+
participant.identity = "test-user"
42+
participant._open_stream_writers = {}
43+
44+
writer = TextStreamWriter(participant, topic="t")
45+
writer._register_open(9999)
46+
47+
order: list[str] = []
48+
write_in_progress = asyncio.Event()
49+
allow_write_finish = asyncio.Event()
50+
51+
async def mock_wait_for_callback(req: object, callback_field: str, response_field: str) -> None:
52+
if callback_field == "text_stream_writer_write":
53+
order.append("write_start")
54+
write_in_progress.set()
55+
await allow_write_finish.wait()
56+
order.append("write_done")
57+
elif callback_field == "text_stream_writer_close":
58+
order.append("close_sent")
59+
60+
writer._wait_for_callback = mock_wait_for_callback # type: ignore[assignment]
61+
62+
# Start write in background
63+
write_task = asyncio.create_task(writer.write("chunk"))
64+
await write_in_progress.wait()
65+
66+
# Initiate aclose while write is still in flight
67+
close_task = asyncio.create_task(writer.aclose())
68+
69+
# Give close_task a chance to run; it must not execute close yet because write holds lock
70+
await asyncio.sleep(0.01)
71+
assert order == ["write_start"]
72+
73+
# Now allow write to complete
74+
allow_write_finish.set()
75+
await write_task
76+
await close_task
77+
78+
# Close must execute only AFTER write completes
79+
assert order == ["write_start", "write_done", "close_sent"]
80+
assert writer._closed is True
81+
82+
# Subsequent write must immediately reject
83+
with pytest.raises(RuntimeError, match="Cannot write after stream is closed"):
84+
await writer.write("late_chunk")

0 commit comments

Comments
 (0)