819513 - Crashes due to null media() during PeerCOnnection shutdown

Status: RESOLVED DUPLICATE of bug 820671

(Core :: WebRTC, defect)

Description

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

Christoph Diehl [:cdiehl] 

It does crash at random locations:

#0 0x108aee15c in mozilla::RefPtr<mozilla::NrIceCtx>::RefPtr RefPtr.h:102
#1 0x108b3d3d4 in sipcc::PeerConnectionMedia::ice_ctx const PeerConnectionMedia.h:259

#0 0x10501398c in nsTArray_base<nsTArrayDefaultAllocator>::Length const nsTArray.h:204
#1 0x108b8497b in sipcc::PeerConnectionMedia::AddRemoteStream PeerConnectionMedia.cpp:304

These are null derefs due to pc->impl()->media() is NULL; the PeerConnectionImpl is being shutdown and has nulled the media pointer.  There might be a worse hole if the pointer has been grabbed before it was nulled, and then used after the PeerConnectionMedia object is freed.  We don't have a strong ref to it.

Likely this will be resolved by bug 792175; if it's not then we'll need to look closer.

Comment 1

[Jason Smith [:jsmith]]

Not enough information at this point to warrant a crashtest.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Adding automation-blocked here given that my bug is a dupe of that one.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

My duped bug 820072 has a testcase attached. It's attachment 690492 [details]. So everything we need is available.

Comment 5

[Daniel Veditz [:dveditz]]

bug 792175 is now fixed, is this bug also resolved as comment 0 guessed it would be?

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Bug 820671 is not a specific crash so we shouldn't dupe but mark it on the dependency list. That way it's also easier to proof if known crashes are really fixed by the implementation or not.

Comment 8

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

Please retest or weigh in on whether this is still an issue, now that the Lock/dispatch and media() fixes have landed.

-> cdiehl for re-evaluation

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

It's still crashing with the latest nightly build from today. So it's not fixed yet and the patch on bug 820671 was not that effective as we probably thought. :/

Report: bp-59063487-25ad-418f-a444-826682121214

Comment 10

[Christoph Diehl [:posidron]]

(In reply to Henrik Skupin (:whimboo) from comment #9)
> So it's not fixed yet and the patch on bug 820671 was not that effective as we probably thought. :/

Ack.

Comment 11

[Eric Rescorla (:ekr)]

The crash shown in c9 doesn't appear to be in this section of code and in fact appears to be 

I suggest we close this bug and create a new one for the crash in c9.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

In this case I will reopen my bug 820072 which has been duped against this one. Feel free to close this bug or whatever is appropriate.

Comment 13

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

re-duping

Comment 14

[Jason Smith [:jsmith]]

Moved in-testsuite over to dupe