From 66c25a2ea82338028149ab0bf2c8329b8d753af6 Mon Sep 17 00:00:00 2001 From: Santhosh Vaiyapuri Date: Fri, 25 Sep 2026 15:06:31 +0200 Subject: [PATCH] fix(ios): harden peer connection disposal against queued callbacks iOS counterpart of #70. Factory teardown is now two-phase (close PCs, then dispose on a later worker-queue turn so close-triggered callbacks drain first), createCallFactory/disposeCallFactory are serialized behind an in-flight teardown, and SDP/ICE completions plus ICE-candidate/data-channel/receiver delegate callbacks are skipped (or rejected with E_PC_DISPOSED) once their PeerConnection is disposed. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../WebRTCModule+RTCPeerConnection.m | 39 ++++++-- ios/RCTWebRTC/WebRTCModule.m | 96 +++++++++++++++---- 2 files changed, 107 insertions(+), 28 deletions(-) diff --git a/ios/RCTWebRTC/WebRTCModule+RTCPeerConnection.m b/ios/RCTWebRTC/WebRTCModule+RTCPeerConnection.m index 167c9e94f..4e5941350 100644 --- a/ios/RCTWebRTC/WebRTCModule+RTCPeerConnection.m +++ b/ios/RCTWebRTC/WebRTCModule+RTCPeerConnection.m @@ -78,6 +78,27 @@ - (void)setWebRTCModule:(id)webRTCModule { static NSMutableDictionary *gCertificates = nil; +/* + * Runs `block` on the worker queue only if `pc` is still the live PeerConnection for its tag; + * otherwise rejects (when `reject` is given) or skips. libwebrtc callbacks can be queued behind + * peerConnectionDispose, and running them afterwards would mutate a disposed PeerConnection or + * resolve a promise from it. + */ +static void RunOnLivePeerConnection(WebRTCModule *module, + RTCPeerConnection *pc, + RCTPromiseRejectBlock _Nullable reject, + dispatch_block_t block) { + dispatch_async(module.workerQueue, ^{ + if (module.peerConnections[pc.reactTag] != pc) { + if (reject) { + reject(@"E_PC_DISPOSED", @"PeerConnection disposed", nil); + } + return; + } + block(); + }); +} + @implementation WebRTCModule (RTCPeerConnection) + (void)initialize { @@ -160,7 +181,7 @@ + (RTCCertificate *)getCertificate:(NSString *)certId { } RTCCreateSessionDescriptionCompletionHandler handler = ^(RTCSessionDescription *desc, NSError *error) { - dispatch_async(self.workerQueue, ^{ + RunOnLivePeerConnection(self, peerConnection, reject, ^{ if (error) { reject(@"E_OPERATION_ERROR", error.localizedDescription, nil); } else { @@ -203,7 +224,7 @@ + (RTCCertificate *)getCertificate:(NSString *)certId { optionalConstraints:nil]; RTCCreateSessionDescriptionCompletionHandler handler = ^(RTCSessionDescription *desc, NSError *error) { - dispatch_async(self.workerQueue, ^{ + RunOnLivePeerConnection(self, peerConnection, reject, ^{ if (error) { reject(@"E_OPERATION_ERROR", error.localizedDescription, nil); } else { @@ -232,7 +253,7 @@ + (RTCCertificate *)getCertificate:(NSString *)certId { } RTCSetSessionDescriptionCompletionHandler handler = ^(NSError *error) { - dispatch_async(self.workerQueue, ^{ + RunOnLivePeerConnection(self, peerConnection, reject, ^{ if (error) { reject(@"E_OPERATION_ERROR", error.localizedDescription, nil); } else { @@ -276,7 +297,7 @@ + (RTCCertificate *)getCertificate:(NSString *)certId { } RTCSetSessionDescriptionCompletionHandler handler = ^(NSError *error) { - dispatch_async(self.workerQueue, ^{ + RunOnLivePeerConnection(self, peerConnection, reject, ^{ if (error) { reject(@"E_OPERATION_ERROR", error.localizedDescription, nil); } else { @@ -323,7 +344,7 @@ + (RTCCertificate *)getCertificate:(NSString *)certId { } id handler = ^(NSError *error) { - dispatch_async(self.workerQueue, ^{ + RunOnLivePeerConnection(self, peerConnection, reject, ^{ if (error) { reject(@"E_OPERATION_ERROR", @"addIceCandidate failed", error); } else { @@ -870,7 +891,7 @@ - (void)peerConnection:(RTCPeerConnection *)peerConnection didChangeIceGathering } - (void)peerConnection:(RTCPeerConnection *)peerConnection didGenerateIceCandidate:(RTCIceCandidate *)candidate { - dispatch_async(self.workerQueue, ^{ + RunOnLivePeerConnection(self, peerConnection, nil, ^{ id newSdp = @{}; // Can happen when doing a rollback. if (peerConnection.localDescription) { @@ -894,7 +915,7 @@ - (void)peerConnection:(RTCPeerConnection *)peerConnection didGenerateIceCandida } - (void)peerConnection:(RTCPeerConnection *)peerConnection didOpenDataChannel:(RTCDataChannel *)dataChannel { - dispatch_async(self.workerQueue, ^{ + RunOnLivePeerConnection(self, peerConnection, nil, ^{ NSString *reactTag = [[NSUUID UUID] UUIDString]; DataChannelWrapper *dcw = [[DataChannelWrapper alloc] initWithChannel:dataChannel reactTag:reactTag]; dcw.pcId = peerConnection.reactTag; @@ -922,7 +943,7 @@ - (void)peerConnection:(RTCPeerConnection *)peerConnection didOpenDataChannel:(R - (void)peerConnection:(RTC_OBJC_TYPE(RTCPeerConnection) *)peerConnection didAddReceiver:(RTC_OBJC_TYPE(RTCRtpReceiver) *)rtpReceiver streams:(NSArray *)mediaStreams { - dispatch_async(self.workerQueue, ^{ + RunOnLivePeerConnection(self, peerConnection, nil, ^{ RTCRtpTransceiver *transceiver = nil; for (RTCRtpTransceiver *t in peerConnection.transceivers) { if ([rtpReceiver.receiverId isEqual:t.receiver.receiverId]) { @@ -997,7 +1018,7 @@ - (void)peerConnection:(RTC_OBJC_TYPE(RTCPeerConnection) *)peerConnection - (void)peerConnection:(RTC_OBJC_TYPE(RTCPeerConnection) *)peerConnection didRemoveReceiver:(RTC_OBJC_TYPE(RTCRtpReceiver) *)rtpReceiver { - dispatch_async(self.workerQueue, ^{ + RunOnLivePeerConnection(self, peerConnection, nil, ^{ // Tear down track adapters so a subsequent didAddReceiver with the // same trackId (SFU participant rejoin) creates a fresh adapter on // the new RTCMediaStreamTrack object. Without this, the old renderer diff --git a/ios/RCTWebRTC/WebRTCModule.m b/ios/RCTWebRTC/WebRTCModule.m index ffde484d5..def6a9660 100644 --- a/ios/RCTWebRTC/WebRTCModule.m +++ b/ios/RCTWebRTC/WebRTCModule.m @@ -28,6 +28,10 @@ @interface WebRTCModule () @property(nonatomic, strong) AudioDeviceModuleObserver *rtcAudioDeviceModuleObserver; +// Accessed only on the worker queue. Lifecycle requests wait for both teardown phases. +@property(nonatomic, strong) NSMutableArray *pendingFactoryOperations; +@property(nonatomic, assign) BOOL factoryDisposalPending; + @end @implementation WebRTCModule @@ -120,6 +124,7 @@ - (instancetype)init { _peerConnections = [NSMutableDictionary new]; _localStreams = [NSMutableDictionary new]; _localTracks = [NSMutableDictionary new]; + _pendingFactoryOperations = [NSMutableArray new]; dispatch_queue_attr_t attributes = dispatch_queue_attr_make_with_qos_class(DISPATCH_QUEUE_SERIAL, QOS_CLASS_USER_INITIATED, -1); @@ -201,25 +206,53 @@ - (dispatch_queue_t)methodQueue { : (RCTPromiseRejectBlock)reject) { BOOL bypassVoiceProcessing = [options[@"bypassVoiceProcessing"] boolValue]; - // This makes default factory being disposed in a proper sequence. - if ([self.factoryRegistry isBareForkDefaultLive]) { - RCTLogInfo(@"createCallFactory(): tearing down stale bare-fork default (ordered) before " - "creating the call factory"); - [self disposeCurrentFactoryOrdered]; - } - - PeerConnectionFactoryProvider *factory = [self.factoryRegistry create:bypassVoiceProcessing]; - if (factory == nil) { - reject(@"E_FACTORY_CREATE", @"Failed to create call factory: registry is disposed", nil); - return; - } - resolve(nil); + [self runFactoryOperation:^{ + void (^create)(void) = ^{ + PeerConnectionFactoryProvider *factory = [self.factoryRegistry create:bypassVoiceProcessing]; + if (factory == nil) { + reject(@"E_FACTORY_CREATE", @"Failed to create call factory: registry is disposed", nil); + return; + } + resolve(nil); + }; + + // Tear a stale bare-fork default down in order first. The teardown is two-phase, so the new + // factory must be built from the completion — building it inline would create it while the + // old factory's PeerConnections were still alive. + if ([self.factoryRegistry isBareForkDefaultLive]) { + RCTLogInfo(@"createCallFactory(): tearing down stale bare-fork default (ordered) before " + "creating the call factory"); + [self disposeCurrentFactoryOrdered:^(BOOL disposed) { + create(); + }]; + } else { + create(); + } + }]; } RCT_EXPORT_METHOD(disposeCallFactory : (RCTPromiseResolveBlock)resolve rejecter : (RCTPromiseRejectBlock)reject) { - resolve(@([self disposeCurrentFactoryOrdered])); + [self runFactoryOperation:^{ + [self disposeCurrentFactoryOrdered:^(BOOL disposed) { + resolve(@(disposed)); + }]; + }]; +} + +// Must be called on the worker queue (the module's methodQueue). +- (void)runFactoryOperation:(dispatch_block_t)operation { + [self.pendingFactoryOperations addObject:operation]; + [self drainFactoryOperations]; +} + +- (void)drainFactoryOperations { + while (!self.factoryDisposalPending && self.pendingFactoryOperations.count > 0) { + dispatch_block_t operation = self.pendingFactoryOperations.firstObject; + [self.pendingFactoryOperations removeObjectAtIndex:0]; + operation(); + } } /** @@ -227,16 +260,43 @@ - (dispatch_queue_t)methodQueue { * streams → video-effects processor → factory + ADM. Everything is ARC-refcounted, so the factory * is freed only when its LAST reference drops — every dependent that strong-refs it (PCs, tracks, * streams, and the videoEffectProcessor associated object) must be released first or the factory - * leaks. No-op unless this is the last reference; returns whether it disposed the factory. + * leaks. No-op unless this is the last reference; `onDisposed` receives whether the factory was + * actually disposed. + * + * Split across two worker-queue turns: phase 1 only closes the PeerConnections, phase 2 disposes + * them and everything else. -[RTCPeerConnection close] is synchronous and its delegate callbacks + * dispatch_async onto the worker queue before it returns, so phase 2 (queued after them) runs + * only once that backlog has drained against still-registered PeerConnections. */ -- (BOOL)disposeCurrentFactoryOrdered { +- (void)disposeCurrentFactoryOrdered:(void (^)(BOOL disposed))onDisposed { if (![self.factoryRegistry releaseReference]) { - return NO; + onDisposed(NO); + return; } + self.factoryDisposalPending = YES; for (NSNumber *pcId in [self.peerConnections.allKeys copy]) { @try { [self peerConnectionClose:pcId]; + } @catch (NSException *e) { + RCTLogWarn(@"disposeCurrentFactoryOrdered(): error closing pc %@: %@", pcId, e.reason); + } + } + + dispatch_async(self.workerQueue, ^{ + @try { + [self disposeCurrentFactoryDependents]; + onDisposed([self.factoryRegistry disposeCurrent]); + } @finally { + self.factoryDisposalPending = NO; + [self drainFactoryOperations]; + } + }); +} + +- (void)disposeCurrentFactoryDependents { + for (NSNumber *pcId in [self.peerConnections.allKeys copy]) { + @try { [self peerConnectionDispose:pcId]; } @catch (NSException *e) { RCTLogWarn(@"disposeCurrentFactoryOrdered(): error disposing pc %@: %@", pcId, e.reason); @@ -267,8 +327,6 @@ - (BOOL)disposeCurrentFactoryOrdered { } self.videoEffectProcessor = nil; - - return [self.factoryRegistry disposeCurrent]; } - (NSArray *)supportedEvents {