1334840 - We can Reconfigure the send codec before we create the SendStream

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P1)

Description

[Randell Jesup [:jesup] (needinfo me)]

This will cause a crash, since we dereference mSendStream.

Due to the webrtc49 landing

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Attachment: handle ReconfigureSendCodec before StartTransmitting

MozReview-Commit-ID: Aj9EH0sHkOS

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Attachment: handle ReconfigureSendCodec before StartTransmitting

forgot the caller is responsible for releasing the frame

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Attachment: handle ReconfigureSendCodec before StartTransmitting

one additional spot where mSendStream can get used before StartTransmitting

Comment 4

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Comment on attachment 8831540 [details] [diff] [review]
handle ReconfigureSendCodec before StartTransmitting

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

Looks good. r+

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d842007ad15f
handle ReconfigureSendCodec before StartTransmitting r=ng

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

NI to request uplift to Aurora

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d842007ad15f

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

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

Comment 9

[Gerry Chang [:gchang]]

Comment on attachment 8831540 [details] [diff] [review]
handle ReconfigureSendCodec before StartTransmitting

Fix a potential crash. Aurora53+.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f3491aa69b18