From 7a16c54571c563970cfa5970d7ef4476bd8ef13d Mon Sep 17 00:00:00 2001 From: Yura Yaroshevich Date: Wed, 11 Jul 2018 12:55:04 +0300 Subject: [PATCH] Fixed crash when PCF is destroyed before RTCRtpReceiver in ObjC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:9231 Change-Id: Ic532b7661bb8765f0fc2309d2ad530f664ccfd14 Reviewed-on: https://webrtc-review.googlesource.com/87840 Reviewed-by: Kári Helgason Commit-Queue: Kári Helgason Cr-Commit-Position: refs/heads/master@{#23931} --- .../PeerConnection/RTCPeerConnection.mm | 5 +- .../PeerConnection/RTCRtpReceiver+Private.h | 6 +- .../Classes/PeerConnection/RTCRtpReceiver.mm | 7 +- .../PeerConnection/RTCRtpTransceiver.mm | 3 +- .../RTCPeerConnectionFactory_xctest.m | 117 ++++++++++++++++-- 5 files changed, 121 insertions(+), 17 deletions(-) diff --git a/sdk/objc/Framework/Classes/PeerConnection/RTCPeerConnection.mm b/sdk/objc/Framework/Classes/PeerConnection/RTCPeerConnection.mm index c61e0e15f1..947bba6e1c 100644 --- a/sdk/objc/Framework/Classes/PeerConnection/RTCPeerConnection.mm +++ b/sdk/objc/Framework/Classes/PeerConnection/RTCPeerConnection.mm @@ -231,7 +231,8 @@ void PeerConnectionDelegateAdapter::OnAddTrack( nativeMediaStream:nativeStream]; [mediaStreams addObject:mediaStream]; } - RTCRtpReceiver *rtpReceiver = [[RTCRtpReceiver alloc] initWithNativeRtpReceiver:receiver]; + RTCRtpReceiver *rtpReceiver = + [[RTCRtpReceiver alloc] initWithFactory:peer_connection.factory nativeRtpReceiver:receiver]; [peer_connection.delegate peerConnection:peer_connection didAddReceiver:rtpReceiver @@ -545,7 +546,7 @@ void PeerConnectionDelegateAdapter::OnAddTrack( NSMutableArray *receivers = [[NSMutableArray alloc] init]; for (const auto &nativeReceiver : nativeReceivers) { RTCRtpReceiver *receiver = - [[RTCRtpReceiver alloc] initWithNativeRtpReceiver:nativeReceiver]; + [[RTCRtpReceiver alloc] initWithFactory:self.factory nativeRtpReceiver:nativeReceiver]; [receivers addObject:receiver]; } return receivers; diff --git a/sdk/objc/Framework/Classes/PeerConnection/RTCRtpReceiver+Private.h b/sdk/objc/Framework/Classes/PeerConnection/RTCRtpReceiver+Private.h index 73f68f4885..6ec49bede4 100644 --- a/sdk/objc/Framework/Classes/PeerConnection/RTCRtpReceiver+Private.h +++ b/sdk/objc/Framework/Classes/PeerConnection/RTCRtpReceiver+Private.h @@ -14,6 +14,8 @@ NS_ASSUME_NONNULL_BEGIN +@class RTCPeerConnectionFactory; + namespace webrtc { class RtpReceiverDelegateAdapter : public RtpReceiverObserverInterface { @@ -33,8 +35,8 @@ class RtpReceiverDelegateAdapter : public RtpReceiverObserverInterface { @property(nonatomic, readonly) rtc::scoped_refptr nativeRtpReceiver; /** Initialize an RTCRtpReceiver with a native RtpReceiverInterface. */ -- (instancetype)initWithNativeRtpReceiver: - (rtc::scoped_refptr)nativeRtpReceiver +- (instancetype)initWithFactory:(RTCPeerConnectionFactory*)factory + nativeRtpReceiver:(rtc::scoped_refptr)nativeRtpReceiver NS_DESIGNATED_INITIALIZER; + (RTCRtpMediaType)mediaTypeForNativeMediaType:(cricket::MediaType)nativeMediaType; diff --git a/sdk/objc/Framework/Classes/PeerConnection/RTCRtpReceiver.mm b/sdk/objc/Framework/Classes/PeerConnection/RTCRtpReceiver.mm index b1f5be9c8a..1342c166c0 100644 --- a/sdk/objc/Framework/Classes/PeerConnection/RTCRtpReceiver.mm +++ b/sdk/objc/Framework/Classes/PeerConnection/RTCRtpReceiver.mm @@ -36,6 +36,7 @@ void RtpReceiverDelegateAdapter::OnFirstPacketReceived( } // namespace webrtc @implementation RTCRtpReceiver { + RTCPeerConnectionFactory *_factory; rtc::scoped_refptr _nativeRtpReceiver; std::unique_ptr _observer; } @@ -102,9 +103,11 @@ void RtpReceiverDelegateAdapter::OnFirstPacketReceived( return _nativeRtpReceiver; } -- (instancetype)initWithNativeRtpReceiver: - (rtc::scoped_refptr)nativeRtpReceiver { +- (instancetype)initWithFactory:(RTCPeerConnectionFactory *)factory + nativeRtpReceiver: + (rtc::scoped_refptr)nativeRtpReceiver { if (self = [super init]) { + _factory = factory; _nativeRtpReceiver = nativeRtpReceiver; RTCLogInfo( @"RTCRtpReceiver(%p): created receiver: %@", self, self.description); diff --git a/sdk/objc/Framework/Classes/PeerConnection/RTCRtpTransceiver.mm b/sdk/objc/Framework/Classes/PeerConnection/RTCRtpTransceiver.mm index ea4b3a9df8..cf39d5beff 100644 --- a/sdk/objc/Framework/Classes/PeerConnection/RTCRtpTransceiver.mm +++ b/sdk/objc/Framework/Classes/PeerConnection/RTCRtpTransceiver.mm @@ -131,7 +131,8 @@ _nativeRtpTransceiver = nativeRtpTransceiver; _sender = [[RTCRtpSender alloc] initWithFactory:_factory nativeRtpSender:nativeRtpTransceiver->sender()]; - _receiver = [[RTCRtpReceiver alloc] initWithNativeRtpReceiver:nativeRtpTransceiver->receiver()]; + _receiver = [[RTCRtpReceiver alloc] initWithFactory:_factory + nativeRtpReceiver:nativeRtpTransceiver->receiver()]; RTCLogInfo(@"RTCRtpTransceiver(%p): created transceiver: %@", self, self.description); } return self; diff --git a/sdk/objc/Framework/UnitTests/RTCPeerConnectionFactory_xctest.m b/sdk/objc/Framework/UnitTests/RTCPeerConnectionFactory_xctest.m index b5f3b98b1c..eecc289dc9 100644 --- a/sdk/objc/Framework/UnitTests/RTCPeerConnectionFactory_xctest.m +++ b/sdk/objc/Framework/UnitTests/RTCPeerConnectionFactory_xctest.m @@ -15,13 +15,13 @@ #import #import #import +#import #import #import #import @interface RTCPeerConnectionFactoryTests : XCTestCase -- (void)testPeerConnectionLifetime; @end @implementation RTCPeerConnectionFactoryTests @@ -30,7 +30,7 @@ @autoreleasepool { RTCConfiguration *config = [[RTCConfiguration alloc] init]; - RTCMediaConstraints *contraints = + RTCMediaConstraints *constraints = [[RTCMediaConstraints alloc] initWithMandatoryConstraints:@{} optionalConstraints:nil]; RTCPeerConnectionFactory *factory; @@ -39,7 +39,7 @@ @autoreleasepool { factory = [[RTCPeerConnectionFactory alloc] init]; peerConnection = - [factory peerConnectionWithConfiguration:config constraints:contraints delegate:nil]; + [factory peerConnectionWithConfiguration:config constraints:constraints delegate:nil]; [peerConnection close]; factory = nil; } @@ -68,7 +68,7 @@ - (void)testDataChannelLifetime { @autoreleasepool { RTCConfiguration *config = [[RTCConfiguration alloc] init]; - RTCMediaConstraints *contraints = + RTCMediaConstraints *constraints = [[RTCMediaConstraints alloc] initWithMandatoryConstraints:@{} optionalConstraints:nil]; RTCDataChannelConfiguration *dataChannelConfig = [[RTCDataChannelConfiguration alloc] init]; @@ -79,10 +79,10 @@ @autoreleasepool { factory = [[RTCPeerConnectionFactory alloc] init]; peerConnection = - [factory peerConnectionWithConfiguration:config constraints:contraints delegate:nil]; + [factory peerConnectionWithConfiguration:config constraints:constraints delegate:nil]; dataChannel = [peerConnection dataChannelForLabel:@"test_channel" configuration:dataChannelConfig]; - XCTAssertTrue(dataChannel != nil); + XCTAssertNotNil(dataChannel); [peerConnection close]; peerConnection = nil; factory = nil; @@ -110,7 +110,7 @@ peerConnection = [factory peerConnectionWithConfiguration:config constraints:contraints delegate:nil]; tranceiver = [peerConnection addTransceiverOfType:RTCRtpMediaTypeAudio init:init]; - XCTAssertTrue(tranceiver != nil); + XCTAssertNotNil(tranceiver); [peerConnection close]; peerConnection = nil; factory = nil; @@ -124,7 +124,7 @@ - (void)testRTCRtpSenderLifetime { @autoreleasepool { RTCConfiguration *config = [[RTCConfiguration alloc] init]; - RTCMediaConstraints *contraints = + RTCMediaConstraints *constraints = [[RTCMediaConstraints alloc] initWithMandatoryConstraints:@{} optionalConstraints:nil]; RTCPeerConnectionFactory *factory; @@ -134,9 +134,9 @@ @autoreleasepool { factory = [[RTCPeerConnectionFactory alloc] init]; peerConnection = - [factory peerConnectionWithConfiguration:config constraints:contraints delegate:nil]; + [factory peerConnectionWithConfiguration:config constraints:constraints delegate:nil]; sender = [peerConnection senderWithKind:kRTCMediaStreamTrackKindVideo streamId:@"stream"]; - XCTAssertTrue(sender != nil); + XCTAssertNotNil(sender); [peerConnection close]; peerConnection = nil; factory = nil; @@ -147,4 +147,101 @@ XCTAssertTrue(true, "Expect test does not crash"); } +- (void)testRTCRtpReceiverLifetime { + @autoreleasepool { + RTCConfiguration *config = [[RTCConfiguration alloc] init]; + RTCMediaConstraints *constraints = + [[RTCMediaConstraints alloc] initWithMandatoryConstraints:@{} optionalConstraints:nil]; + + RTCPeerConnectionFactory *factory; + RTCPeerConnection *pc1; + RTCPeerConnection *pc2; + + NSArray *receivers1; + NSArray *receivers2; + + @autoreleasepool { + factory = [[RTCPeerConnectionFactory alloc] init]; + pc1 = [factory peerConnectionWithConfiguration:config constraints:constraints delegate:nil]; + [pc1 senderWithKind:kRTCMediaStreamTrackKindAudio streamId:@"stream"]; + + pc2 = [factory peerConnectionWithConfiguration:config constraints:constraints delegate:nil]; + [pc2 senderWithKind:kRTCMediaStreamTrackKindAudio streamId:@"stream"]; + + NSTimeInterval negotiationTimeout = 15; + XCTAssertTrue([self negotiatePeerConnection:pc1 + withPeerConnection:pc2 + negotiationTimeout:negotiationTimeout]); + + XCTAssertEqual(pc1.signalingState, RTCSignalingStateStable); + XCTAssertEqual(pc2.signalingState, RTCSignalingStateStable); + + receivers1 = pc1.receivers; + receivers2 = pc2.receivers; + XCTAssertTrue(receivers1.count > 0); + XCTAssertTrue(receivers2.count > 0); + [pc1 close]; + [pc2 close]; + pc1 = nil; + pc2 = nil; + factory = nil; + } + receivers1 = nil; + receivers2 = nil; + } + + XCTAssertTrue(true, "Expect test does not crash"); +} + +- (bool)negotiatePeerConnection:(RTCPeerConnection *)pc1 + withPeerConnection:(RTCPeerConnection *)pc2 + negotiationTimeout:(NSTimeInterval)timeout { + __weak RTCPeerConnection *weakPC1 = pc1; + __weak RTCPeerConnection *weakPC2 = pc2; + RTCMediaConstraints *sdpConstraints = + [[RTCMediaConstraints alloc] initWithMandatoryConstraints:@{ + kRTCMediaConstraintsOfferToReceiveAudio : kRTCMediaConstraintsValueTrue + } + optionalConstraints:nil]; + + dispatch_semaphore_t negotiatedSem = dispatch_semaphore_create(0); + [weakPC1 offerForConstraints:sdpConstraints + completionHandler:^(RTCSessionDescription *offer, NSError *error) { + XCTAssertNil(error); + XCTAssertNotNil(offer); + [weakPC1 + setLocalDescription:offer + completionHandler:^(NSError *error) { + XCTAssertNil(error); + [weakPC2 + setRemoteDescription:offer + completionHandler:^(NSError *error) { + XCTAssertNil(error); + [weakPC2 + answerForConstraints:sdpConstraints + completionHandler:^(RTCSessionDescription *answer, + NSError *error) { + XCTAssertNil(error); + XCTAssertNotNil(answer); + [weakPC2 + setLocalDescription:answer + completionHandler:^(NSError *error) { + XCTAssertNil(error); + [weakPC1 + setRemoteDescription:answer + completionHandler:^(NSError *error) { + XCTAssertNil(error); + dispatch_semaphore_signal(negotiatedSem); + }]; + }]; + }]; + }]; + }]; + }]; + + return 0 == + dispatch_semaphore_wait(negotiatedSem, + dispatch_time(DISPATCH_TIME_NOW, (int64_t)(timeout * NSEC_PER_SEC))); +} + @end