Detach publisher senders before closing the PC so a server-ended room releases its local tracks - #1444
Open
sgu-bithuman wants to merge 3 commits into
Open
Detach publisher senders before closing the PC so a server-ended room releases its local tracks#1444sgu-bithuman wants to merge 3 commits into
sgu-bithuman wants to merge 3 commits into
Conversation
… releases its local tracks
On a server-initiated disconnect (signal `Leave{Disconnect}`, room deleted,
participant removed, duplicate identity) the engine closes the publisher
PeerConnection before the room's unpublish loop runs:
rtc_session.rs on_signal_event(Leave) -> on_session_disconnected
-> SessionEvent::Close -> EngineInner::close -> SessionInner::close
-> publisher_pc.close() (tracks still attached)
-> EngineEvent::Disconnected -> RoomSession::handle_disconnected
-> RoomSession::close -> unpublish_track(sid) for each publication
-> rtc_engine.remove_track(sender)? (INVALID_STATE, aborts)
libwebrtc refuses RemoveTrack/SetTrack on a closed PeerConnection and
RtpSenderBase::Stop() never releases `track_`, so every RtpSender of the
closed PC keeps its MediaStreamTrack for the life of the process. The `?`
then skips `set_transceiver(None)`, the `local_track_unpublished` callback
and `publication.set_track(None)`, so the publication keeps the track too.
For an audio track this strands one native AudioSourceCapture thread
(~96 MB) per server-ended room; the client-initiated path (unpublish loop
first, then close) does not leak.
* SessionInner::close: remove every sender from the publisher PC while it
is still open, so the tracks are released before close().
* LocalParticipant::unpublish_track: do not abort the bookkeeping when the
engine's remove_track fails (the PC is already closed on this path).
* webrtc-sys RtpSender::track(): a sender legitimately has no track after
RemoveTrack; map the null to None instead of dereferencing it.
Measured with the python SDK (livekit 1.1.19, liblivekit_ffi 0.12.79),
counting AudioSourceCapture threads in /proc/self/task after a
server-deleted room, all wrappers dropped, 10 s settle:
stock 1 -> 1 (leaked); patched 1 -> 0. Client-ended control 1 -> 0 both.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BgyJFvVUi9ZjMPkqg2mFqA
|
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Patch bumps for the changed crates (livekit, webrtc-sys) and their transitive dependents (libwebrtc, livekit-capture, livekit-ffi), as the changeset check requires. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BgyJFvVUi9ZjMPkqg2mFqA
…y the closed-PC case
Review (Devin): swallowing every remove_track error made unpublish_track report
success while an open publisher PeerConnection could still be sending on the
attached sender.
libwebrtc's RemoveTrackOrError reports exactly one condition as INVALID_STATE,
"PeerConnection is closed" (api/peer_connection_interface.h); a sender it does
not know is not an error under Unified Plan. So unpublish_track now matches
EngineError::Rtc(RtcError { error_type: RtcErrorType::InvalidState, .. }) as
the expected outcome of a server-ended room (the engine already detached the
sender in SessionInner::close), and returns every other error to the caller —
after the local bookkeeping (set_transceiver(None), the unpublished callback,
publication.set_track(None)) has run, so the publication still drops its
reference to the local track.
The sender-detach loop in SessionInner::close keeps logging instead of
returning: close() has no error channel and aborting on one sender would
strand the rest.
Adds a unit test for the predicate and adjusts the changeset summary.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BgyJFvVUi9ZjMPkqg2mFqA
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1443.
On a server-initiated disconnect the engine closes the publisher
PeerConnection(SessionInner::close) beforeRoomSession::close's unpublish loop runs. libwebrtc refusesRemoveTrack/SetTrackon a closed PC (INVALID_STATE "PeerConnection is closed.") andRtpSenderBase::Stop()never releasestrack_, so every sender keeps itsMediaStreamTrackfor the life of the process;unpublish_track'sremove_track(sender)?then aborts beforeset_transceiver(None)/publication.set_track(None), so the publication keeps the track as well. For a local audio track that strands oneAudioSourceCapturethread per server-ended room. The client-initiated path (unpublish loop first, then engine close) does not leak.Path (main @ c944510)
livekit/src/rtc_engine/rtc_session.rs:2044on_session_disconnected(signalLeave{Disconnect}) →:2057emitsSessionEvent::Close→EngineInner::close→SessionInner::close(:2066)livekit/src/rtc_engine/rtc_session.rs:2074self.publisher_pc.close()— tracks still attachedEngineEvent::Disconnected→livekit/src/room/mod.rs:1807RoomSession::handle_disconnected→RoomSession::close→:1187unpublish_track(sid)per publicationlivekit/src/room/participant/local_participant.rs:670self.inner.rtc_engine.remove_track(sender)?— libwebrtc answersINVALID_STATE "PeerConnection is closed.", the?returns beforetrack.set_transceiver(None)/publication.set_track(None)/local_track_unpublished, andRtpSenderBase::Stop()has lefttrack_set, so the sender and the publication both keep theMediaStreamTrack.Changes
livekit/src/rtc_engine/rtc_session.rs:2074SessionInner::close: remove every sender from the publisher PC while it is still open, beforepublisher_pc.close(), so the tracks are released.livekit/src/room/participant/local_participant.rs:670unpublish_track: log and continue whenrtc_engine.remove_trackfails instead of?, so the transceiver/publication bookkeeping and thelocal_track_unpublishedcallback always run.webrtc-sys/src/rtp_sender.cpp:156RtpSender::track(): a sender legitimately has no track afterRemoveTrack; return null instead of handing a nullscoped_refptrtoget_or_create_media_stream_track(which dereferences it — without this,SessionInner::remove_track'ssender.track()crashes once the first change detaches the sender)..changeset/detach_senders_before_pc_close.md: patch bumps forlivekit,webrtc-sysand their dependents (libwebrtc,livekit-capture,livekit-ffi), as the changeset check requires.Measured
Python SDK
livekit1.1.19 with thisliblivekit_ffi.so(0.12.79 + this patch) dropped into the wheel, Linux x86_64;AudioSourceCapturethreads counted in/proc/self/taskafter all wrappers and FFI handles are dropped and a 10 s settle:The remaining thread set after the patched server-ended run is
asyncio_0, livekit-audio, python, tokio-rt-workeronly (stock still showsAudioSourceCapt, network_thread, signaling_threa, worker_thread 0, rtc-low-prio).🤖 Generated with Claude Code
https://claude.ai/code/session_01BgyJFvVUi9ZjMPkqg2mFqA