From 01fc4c135e9bd5a2a1ce66a39e611708dc3b7360 Mon Sep 17 00:00:00 2001 From: Alan George Date: Fri, 26 Jun 2026 16:58:46 -0600 Subject: [PATCH 1/3] Fix FFI dispose cleanup and platform ADM teardown race Clear remaining FFI handles during dispose so native resources are released across repeated initialize/shutdown cycles. Stop and detach platform/synthetic audio I/O before peer connection factory teardown so macOS CaptureWorkerThread cannot deliver frames into AudioTransportImpl while transports are being destroyed. Close rooms before dropping track handles and stop platform capture when releasing the platform ADM reference. Co-authored-by: Cursor --- .changeset/ffi-handle-cleanup.md | 5 ++ .../src/native/peer_connection_factory.rs | 8 ++ libwebrtc/src/peer_connection_factory.rs | 7 ++ livekit-ffi/src/server/mod.rs | 3 +- livekit-ffi/src/server/room.rs | 6 +- livekit/src/platform_audio/mod.rs | 5 +- livekit/src/rtc_engine/lk_runtime.rs | 8 ++ webrtc-sys/include/livekit/adm_proxy.h | 15 ++++ .../include/livekit/peer_connection_factory.h | 4 + webrtc-sys/src/adm_proxy.cpp | 73 ++++++++++++++++--- webrtc-sys/src/peer_connection_factory.cpp | 10 +++ webrtc-sys/src/peer_connection_factory.rs | 2 + 12 files changed, 130 insertions(+), 16 deletions(-) create mode 100644 .changeset/ffi-handle-cleanup.md diff --git a/.changeset/ffi-handle-cleanup.md b/.changeset/ffi-handle-cleanup.md new file mode 100644 index 000000000..7ec047614 --- /dev/null +++ b/.changeset/ffi-handle-cleanup.md @@ -0,0 +1,5 @@ +--- +livekit-ffi: patch +--- + +Clear remaining FFI handles during dispose so platform audio resources are released across repeated initialize/shutdown cycles. diff --git a/libwebrtc/src/native/peer_connection_factory.rs b/libwebrtc/src/native/peer_connection_factory.rs index b48f2bd83..406d9116f 100644 --- a/libwebrtc/src/native/peer_connection_factory.rs +++ b/libwebrtc/src/native/peer_connection_factory.rs @@ -316,6 +316,14 @@ impl PeerConnectionFactory { pub fn is_platform_adm_active(&self) -> bool { self.sys_handle.audio_device().is_platform_adm_active() } + + /// Stops platform/synthetic audio I/O and detaches WebRTC callbacks. + /// + /// Call before tearing down peer connections so capture worker threads + /// cannot deliver frames into transports that are being destroyed. + pub fn shutdown_audio_io(&self) { + self.sys_handle.shutdown_audio_io(); + } } #[cfg(test)] diff --git a/libwebrtc/src/peer_connection_factory.rs b/libwebrtc/src/peer_connection_factory.rs index d4e7cb8d9..6fdbcf486 100644 --- a/libwebrtc/src/peer_connection_factory.rs +++ b/libwebrtc/src/peer_connection_factory.rs @@ -156,6 +156,9 @@ pub mod native { fn release_platform_adm(&self); fn platform_adm_ref_count(&self) -> i32; fn is_platform_adm_active(&self) -> bool; + + /// Stops platform/synthetic audio I/O before runtime teardown. + fn shutdown_audio_io(&self); } impl PeerConnectionFactoryExt for PeerConnectionFactory { @@ -298,5 +301,9 @@ pub mod native { fn is_platform_adm_active(&self) -> bool { self.handle.is_platform_adm_active() } + + fn shutdown_audio_io(&self) { + self.handle.shutdown_audio_io(); + } } } diff --git a/livekit-ffi/src/server/mod.rs b/livekit-ffi/src/server/mod.rs index 1689bdbbe..b7b22bd9b 100644 --- a/livekit-ffi/src/server/mod.rs +++ b/livekit-ffi/src/server/mod.rs @@ -176,8 +176,9 @@ impl FfiServer { self.logger.set_capture_logs(false); - // Drop all handles *self.config.lock() = None; // Invalidate the config + self.ffi_handles.clear(); + self.handle_dropped_txs.clear(); } pub fn send_event(&self, message: proto::ffi_event::Message) -> FfiResult<()> { diff --git a/livekit-ffi/src/server/room.rs b/livekit-ffi/src/server/room.rs index fd7aefe63..ab52f1ee1 100644 --- a/livekit-ffi/src/server/room.rs +++ b/livekit-ffi/src/server/room.rs @@ -361,6 +361,10 @@ impl FfiRoom { /// Close the room and stop the tasks pub async fn close(&self, server: &'static FfiServer, reason: DisconnectReason) { + // Close the room first so local tracks are unpublished and WebRTC + // stops platform capture before FFI track handles are dropped. + let _ = self.inner.room.close_with_reason(reason.into()).await; + // drop associated track handles for (_, &handle) in self.inner.track_handle_lookup.lock().iter() { if server.drop_handle(handle) { @@ -369,8 +373,6 @@ impl FfiRoom { } } - let _ = self.inner.room.close_with_reason(reason.into()).await; - let handle = self.handle.lock().await.take(); if let Some(handle) = handle { let _ = handle.close_tx.send(()); diff --git a/livekit/src/platform_audio/mod.rs b/livekit/src/platform_audio/mod.rs index 7af17dfc2..63e9c90f3 100644 --- a/livekit/src/platform_audio/mod.rs +++ b/livekit/src/platform_audio/mod.rs @@ -304,8 +304,9 @@ struct PlatformAdmHandle { impl Drop for PlatformAdmHandle { fn drop(&mut self) { log::debug!("PlatformAdmHandle dropped - releasing Platform ADM"); - // Release Platform ADM reference - // When ref_count reaches 0, the Platform ADM is terminated + // Stop platform capture before releasing the ADM reference so the + // capture worker cannot deliver frames during teardown. + let _ = self.runtime.stop_recording(); self.runtime.release_platform_adm(); log::info!( "PlatformAdmHandle: released Platform ADM (ref_count now: {})", diff --git a/livekit/src/rtc_engine/lk_runtime.rs b/livekit/src/rtc_engine/lk_runtime.rs index 81ad95e70..287c5de6a 100644 --- a/livekit/src/rtc_engine/lk_runtime.rs +++ b/livekit/src/rtc_engine/lk_runtime.rs @@ -275,10 +275,18 @@ impl LkRuntime { pub(crate) fn is_platform_adm_active(&self) -> bool { self.pc_factory.is_platform_adm_active() } + + /// Stops platform/synthetic audio I/O before runtime teardown. + #[cfg(not(target_arch = "wasm32"))] + pub fn shutdown_audio_io(&self) { + self.pc_factory.shutdown_audio_io(); + } } impl Drop for LkRuntime { fn drop(&mut self) { log::debug!("LkRuntime::drop()"); + #[cfg(not(target_arch = "wasm32"))] + self.shutdown_audio_io(); } } diff --git a/webrtc-sys/include/livekit/adm_proxy.h b/webrtc-sys/include/livekit/adm_proxy.h index d099f916e..27969f4ec 100644 --- a/webrtc-sys/include/livekit/adm_proxy.h +++ b/webrtc-sys/include/livekit/adm_proxy.h @@ -93,6 +93,13 @@ class AdmProxy : public webrtc::AudioDeviceModule { /// Returns true if Platform ADM is currently active (ref_count > 0). bool is_platform_adm_active() const; + /// Stops platform and synthetic audio I/O and detaches the WebRTC + /// AudioTransport callback. + /// + /// Call before tearing down the peer connection factory so capture worker + /// threads cannot deliver frames into transports that are being destroyed. + void ShutdownAudioIO(); + // =========================================================================== // Recording/Playout Control // =========================================================================== @@ -227,6 +234,14 @@ class AdmProxy : public webrtc::AudioDeviceModule { // Must be called with mutex_ held. void SwitchRecordingAdmIfNeeded(); + // Stops active capture/playout and detaches audio callbacks from both ADMs. + // Must be called with mutex_ held. + void StopAudioIOLocked(); + + // Stops platform capture/playout and detaches its callback without touching + // synthetic mode. Must be called with mutex_ held. + void StopPlatformAudioIOLocked(); + #if defined(__ANDROID__) // Lazily creates the Platform ADM on Android. // Must be called with mutex_ held. diff --git a/webrtc-sys/include/livekit/peer_connection_factory.h b/webrtc-sys/include/livekit/peer_connection_factory.h index 371d5313a..048bb600c 100644 --- a/webrtc-sys/include/livekit/peer_connection_factory.h +++ b/webrtc-sys/include/livekit/peer_connection_factory.h @@ -68,6 +68,10 @@ class PeerConnectionFactory { RtpCapabilities rtp_receiver_capabilities(MediaType type) const; + /// Stops platform/synthetic audio I/O and detaches callbacks on the worker + /// thread before peer connection factory teardown. + void shutdown_audio_io() const; + std::shared_ptr rtc_runtime() const { return rtc_runtime_; } std::shared_ptr audio_device() const; diff --git a/webrtc-sys/src/adm_proxy.cpp b/webrtc-sys/src/adm_proxy.cpp index 209f39d53..7ae24b457 100644 --- a/webrtc-sys/src/adm_proxy.cpp +++ b/webrtc-sys/src/adm_proxy.cpp @@ -71,14 +71,18 @@ AdmProxy::AdmProxy(const webrtc::Environment& env, webrtc::Thread* worker_thread AdmProxy::~AdmProxy() { RTC_LOG(LS_VERBOSE) << "AdmProxy::~AdmProxy()"; - if (synthetic_adm_) { - synthetic_adm_->Terminate(); - synthetic_adm_ = nullptr; - } + { + webrtc::MutexLock lock(&mutex_); + StopAudioIOLocked(); + if (synthetic_adm_) { + synthetic_adm_->Terminate(); + synthetic_adm_ = nullptr; + } - if (platform_adm_) { - platform_adm_->Terminate(); - platform_adm_ = nullptr; + if (platform_adm_) { + platform_adm_->Terminate(); + platform_adm_ = nullptr; + } } } @@ -180,6 +184,7 @@ void AdmProxy::ReleasePlatformAdm() { // Note: We do NOT terminate the Platform ADM - it stays alive until destructor. // This avoids iOS KVO race conditions from re-creating the ADM. if (platform_adm_ref_count_ == 0) { + StopPlatformAudioIOLocked(); SwitchPlayoutModeIfNeeded(); SwitchRecordingAdmIfNeeded(); } @@ -273,6 +278,42 @@ void AdmProxy::SwitchRecordingAdmIfNeeded() { } } +void AdmProxy::StopPlatformAudioIOLocked() { + recording_ = false; + + if (platform_adm_) { + platform_adm_->RegisterAudioCallback(nullptr); + platform_adm_->StopRecording(); + platform_adm_->StopPlayout(); + } +} + +void AdmProxy::StopAudioIOLocked() { + recording_ = false; + playing_ = false; + recording_initialized_ = false; + playout_initialized_ = false; + + if (platform_adm_) { + platform_adm_->RegisterAudioCallback(nullptr); + platform_adm_->StopRecording(); + platform_adm_->StopPlayout(); + } + + if (synthetic_adm_) { + synthetic_adm_->RegisterAudioCallback(nullptr); + synthetic_adm_->StopRecording(); + synthetic_adm_->StopPlayout(); + } + + audio_transport_ = nullptr; +} + +void AdmProxy::ShutdownAudioIO() { + webrtc::MutexLock lock(&mutex_); + StopAudioIOLocked(); +} + // ============================================================================= // AudioDeviceModule Interface Implementation // ============================================================================= @@ -307,6 +348,7 @@ int32_t AdmProxy::Init() { int32_t AdmProxy::Terminate() { webrtc::MutexLock lock(&mutex_); + StopAudioIOLocked(); int32_t result = 0; if (synthetic_adm_) { @@ -554,11 +596,20 @@ int32_t AdmProxy::StopRecording() { webrtc::MutexLock lock(&mutex_); recording_ = false; - auto* adm = recording_adm(); - if (adm) { - return adm->StopRecording(); + int32_t result = 0; + if (platform_adm_) { + const int32_t platform_result = platform_adm_->StopRecording(); + if (result == 0) { + result = platform_result; + } } - return 0; + if (synthetic_adm_) { + const int32_t synthetic_result = synthetic_adm_->StopRecording(); + if (result == 0) { + result = synthetic_result; + } + } + return result; } bool AdmProxy::Recording() const { diff --git a/webrtc-sys/src/peer_connection_factory.cpp b/webrtc-sys/src/peer_connection_factory.cpp index cfb2d0c06..836ad199c 100644 --- a/webrtc-sys/src/peer_connection_factory.cpp +++ b/webrtc-sys/src/peer_connection_factory.cpp @@ -90,6 +90,8 @@ PeerConnectionFactory::PeerConnectionFactory( PeerConnectionFactory::~PeerConnectionFactory() { RTC_LOG(LS_VERBOSE) << "PeerConnectionFactory::~PeerConnectionFactory()"; + shutdown_audio_io(); + peer_factory_ = nullptr; audio_device_ = nullptr; rtc_runtime_->worker_thread()->BlockingCall( @@ -163,6 +165,14 @@ std::shared_ptr PeerConnectionFactory::audio_device() con return audio_device_; } +void PeerConnectionFactory::shutdown_audio_io() const { + rtc_runtime_->worker_thread()->BlockingCall([this] { + if (adm_proxy_) { + adm_proxy_->ShutdownAudioIO(); + } + }); +} + std::shared_ptr create_peer_connection_factory() { return std::make_shared(RtcRuntime::create()); } diff --git a/webrtc-sys/src/peer_connection_factory.rs b/webrtc-sys/src/peer_connection_factory.rs index 16794f6bf..5f5ed1242 100644 --- a/webrtc-sys/src/peer_connection_factory.rs +++ b/webrtc-sys/src/peer_connection_factory.rs @@ -89,6 +89,8 @@ pub mod ffi { fn create_peer_connection_factory() -> SharedPtr; + fn shutdown_audio_io(self: &PeerConnectionFactory); + fn create_peer_connection( self: &PeerConnectionFactory, config: RtcConfiguration, From 44e17fed988e15373bd3a2a10f5298642b64a2ec Mon Sep 17 00:00:00 2001 From: Alan George Date: Fri, 26 Jun 2026 17:19:29 -0600 Subject: [PATCH 2/3] Update changeset for platform audio stability work Rename the changeset and document the full ADM teardown + FFI dispose fix. Bump livekit and libwebrtc as minor for the new shutdown_audio_io lifecycle hooks; keep livekit-ffi and webrtc-sys as patch. Co-authored-by: Cursor --- .changeset/ffi-handle-cleanup.md | 5 ----- .changeset/platform-audio-stability.md | 13 +++++++++++++ 2 files changed, 13 insertions(+), 5 deletions(-) delete mode 100644 .changeset/ffi-handle-cleanup.md create mode 100644 .changeset/platform-audio-stability.md diff --git a/.changeset/ffi-handle-cleanup.md b/.changeset/ffi-handle-cleanup.md deleted file mode 100644 index 7ec047614..000000000 --- a/.changeset/ffi-handle-cleanup.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -livekit-ffi: patch ---- - -Clear remaining FFI handles during dispose so platform audio resources are released across repeated initialize/shutdown cycles. diff --git a/.changeset/platform-audio-stability.md b/.changeset/platform-audio-stability.md new file mode 100644 index 000000000..26e4c23df --- /dev/null +++ b/.changeset/platform-audio-stability.md @@ -0,0 +1,13 @@ +--- +livekit: minor +libwebrtc: minor +livekit-ffi: patch +webrtc-sys: patch +--- + +Fix platform ADM teardown races on macOS and release FFI handles on dispose. + +- Clear remaining FFI handles during `FfiServer::dispose` so native resources are released across repeated initialize/shutdown cycles. +- Stop and detach platform/synthetic audio I/O before peer connection factory teardown, preventing `CaptureWorkerThread` from delivering into destroyed transports. +- Close rooms before dropping FFI track handles and stop platform capture when releasing the platform ADM reference. +- Add `LkRuntime::shutdown_audio_io()` and `PeerConnectionFactoryExt::shutdown_audio_io()` for explicit audio I/O shutdown during runtime teardown. From d3bb14531f88bfc7f4c43cdcd9a6f68911bfd9f3 Mon Sep 17 00:00:00 2001 From: Alan George Date: Tue, 30 Jun 2026 12:39:17 -0600 Subject: [PATCH 3/3] Address PR review feedback on AdmProxy audio shutdown Rename StopAudioIOLocked/StopPlatformAudioIOLocked to StopAudioIO and StopPlatformAudioIO, collapse ShutdownAudioIO into public StopAudioIO, and document why ADM instances are retained after stopping I/O. Co-authored-by: Cursor --- webrtc-sys/include/livekit/adm_proxy.h | 8 ++--- webrtc-sys/src/adm_proxy.cpp | 41 +++++++++++----------- webrtc-sys/src/peer_connection_factory.cpp | 2 +- 3 files changed, 23 insertions(+), 28 deletions(-) diff --git a/webrtc-sys/include/livekit/adm_proxy.h b/webrtc-sys/include/livekit/adm_proxy.h index 27969f4ec..6e0fe3c51 100644 --- a/webrtc-sys/include/livekit/adm_proxy.h +++ b/webrtc-sys/include/livekit/adm_proxy.h @@ -98,7 +98,7 @@ class AdmProxy : public webrtc::AudioDeviceModule { /// /// Call before tearing down the peer connection factory so capture worker /// threads cannot deliver frames into transports that are being destroyed. - void ShutdownAudioIO(); + void StopAudioIO(); // =========================================================================== // Recording/Playout Control @@ -234,13 +234,9 @@ class AdmProxy : public webrtc::AudioDeviceModule { // Must be called with mutex_ held. void SwitchRecordingAdmIfNeeded(); - // Stops active capture/playout and detaches audio callbacks from both ADMs. - // Must be called with mutex_ held. - void StopAudioIOLocked(); - // Stops platform capture/playout and detaches its callback without touching // synthetic mode. Must be called with mutex_ held. - void StopPlatformAudioIOLocked(); + void StopPlatformAudioIO(); #if defined(__ANDROID__) // Lazily creates the Platform ADM on Android. diff --git a/webrtc-sys/src/adm_proxy.cpp b/webrtc-sys/src/adm_proxy.cpp index 7ae24b457..29b75bfc8 100644 --- a/webrtc-sys/src/adm_proxy.cpp +++ b/webrtc-sys/src/adm_proxy.cpp @@ -71,18 +71,17 @@ AdmProxy::AdmProxy(const webrtc::Environment& env, webrtc::Thread* worker_thread AdmProxy::~AdmProxy() { RTC_LOG(LS_VERBOSE) << "AdmProxy::~AdmProxy()"; - { - webrtc::MutexLock lock(&mutex_); - StopAudioIOLocked(); - if (synthetic_adm_) { - synthetic_adm_->Terminate(); - synthetic_adm_ = nullptr; - } + StopAudioIO(); - if (platform_adm_) { - platform_adm_->Terminate(); - platform_adm_ = nullptr; - } + webrtc::MutexLock lock(&mutex_); + if (synthetic_adm_) { + synthetic_adm_->Terminate(); + synthetic_adm_ = nullptr; + } + + if (platform_adm_) { + platform_adm_->Terminate(); + platform_adm_ = nullptr; } } @@ -184,7 +183,7 @@ void AdmProxy::ReleasePlatformAdm() { // Note: We do NOT terminate the Platform ADM - it stays alive until destructor. // This avoids iOS KVO race conditions from re-creating the ADM. if (platform_adm_ref_count_ == 0) { - StopPlatformAudioIOLocked(); + StopPlatformAudioIO(); SwitchPlayoutModeIfNeeded(); SwitchRecordingAdmIfNeeded(); } @@ -278,17 +277,21 @@ void AdmProxy::SwitchRecordingAdmIfNeeded() { } } -void AdmProxy::StopPlatformAudioIOLocked() { +void AdmProxy::StopPlatformAudioIO() { recording_ = false; if (platform_adm_) { platform_adm_->RegisterAudioCallback(nullptr); platform_adm_->StopRecording(); platform_adm_->StopPlayout(); + // platform_adm_ is kept alive for re-acquire and iOS compatibility; see + // ReleasePlatformAdm(). } } -void AdmProxy::StopAudioIOLocked() { +void AdmProxy::StopAudioIO() { + webrtc::MutexLock lock(&mutex_); + recording_ = false; playing_ = false; recording_initialized_ = false; @@ -304,16 +307,12 @@ void AdmProxy::StopAudioIOLocked() { synthetic_adm_->RegisterAudioCallback(nullptr); synthetic_adm_->StopRecording(); synthetic_adm_->StopPlayout(); + // synthetic_adm_ is kept alive until ~AdmProxy() / Terminate(). } audio_transport_ = nullptr; } -void AdmProxy::ShutdownAudioIO() { - webrtc::MutexLock lock(&mutex_); - StopAudioIOLocked(); -} - // ============================================================================= // AudioDeviceModule Interface Implementation // ============================================================================= @@ -347,9 +346,9 @@ int32_t AdmProxy::Init() { } int32_t AdmProxy::Terminate() { - webrtc::MutexLock lock(&mutex_); - StopAudioIOLocked(); + StopAudioIO(); + webrtc::MutexLock lock(&mutex_); int32_t result = 0; if (synthetic_adm_) { result = synthetic_adm_->Terminate(); diff --git a/webrtc-sys/src/peer_connection_factory.cpp b/webrtc-sys/src/peer_connection_factory.cpp index 836ad199c..b34ed7cdf 100644 --- a/webrtc-sys/src/peer_connection_factory.cpp +++ b/webrtc-sys/src/peer_connection_factory.cpp @@ -168,7 +168,7 @@ std::shared_ptr PeerConnectionFactory::audio_device() con void PeerConnectionFactory::shutdown_audio_io() const { rtc_runtime_->worker_thread()->BlockingCall([this] { if (adm_proxy_) { - adm_proxy_->ShutdownAudioIO(); + adm_proxy_->StopAudioIO(); } }); }