Closed Bug 1150966 Opened 5 years ago Closed 5 years ago

Stats methods on NrIceMediaStream do not check whether |stream_| is null

Categories

(Core :: WebRTC: Networking, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox38 --- unaffected
firefox39 --- fixed
firefox40 --- fixed

People

(Reporter: bwc, Assigned: bwc)

References

Details

(Whiteboard: regression)

Attachments

(1 file, 1 obsolete file)

Since the stats query stuff acquires references to NrIceMediaStreams, and then dispatches to STS to interrogate these references, it is possible for them to be closed by the time this happens, resulting in a null deref.

I'm not seeing any sign of this on crash-stats, but we should go ahead and fix.
Attached file MozReview Request: bz://1150966/bwc (obsolete) —
/r/6607 - Bug 1150966: Check whether |streams_| is null on stats methods in NrIceMediaStream.

Pull down this commit:

hg pull -r 28d628fa3f8682cea020883db79889127422bed7 https://reviewboard-hg.mozilla.org/gecko/
Comment on attachment 8588065 [details]
MozReview Request: bz://1150966/bwc

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3a78373134fa
Attachment #8588065 - Flags: review?(drno)
Comment on attachment 8588065 [details]
MozReview Request: bz://1150966/bwc

https://reviewboard.mozilla.org/r/6605/#review5487

LGTM
Attachment #8588065 - Flags: review?(drno) → review+
(In reply to Byron Campen [:bwc] from comment #0)
> I'm not seeing any sign of this on crash-stats, but we should go ahead and
> fix.

Might that change following the fixing of Bug 1147857?
See Also: → 1147857
Specifically, might the symptoms of that bug just be replaced with the symptoms of this?
I do not think so.
Try needed a retrigger.
Flags: needinfo?(docfaraday)
https://hg.mozilla.org/mozilla-central/rev/4420597e8432
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Comment on attachment 8588065 [details]
MozReview Request: bz://1150966/bwc

Approval Request Comment
[Feature/regressing bug #]:

   Bug 1146462

[User impact if declined]:

   Rare null-ptr crashes while starting a webrtc call.

[Describe test coverage new/current, TreeHerder]:

   This is a sufficiently rare crash that it does not seem to show up in CI. I have not investigated what it would take to reliably cause the crash.

[Risks and why]: 

   Extremely low, just adds some early return checks in a number of places.

[String/UUID change made/needed]:

   None.
Attachment #8588065 - Flags: approval-mozilla-aurora?
Comment on attachment 8588065 [details]
MozReview Request: bz://1150966/bwc

Looks like a safe fix that has been on m-c for 9 days. Let's get this on m-a as we still have half the cycle left. Aurora+
Attachment #8588065 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8588065 - Attachment is obsolete: true
Attachment #8619964 - Flags: review+
You need to log in before you can comment on or make changes to this bug.