Skip to content

Commit 5e6bddc

Browse files
tmkarthiclaude
andcommitted
fix(rtc): satisfy mypy strict and preserve track cleanup on unpublish race
Address two issues from review of the local_track_unpublished fix: 1. mypy strict (CI gate) flagged room.py: `pop(sid, None)` was assigned to `lpublication`, which is also bound to `track_publications[sid]` (non-optional LocalTrackPublication) in sibling branches of the same method. mypy fixes the variable's type from that first binding and pushes it as the expected return type into `pop`, selecting the `pop(key, default: _VT) -> _VT` overload and rejecting None. Use a fresh variable with `.get()` + conditional `del`, mirroring the existing local_track_republished handler, which already passes the gate. 2. With the handler now removing the publication first in the common ordering, unpublish_track's `pop(track_sid, None)` returned None and skipped the `_track = None` cleanup, leaving a LocalTrackPublication reference held by application code reporting a stale non-None track after `await unpublish_track(...)` completed. Capture the publication reference before the FFI round-trip so the track is cleared once unpublish completes regardless of which path removes it from the dict. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 2dbb047 commit 5e6bddc

2 files changed

Lines changed: 20 additions & 15 deletions

File tree

livekit-rtc/livekit/rtc/participant.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -784,6 +784,13 @@ async def unpublish_track(self, track_sid: str) -> None:
784784
Raises:
785785
UnpublishTrackError: If there is an error in unpublishing the track.
786786
"""
787+
# Capture the publication before the FFI round-trip. The
788+
# local_track_unpublished room event races this async response and may
789+
# remove it from _track_publications first; holding our own reference
790+
# guarantees the track is cleared once unpublish completes, regardless
791+
# of which path removes the publication from the dict.
792+
publication = self._track_publications.get(track_sid)
793+
787794
req = proto_ffi.FfiRequest()
788795
req.unpublish_track.local_participant_handle = self._ffi_handle.handle
789796
req.unpublish_track.track_sid = track_sid
@@ -799,10 +806,9 @@ async def unpublish_track(self, track_sid: str) -> None:
799806
if cb.unpublish_track.error:
800807
raise UnpublishTrackError(cb.unpublish_track.error)
801808

802-
# The local_track_unpublished room event may have already removed
803-
# this publication from the dict (the FFI event and this async
804-
# response race during teardown), so pop defensively.
805-
publication = self._track_publications.pop(track_sid, None)
809+
# Remove defensively: the room-event handler may already have done
810+
# so when it processed local_track_unpublished first.
811+
self._track_publications.pop(track_sid, None)
806812
if publication is not None:
807813
publication._track = None
808814
queue.task_done()

livekit-rtc/livekit/rtc/room.py

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -737,19 +737,18 @@ def _on_room_event(self, event: proto_room.RoomEvent) -> None:
737737
elif which == "local_track_unpublished":
738738
# During teardown the publication may already have been removed
739739
# from the participant's dict by LocalParticipant.unpublish_track
740-
# (or by a previously processed event), so the SID can be gone by
741-
# the time this event is dispatched. Pop defensively and skip the
742-
# emit when it is no longer tracked, mirroring the remote
743-
# track_unpublished and local_track_republished handlers, instead
744-
# of raising a KeyError that _listen_task logs as an error.
740+
# (the FFI event races that async response), so the SID can be gone
741+
# by the time this event is dispatched. Look it up defensively and
742+
# skip the emit when it is no longer tracked, mirroring the
743+
# local_track_republished and remote track_unpublished handlers,
744+
# instead of raising a KeyError that _listen_task logs as an error.
745745
sid = event.local_track_unpublished.publication_sid
746-
lpublication = self.local_participant._track_publications.pop(sid, None)
747-
if lpublication is not None:
748-
self.emit("local_track_unpublished", lpublication)
746+
unpublished = self.local_participant._track_publications.get(sid)
747+
if unpublished is not None:
748+
del self.local_participant._track_publications[sid]
749+
self.emit("local_track_unpublished", unpublished)
749750
else:
750-
logging.debug(
751-
"local_track_unpublished for untracked publication sid %s", sid
752-
)
751+
logging.debug("local_track_unpublished for untracked publication sid %s", sid)
753752
elif which == "local_track_republished":
754753
# The SDK auto-republished a local track during a full
755754
# reconnect: the underlying Track (and its bound source) is

0 commit comments

Comments
 (0)