Fix crash on addIceCandidate, setRemote/LocalDescription #11
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There are two problems with setLocalDescription / setRemoteDescription in ObjC SDK. First, RTCSessionDescription.nativeDescription returns a raw nullableSessionDescriptionInterface pointer, where sLD/sRD are calling Clone() method unconditionally, so it might crash. Second, unnecessary sLD/sRD calls Clone() of the raw pointer and does not delete it, so this pointer will leak. To solve these problems, I changed the return type of nativeDescription to std::unique_ptr and removed the call to Clone() method. Bug: webrtc:13022, webrtc:13035 Change-Id: Icbb87dda62d3a11af47ec74621cf64b8a6c05228 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/227380 Reviewed-by: Kári Helgason <kthelgason@webrtc.org> Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org> Commit-Queue: Byoungchan Lee <daniel.l@hpcnt.com> Cr-Commit-Position: refs/heads/master@{#34647}
RTCIceCandidate.nativeCandidate returns a unique_ptr that can be null. As with the previous CL, this is used without checking whether it is null or not, so it should be fixed. Bug: None Change-Id: I70a84f7a2a9a9d47b8cefa198204f9b6d63a7c7f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/227620 Commit-Queue: Byoungchan Lee <daniel.l@hpcnt.com> Reviewed-by: Kári Helgason <kthelgason@webrtc.org> Cr-Commit-Position: refs/heads/master@{#34649} (cherry picked from commit 8c48757)
intoxicated
approved these changes
Sep 8, 2021
|
저 타입이 c++타입이라서 그렇습니당 |
x-0o0
reviewed
Sep 15, 2021
Comment on lines
+162
to
+170
| dispatch_semaphore_signal(negotiatedSem); | ||
| } | ||
| }]; | ||
|
|
||
| NSTimeInterval timeout = 5; | ||
| ASSERT_EQ( | ||
| 0, | ||
| dispatch_semaphore_wait(negotiatedSem, | ||
| dispatch_time(DISPATCH_TIME_NOW, (int64_t)(timeout * NSEC_PER_SEC)))); |
There was a problem hiding this comment.
167 번째줄의 ASSERT_EQ은 162번째줄의 disaptch_semaphor_signal 메소드 호출까지 기다리는게 맞나요?
| [[RTC_OBJC_TYPE(RTCSessionDescription) alloc] initWithType:RTCSdpTypeAnswer sdp:[self sdp]]; | ||
|
|
||
| webrtc::SessionDescriptionInterface *nativeDescription = | ||
| std::unique_ptr<webrtc::SessionDescriptionInterface> nativeDescription = |
Comment on lines
-584
to
+582
| _peerConnection->SetLocalDescription(sdp.nativeDescription->Clone(), observer); | ||
| _peerConnection->SetLocalDescription(sdp.nativeDescription, observer); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.