Closed Bug 1276529 Opened 5 years ago Closed 5 years ago
Crash in mozilla::Base
Media Resource::Modify Load Flags
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.
5 years ago
Priority: -- → P1
(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.
Looks like Matthew handled this, so I'm clearing my ni?.
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
Matthew - does bug 910329 fix the issue?
(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
5 years ago
Assignee: nobody → cpearce
JW, Chris is always busy. :-) Can you help on this one?
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)
(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.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: CVE-2017-5396
You need to log in before you can comment on or make changes to this bug.