Closed
Bug 1423756
Opened 7 years ago
Closed 7 years ago
Nullptr crash in MediaPipeline
Categories
(Core :: WebRTC, defect, P2)
Core
WebRTC
Tracking
()
RESOLVED
INCOMPLETE
Tracking | Status | |
---|---|---|
firefox59 | --- | affected |
People
(Reporter: drno, Assigned: drno)
References
(Blocks 1 open bug)
Details
Crash Data
Attachments
(1 file)
(deleted),
text/x-review-board-request
|
Details |
Low frequency null pointer crash in MediaPipeline: https://crash-stats.mozilla.com/report/index/822d599b-a9ed-4cf1-9723-4a6280171205
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → drno
Rank: 15
Priority: -- → P2
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•7 years ago
|
||
The idea behind the initial patch is that it should be easily upliftable to stop the bleeding.
Not sure if we can figure out the root cause and fix that as well.
Assignee | ||
Comment 3•7 years ago
|
||
The only point where conduit_ appears to be set to null is here https://searchfox.org/mozilla-central/rev/2e08acdf8862e68b13166970e17809a3b5d6a555/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp#1259
Which means the destructor for PipelineListener ran on MainThread, and therefor the desctructor for MediaPipelineTransmit ran as well. What I fail to understand where this stuff gets connected to the MSG.
My assumption here is since the destructors ran MSG should no try to deliver any of this data any more.
bwc: do you have thoughts on why we get into this crash situation?
Flags: needinfo?(docfaraday)
Comment 4•7 years ago
|
||
Yeah, MSG should be holding onto a ref to the PipelineListener at this point, so it should not have been destroyed yet. Also, NotifyRealtimeTrackData is a virtual function; if PipelineListener had been destroyed, that vtable lookup should not have worked anyhow. Maybe there's some way we can init with a null conduit successfully?
Flags: needinfo?(docfaraday)
Comment 5•7 years ago
|
||
I don't see this happening on 59 at all; have we landed a fix there or something? The transceivers work was tripping over failure to create conduits in a different place, and now that _should_ be patched up.
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8935165 [details]
Bug 1423756: check for conduit before trying to use it.
https://reviewboard.mozilla.org/r/206024/#review211838
I fear that we're seeing some kind of undefined behavior here, so messing with it carries the risk of turning a safe nullptr crash into an unsafe one. On nightly, maybe this would be ok if it helped us diagnose what is going on, but I don't know if I like the idea of uplifting this.
Attachment #8935165 -
Flags: review?(docfaraday)
Assignee | ||
Comment 7•7 years ago
|
||
(In reply to Byron Campen [:bwc] from comment #5)
> I don't see this happening on 59 at all;
Well for some unknown reason this never crashed in Nightly very much. In the last 6 months I see:
- 2 crashes in 56 Nightly
- 6 crashes in 57 Nightly
- 2 crashes in 58 Nightly
- 0 crashes in 59 Nightly so far
> have we landed a fix there or
> something?
I think it's to early in the Nightly cycle to assume that this got fixed in 59 somehow.
> The transceivers work was tripping over failure to create
> conduits in a different place, and now that _should_ be patched up.
Since the crash rate on beta is a lot higher it might make sense to wait for 59 to reach Beta and see what happens.
Assignee | ||
Comment 8•7 years ago
|
||
The only recent crashes happened with outdated version of Firefox, e.g. 55 or 56.
I see only a single crash in Fx 59, when it was still in the Nightly cycle.
I think it's safe to assume this got fixed through some other bug, for example the Transceivers landing in 59.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → INCOMPLETE
You need to log in
before you can comment on or make changes to this bug.
Description
•