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

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect)

Description

[Byron Campen [:bwc]]

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.

Comment 1

[Byron Campen [:bwc]]

Attachment: MozReview Request: bz://1150966/bwc

/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 2

[Byron Campen [:bwc]]

Comment on attachment 8588065 [details]
MozReview Request: bz://1150966/bwc

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3a78373134fa

Comment 3

[Nils Ohlmeier [:drno]]

Comment on attachment 8588065 [details]
MozReview Request: bz://1150966/bwc

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

LGTM

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

(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?

Comment 5

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Specifically, might the symptoms of that bug just be replaced with the symptoms of this?

Comment 6

[Byron Campen [:bwc]]

I do not think so.

Comment 7

[Byron Campen [:bwc]]

Try needed a retrigger.

Comment 8

[Byron Campen [:bwc]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4420597e8432

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4420597e8432

Comment 10

[Byron Campen [:bwc]]

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.

Comment 11

[Lawrence Mandel [:lmandel] (use needinfo)]

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+

Comment 12

[Ryan VanderMeulen [:RyanVM]]

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

Comment 14

[Byron Campen [:bwc]]

Attachment: MozReview Request: Bug 1150966: Check whether |streams_| is null on stats methods in NrIceMediaStream.