From ef43aafcf5953845b71840e14c94942d60c541c1 Mon Sep 17 00:00:00 2001 From: Yura Yaroshevich Date: Mon, 9 Jul 2018 19:16:32 +0300 Subject: [PATCH] Fixed crash when PCF is destroyed before RTCRtpSender in ObjC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:9231 Change-Id: I3b90400bf619938817d7a04a7a1130ba86ad65df Reviewed-on: https://webrtc-review.googlesource.com/87623 Reviewed-by: Kári Helgason Commit-Queue: Kári Helgason Cr-Commit-Position: refs/heads/master@{#23896} --- .../PeerConnection/RTCPeerConnection.mm | 9 +++--- .../PeerConnection/RTCRtpSender+Private.h | 7 +++-- .../Classes/PeerConnection/RTCRtpSender.mm | 7 +++-- .../PeerConnection/RTCRtpTransceiver.mm | 3 +- .../RTCPeerConnectionFactory_xctest.m | 28 +++++++++++++++++++ 5 files changed, 45 insertions(+), 9 deletions(-) diff --git a/sdk/objc/Framework/Classes/PeerConnection/RTCPeerConnection.mm b/sdk/objc/Framework/Classes/PeerConnection/RTCPeerConnection.mm index f6b2343eb7..c61e0e15f1 100644 --- a/sdk/objc/Framework/Classes/PeerConnection/RTCPeerConnection.mm +++ b/sdk/objc/Framework/Classes/PeerConnection/RTCPeerConnection.mm @@ -386,7 +386,8 @@ void PeerConnectionDelegateAdapter::OnAddTrack( RTCLogError(@"Failed to add track %@: %s", track, nativeSenderOrError.error().message()); return nil; } - return [[RTCRtpSender alloc] initWithNativeRtpSender:nativeSenderOrError.MoveValue()]; + return [[RTCRtpSender alloc] initWithFactory:self.factory + nativeRtpSender:nativeSenderOrError.MoveValue()]; } - (BOOL)removeTrack:(RTCRtpSender *)sender { @@ -522,8 +523,8 @@ void PeerConnectionDelegateAdapter::OnAddTrack( rtc::scoped_refptr nativeSender( _peerConnection->CreateSender(nativeKind, nativeStreamId)); return nativeSender ? - [[RTCRtpSender alloc] initWithNativeRtpSender:nativeSender] - : nil; + [[RTCRtpSender alloc] initWithFactory:self.factory nativeRtpSender:nativeSender] : + nil; } - (NSArray *)senders { @@ -532,7 +533,7 @@ void PeerConnectionDelegateAdapter::OnAddTrack( NSMutableArray *senders = [[NSMutableArray alloc] init]; for (const auto &nativeSender : nativeSenders) { RTCRtpSender *sender = - [[RTCRtpSender alloc] initWithNativeRtpSender:nativeSender]; + [[RTCRtpSender alloc] initWithFactory:self.factory nativeRtpSender:nativeSender]; [senders addObject:sender]; } return senders; diff --git a/sdk/objc/Framework/Classes/PeerConnection/RTCRtpSender+Private.h b/sdk/objc/Framework/Classes/PeerConnection/RTCRtpSender+Private.h index 7164bca56f..5b671ae7c7 100644 --- a/sdk/objc/Framework/Classes/PeerConnection/RTCRtpSender+Private.h +++ b/sdk/objc/Framework/Classes/PeerConnection/RTCRtpSender+Private.h @@ -14,13 +14,16 @@ NS_ASSUME_NONNULL_BEGIN +@class RTCPeerConnectionFactory; + @interface RTCRtpSender () @property(nonatomic, readonly) rtc::scoped_refptr nativeRtpSender; /** Initialize an RTCRtpSender with a native RtpSenderInterface. */ -- (instancetype)initWithNativeRtpSender: - (rtc::scoped_refptr)nativeRtpSender NS_DESIGNATED_INITIALIZER; +- (instancetype)initWithFactory:(RTCPeerConnectionFactory*)factory + nativeRtpSender:(rtc::scoped_refptr)nativeRtpSender + NS_DESIGNATED_INITIALIZER; @end diff --git a/sdk/objc/Framework/Classes/PeerConnection/RTCRtpSender.mm b/sdk/objc/Framework/Classes/PeerConnection/RTCRtpSender.mm index e980adec40..1df7ae5a21 100644 --- a/sdk/objc/Framework/Classes/PeerConnection/RTCRtpSender.mm +++ b/sdk/objc/Framework/Classes/PeerConnection/RTCRtpSender.mm @@ -19,6 +19,7 @@ #include "api/mediastreaminterface.h" @implementation RTCRtpSender { + RTCPeerConnectionFactory *_factory; rtc::scoped_refptr _nativeRtpSender; } @@ -84,10 +85,12 @@ return _nativeRtpSender; } -- (instancetype)initWithNativeRtpSender: - (rtc::scoped_refptr)nativeRtpSender { +- (instancetype)initWithFactory:(RTCPeerConnectionFactory *)factory + nativeRtpSender:(rtc::scoped_refptr)nativeRtpSender { + NSParameterAssert(factory); NSParameterAssert(nativeRtpSender); if (self = [super init]) { + _factory = factory; _nativeRtpSender = nativeRtpSender; rtc::scoped_refptr nativeDtmfSender( _nativeRtpSender->GetDtmfSender()); diff --git a/sdk/objc/Framework/Classes/PeerConnection/RTCRtpTransceiver.mm b/sdk/objc/Framework/Classes/PeerConnection/RTCRtpTransceiver.mm index 780fcd5fe7..ea4b3a9df8 100644 --- a/sdk/objc/Framework/Classes/PeerConnection/RTCRtpTransceiver.mm +++ b/sdk/objc/Framework/Classes/PeerConnection/RTCRtpTransceiver.mm @@ -129,7 +129,8 @@ if (self = [super init]) { _factory = factory; _nativeRtpTransceiver = nativeRtpTransceiver; - _sender = [[RTCRtpSender alloc] initWithNativeRtpSender:nativeRtpTransceiver->sender()]; + _sender = [[RTCRtpSender alloc] initWithFactory:_factory + nativeRtpSender:nativeRtpTransceiver->sender()]; _receiver = [[RTCRtpReceiver alloc] initWithNativeRtpReceiver:nativeRtpTransceiver->receiver()]; RTCLogInfo(@"RTCRtpTransceiver(%p): created transceiver: %@", self, self.description); } diff --git a/sdk/objc/Framework/UnitTests/RTCPeerConnectionFactory_xctest.m b/sdk/objc/Framework/UnitTests/RTCPeerConnectionFactory_xctest.m index 4e679fab91..b5f3b98b1c 100644 --- a/sdk/objc/Framework/UnitTests/RTCPeerConnectionFactory_xctest.m +++ b/sdk/objc/Framework/UnitTests/RTCPeerConnectionFactory_xctest.m @@ -12,8 +12,10 @@ #import #import #import +#import #import #import +#import #import #import @@ -119,4 +121,30 @@ XCTAssertTrue(true, "Expect test does not crash"); } +- (void)testRTCRtpSenderLifetime { + @autoreleasepool { + RTCConfiguration *config = [[RTCConfiguration alloc] init]; + RTCMediaConstraints *contraints = + [[RTCMediaConstraints alloc] initWithMandatoryConstraints:@{} optionalConstraints:nil]; + + RTCPeerConnectionFactory *factory; + RTCPeerConnection *peerConnection; + RTCRtpSender *sender; + + @autoreleasepool { + factory = [[RTCPeerConnectionFactory alloc] init]; + peerConnection = + [factory peerConnectionWithConfiguration:config constraints:contraints delegate:nil]; + sender = [peerConnection senderWithKind:kRTCMediaStreamTrackKindVideo streamId:@"stream"]; + XCTAssertTrue(sender != nil); + [peerConnection close]; + peerConnection = nil; + factory = nil; + } + sender = nil; + } + + XCTAssertTrue(true, "Expect test does not crash"); +} + @end