From b933f90ba55d5d27fd154adf2a828917737fd46c Mon Sep 17 00:00:00 2001 From: Alan George Date: Fri, 26 Jun 2026 16:58:46 -0600 Subject: [PATCH] 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 7e321a26a..3e7d4a802 100644 --- a/livekit-ffi/src/server/room.rs +++ b/livekit-ffi/src/server/room.rs @@ -356,6 +356,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) { @@ -364,8 +368,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,