Closed
Bug 1276529
Opened 8 years ago
Closed 7 years ago
Crash in mozilla::BaseMediaResource::ModifyLoadFlags
Categories
(Core :: Audio/Video: Playback, defect, P1)
Tracking
()
RESOLVED
DUPLICATE
of bug 1329403
People
(Reporter: dbaron, Assigned: cpearce)
References
()
Details
(Keywords: crash, topcrash)
Crash Data
This bug was filed from the Socorro interface and is report bp-245d5eae-ede9-4e06-bd0e-252ca2160529. ============================================================= There are a decent number of these ModifyLoadFlags crashes across all channels: https://crash-stats.mozilla.com/signature/?product=Firefox&date=%3E%3D2016-05-01&signature=mozilla%3A%3ABaseMediaResource%3A%3AModifyLoadFlags&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=email&_columns=url&_columns=user_comments&page=2 A bugzilla search makes me wonder if they're at all related to the issue in bug 910329. In any case, if you have private crash-stats access, the URLs for these crashes actually look generally useful; they seem to point to various video sites, the most common of which has URLs of the form (from a different crash): http://layarkaca21.tv/london-fallen-2016/2/ http://layarkaca21.tv/captain-america-civil-war-2016/ etc. I debugged the minidump for bp-245d5eae-ede9-4e06-bd0e-252ca2160529 , and the problem there is that (as the MSVC dialog says): this->mChannel.mRawPtr was nullptr. Is it expected that the call to loadGroup->RemoveRequest() can cause this->mChannel to become null? Because it certainly looks like that's what's happening. This isn't at the top of the topcrash list, but seems like it might be straightforward to fix and is worth looking into.
Flags: needinfo?(kinetik)
Flags: needinfo?(cpearce)
Updated•8 years ago
|
Priority: -- → P1
Comment 1•8 years ago
|
||
(In reply to David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) (vacation June 4-12) from comment #0) > A bugzilla search makes me wonder if they're at all related to the issue in > bug 910329. That seems plausible, but I didn't dig enough to prove it's the cause. I posted a patch on that bug, since it (seems) simple and we can see if it solves this problem. > Is it expected that the call to loadGroup->RemoveRequest() can cause > this->mChannel to become null? Because it certainly looks like that's > what's happening. I think we only clear mChannel when the channel is closed, which we don't expect to happen as a result of removing the request from the load group.
Flags: needinfo?(kinetik)
Assignee | ||
Comment 2•8 years ago
|
||
Looks like Matthew handled this, so I'm clearing my ni?.
Flags: needinfo?(cpearce)
Comment 3•8 years ago
|
||
Crash volume for signature 'mozilla::BaseMediaResource::ModifyLoadFlags': - nightly (version 50): 4 crashes from 2016-06-06. - aurora (version 49): 30 crashes from 2016-06-07. - beta (version 48): 392 crashes from 2016-06-06. - release (version 47): 1115 crashes from 2016-05-31. - esr (version 45): 123 crashes from 2016-04-07. Crash volume on the last weeks: Week N-1 Week N-2 Week N-3 Week N-4 Week N-5 Week N-6 Week N-7 - nightly 0 0 1 3 0 0 0 - aurora 3 1 3 5 5 12 1 - beta 63 49 51 68 54 66 20 - release 170 139 182 157 169 195 46 - esr 19 4 5 15 9 15 10 Affected platforms: Windows, Mac OS X, Linux
status-firefox47:
--- → affected
status-firefox48:
--- → affected
status-firefox49:
--- → affected
status-firefox50:
--- → affected
status-firefox-esr45:
--- → affected
Matthew - does bug 910329 fix the issue?
Flags: needinfo?(kinetik)
Comment 5•8 years ago
|
||
(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #4) > Matthew - does bug 910329 fix the issue? Doesn't look like it, there are still crashes (null mChannel) showing up in 50.0a2/20160822004020 builds, e.g.: https://crash-stats.mozilla.com/report/index/6c4cbdb8-0b65-4b78-8c7d-fb2322160823
Flags: needinfo?(kinetik)
Updated•8 years ago
|
Assignee: nobody → cpearce
Comment 6•7 years ago
|
||
JW, Chris is always busy. :-) Can you help on this one?
Flags: needinfo?(jwwang)
Updated•7 years ago
|
See Also: → CVE-2017-5396
Comment 7•7 years ago
|
||
http://searchfox.org/mozilla-central/rev/0f254a30d684796bcc8b6e2a102a0095d25842bb/dom/media/MediaResource.cpp#1561 Is it possible for nsLoadGroup::RemoveRequest() to spin the event loop of the main thread such that something funny happens in between? I wonder if this has something to do with bug 1329403.
Flags: needinfo?(jwwang) → needinfo?(honzab.moz)
Comment 8•7 years ago
|
||
(In reply to JW Wang [:jwwang] [:jw_wang] from comment #7) > http://searchfox.org/mozilla-central/rev/ > 0f254a30d684796bcc8b6e2a102a0095d25842bb/dom/media/MediaResource.cpp#1561 > > Is it possible for nsLoadGroup::RemoveRequest() to spin the event loop of > the main thread such that something funny happens in between? This is very hard to answer, but there certainly is a possibility. If the request being removed is the last foreground request, we trigger OnStopRequest on the listener of the load group. That can do number of things. Personally, I would forbid/remove/burn any API allowing spinning the thread loop this way. But it's of course not that easy. So, to answer - yes, it's possible. > > I wonder if this has something to do with bug 1329403. Didn't check that bug much deeply. I don't have answer to that, sorry.
Flags: needinfo?(honzab.moz)
Updated•7 years ago
|
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•