crash in mozilla::ipc::MessageChannel::OnChannelErrorFromLink() on asan builds
Categories
(Core :: IPC, defect, P2)
Tracking
()
People
(Reporter: hanno, Assigned: snorp)
References
Details
(Whiteboard: [geckoview:p1])
Crash Data
Attachments
(5 files)
3.85 KB,
text/plain
|
Details | |
3.85 KB,
text/plain
|
Details | |
2.63 KB,
text/plain
|
Details | |
4.16 KB,
text/plain
|
Details | |
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
jcristau
:
approval-mozilla-release-
jcristau
:
approval-mozilla-geckoview64+
|
Details | Review |
I'm getting occasional crashes (with crash dumps) with the asan builds of mozilla. (asan build "target.tar.bz2" downloaded on 2017-04-04, sha256 69926deabcd04a4a2e6030001dae365bca5985ab83979b42bf1675b969e2377f) The crashes usually happen if I want to close the browser during heavy activity (I was experimenting with HTML files including lots of iframes). However I could also reproduce them most reliably by just starting firefox and killing it shortly after, e.g.: ./firefox & sleep 3; killall firefox (This doesn't depend on "killall", it also sometimes works with clicking the close symbol, but needs the right timing) I got different stack traces, but the by far most common one indicates a crash in mozilla::ipc::MessageChannel::OnChannelErrorFromLink(). I got a different crash in mozilla::ipc::BackgroundChildImpl::ProcessingError(), however I was not able to reproduce that and only got it once. A third crash is probably irrelevant: It happens in RunWatchdog(), looking at the code it is just some kind of timeout where it kills the application if it supposedly hangs. I just mention it for completeness. I'll attach all three stack traces.
Reporter | ||
Comment 1•7 years ago
|
||
Reporter | ||
Comment 2•7 years ago
|
||
Reporter | ||
Comment 3•7 years ago
|
||
Comment 4•7 years ago
|
||
These are all various intentional crashes when we detect an error. The OnChannelError ones are here: MOZ_CRASH("Aborting on channel error."); The nsTerminator one is here: MOZ_CRASH("Shutdown too long, probably frozen, causing a crash.");
Updated•7 years ago
|
Updated•7 years ago
|
Updated•6 years ago
|
Updated•6 years ago
|
Updated•6 years ago
|
Comment 5•5 years ago
|
||
jld - tsmith mentioned that they hit this in ASan builds at the all hands, this is one of a number of similar bugs that I picked. It looks like this [1] was originally put in to try and clean up orphaned plugin processes (and I guess subsequently all children). I wonder if MOZ_CRASH is really what we want to do here or whether something like ProcessChild::QuickExit would be more appropriate. [1] https://searchfox.org/mozilla-central/rev/fd62b95c187a40b328d9e7fd9d848833a6942b57/ipc/glue/MessageChannel.cpp#2527
Comment 6•5 years ago
|
||
It does look like that was more or less the intent in bug 778866. Assuming nobody's trying to get information out of the crashes, I agree this could just be a QuickExit, but I think we should log something in case the exit happens when it shouldn't (cf. bug 1368036).
Assignee | ||
Comment 7•5 years ago
|
||
Android mercilessly kills the parent in low memory situations, and we don't want that to trigger a crash when the child is abruptly disconnected.
Assignee | ||
Updated•5 years ago
|
Updated•5 years ago
|
Pushed by jwillcox@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/36802e2a3490 Exit instead of MOZ_CRASH on channel error in child process r=jld
Assignee | ||
Comment 10•5 years ago
|
||
Comment on attachment 9035725 [details]
Bug 1354200 - Exit instead of MOZ_CRASH on channel error in child process r=jld
[Beta/Release Uplift Approval Request]
Feature/Bug causing the regression: N/A
User impact if declined: Frequent crashes when Firefox Focus is in the background.
Is this code covered by automated tests?: No
Has the fix been verified in Nightly?: No
Needs manual test from QE?: No
If yes, steps to reproduce:
List of other uplifts needed: None
Risk to taking this patch: Low
Why is the change risky/not risky? (and alternatives if risky): Very small and simply avoids an intentional crash.
String changes made/needed: None
Comment 11•5 years ago
|
||
bugherder |
Comment 12•5 years ago
|
||
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #10)
Attachment #9035725 [details] - Flags: approval-mozilla-release?
James, are you requesting uplift to just the GECKOVIEW_64_RELBRANCH branch? Or also Fennec 64 and Firefox 64 desktop on the Release channel?
IIUC, this fix won't affect Fennec (because it doesn't use e10s), but it could affect/benefit Firefox desktop.
Comment 13•5 years ago
|
||
Comment on attachment 9035725 [details]
Bug 1354200 - Exit instead of MOZ_CRASH on channel error in child process r=jld
Moving uplift request to gv64, I don't think we should be taking this on the default branch.
Comment 14•5 years ago
|
||
Thanks. Julien can we poke someone to approve and land on geckoview64? That's the only current vehicle we have to confirm the fix, and there is very low risk.
Comment 15•5 years ago
|
||
Comment on attachment 9035725 [details]
Bug 1354200 - Exit instead of MOZ_CRASH on channel error in child process r=jld
You can poke me :)
Approved for beta65 and gv64, will land on the latter in a bit.
Comment 16•5 years ago
|
||
uplift |
Comment 17•5 years ago
|
||
bugherder uplift |
Updated•5 years ago
|
Description
•