We can Reconfigure the send codec before we create the SendStream

RESOLVED FIXED in Firefox 53

Status

()

defect
P1
normal
Rank:
15
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jesup, Assigned: jesup)

Tracking

({crash, csectype-nullptr})

45 Branch
mozilla54
Points:
---

Firefox Tracking Flags

(firefox52 unaffected, firefox53 fixed, firefox54 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

This will cause a crash, since we dereference mSendStream.

Due to the webrtc49 landing
MozReview-Commit-ID: Aj9EH0sHkOS
Attachment #8831481 - Flags: review?(na-g)
forgot the caller is responsible for releasing the frame
Attachment #8831539 - Flags: review?(na-g)
Attachment #8831481 - Attachment is obsolete: true
Attachment #8831481 - Flags: review?(na-g)
one additional spot where mSendStream can get used before StartTransmitting
Attachment #8831540 - Flags: review?(na-g)
Attachment #8831539 - Attachment is obsolete: true
Attachment #8831539 - Flags: review?(na-g)
Comment on attachment 8831540 [details] [diff] [review]
handle ReconfigureSendCodec before StartTransmitting

Review of attachment 8831540 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. r+
Attachment #8831540 - Flags: review?(na-g) → review+
Pushed by rjesup@wgate.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d842007ad15f
handle ReconfigureSendCodec before StartTransmitting r=ng
NI to request uplift to Aurora
Flags: needinfo?(rjesup)
Rank: 15
Priority: -- → P1
https://hg.mozilla.org/mozilla-central/rev/d842007ad15f
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment on attachment 8831540 [details] [diff] [review]
handle ReconfigureSendCodec before StartTransmitting

Approval Request Comment
[Feature/Bug causing the regression]: Webrtc.org 49 branch landing (in 53)

[User impact if declined]: race condition causing null-derefs when creating webrtc calls

[Is this code covered by automated tests?]: no, timing related (the code gets lots of coverage, but nothing catches the race).  Found in a local debug build when working with a patch that changed timing.

[Has the fix been verified in Nightly?]: yes

[Needs manual test from QE? If yes, steps to reproduce]:  no

[List of other uplifts needed for the feature/fix]: none

[Is the change risky?]: no

[Why is the change risky/not risky?]: Adds null checks

[String changes made/needed]: none
Flags: needinfo?(rjesup)
Attachment #8831540 - Flags: approval-mozilla-aurora?
Comment on attachment 8831540 [details] [diff] [review]
handle ReconfigureSendCodec before StartTransmitting

Fix a potential crash. Aurora53+.
Attachment #8831540 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.