Skip to content

Commit 077a607

Browse files
pblazejclaude
authored andcommitted
Fix m144 teardown crashes (livekit#967)
[SymbolicatedCrashes.zip](https://github.com/user-attachments/files/26703300/SymbolicatedCrashes.zip) Calling `removeTrack` for all senders before `close()` puts WebRTC into a partially-cleaned state that causes `Close()` to follow a different teardown path, crashing on the worker thread. `RemoveTrackOrError` (`pc/peer_connection.cc:975`) nulls the sender track and changes transceiver direction: ```cpp sender->SetTrack(nullptr); // nulls track transceiver->internal()->set_direction(...); // kSendRecv → kRecvOnly sdp_handler_->UpdateNegotiationNeeded(); ``` `SetTrack(nullptr)` triggers `ClearSend()` (`pc/rtp_sender.cc:494`) because `prev_can_send_track` was true: ```cpp track_ = nullptr; if (can_send_track()) { // false — track_ is null SetSend(); } else if (prev_can_send_track) { // true ClearSend(); // disables audio sending on worker thread } ``` ```cpp void AudioRtpSender::ClearSend() { bool success = worker_thread_->BlockingCall([&] { return voice_media_channel()->SetAudioSend(ssrc_, false, &options, nullptr); }); } ``` Later, `Close()` (`pc/peer_connection.cc:1876`) runs `StopTransceiverProcedure` → `sender->Stop()` (`pc/rtp_sender.cc:562`): ```cpp void RtpSenderBase::Stop() { if (stopped_) return; if (track_) { // NULL — removeTrack already set it DetachTrack(); // SKIPPED track_->UnregisterObserver(this); // SKIPPED } if (can_send_track()) { // FALSE — track_ is null ClearSend(); // SKIPPED RemoveTrackFromStats(); // SKIPPED } media_channel_ = nullptr; stopped_ = true; } ``` Without `removeTrack`, `Stop()` runs the full path: `DetachTrack` → `ClearSend` → `RemoveTrackFromStats`. With it, all three are skipped. The sender enters `DeleteChannel` in a partially-cleaned state. `DeleteChannel` (`pc/rtp_transceiver.cc:385`) then destroys the channel on the worker thread: ```cpp void RtpTransceiver::DeleteChannel() { context()->worker_thread()->BlockingCall([&]() { auto channel_to_delete = std::move(channel_); for (auto& sender : senders_) sender->internal()->SetMediaChannel(nullptr); for (auto& receiver : receivers_) receiver->internal()->SetMediaChannel(nullptr); channel_to_delete.reset(); // channel destroyed → [AVAudioEngine stop] media_engine_ref_.reset(); }); } ``` The channel destructor encountering the modified state leads to crash 1 (freed Port/socket resources accessed during ICE transport teardown) and crash 2 (inconsistent audio engine state → assertion in buffer deallocation during `[AVAudioEngine stop]`). Both `removeTrack` and `close()` go through `PeerConnectionProxy` (`pc/proxy.h`), which marshals them synchronously to the signaling thread — no threading race between them. The issue is the modified internal state, not concurrency. --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> (cherry picked from commit 00d3ca9)
1 parent ae1ff46 commit 077a607

2 files changed

Lines changed: 6 additions & 4 deletions

File tree

.changes/teardown-crash

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
patch type="fixed" "Crashes during track teardown in WebRTC m144"

Sources/LiveKit/Core/Transport.swift

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,11 +224,12 @@ actor Transport: NSObject, Loggable {
224224

225225
// Stop listening to delegate
226226
_pc.delegate = nil
227-
// Remove all senders (if any)
228-
for sender in _pc.senders {
229-
_pc.removeTrack(sender)
230-
}
231227

228+
// Do not call removeTrack before close — it nulls sender tracks and
229+
// changes transceiver directions, causing Close() to skip ClearSend/
230+
// DetachTrack in its StopTransceiverProcedure and hit edge cases in
231+
// the worker-thread teardown (ICE use-after-free, AVAudioEngine
232+
// deallocation assertion). Close() handles full cleanup on its own.
232233
_pc.close()
233234
}
234235
}

0 commit comments

Comments
 (0)