[10.15] Crash in [@ nsInputStreamPump::AsyncRead]
Categories
(Core :: Graphics: ImageLib, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox67 | --- | fixed |
firefox68 | --- | fixed |
firefox69 | --- | fixed |
People
(Reporter: marcia, Assigned: tnikkel)
References
(Blocks 1 open bug, Regression)
Details
(Keywords: crash, regression)
Crash Data
Attachments
(3 files)
154.77 KB,
text/plain
|
Details | |
24.03 KB,
text/plain
|
Details | |
Bug 1556076. Restore checking the return value of MakeInputStream in nsIconChannel on mac. r?aosmond
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
pascalc
:
approval-mozilla-release+
|
Details | Review |
This bug is for crash report bp-a6c2d644-99fe-4d04-9359-a67730190531.
Seen while looking at Mac nightly crash stats: https://bit.ly/2JORAeg
So far the crashes are all on 10.15.0, and crashes are showing up for 67 and 69 so far. Some code was touched in Bug 1520868. ni on :jkt for any ideas.
Top 10 frames of crashing thread:
0 XUL nsInputStreamPump::AsyncRead netwerk/base/nsInputStreamPump.cpp:300
1 XUL nsIconChannel::AsyncOpen image/decoders/icon/mac/nsIconChannelCocoa.mm:204
2 XUL imgLoader::LoadImage image/imgLoader.cpp:2271
3 XUL nsImageBoxFrame::UpdateImage dom/base/nsContentUtils.cpp:3321
4 XUL nsImageBoxFrame::AttributeChanged layout/xul/nsImageBoxFrame.cpp:139
5 XUL mozilla::RestyleManager::AttributeChanged layout/base/RestyleManager.cpp:3408
6 XUL nsNodeUtils::AttributeChanged layout/base/PresShell.cpp:4333
7 XUL mozilla::dom::Element::SetAttrAndNotify dom/base/Element.cpp:2544
8 XUL mozilla::dom::Element::SetAttr dom/base/Element.cpp:2392
9 XUL mozilla::dom::Element::SetAttribute dom/base/Element.cpp:1397
Comment 1•6 years ago
|
||
Change the status for beta to have the same as nightly and release.
For more information, please visit auto_nag documentation.
Comment 2•6 years ago
|
||
There are Windows and Android crashes, to setting the Platform/OS flags to All. It's only a handful of crashes though and we had a handful as well in 66, it doesn't seem worse in 67 so this is wontfix for our current release.
Reporter | ||
Comment 3•6 years ago
|
||
(In reply to Pascal Chevrel:pascalc from comment #2)
There are Windows and Android crashes, to setting the Platform/OS flags to All. It's only a handful of crashes though and we had a handful as well in 66, it doesn't seem worse in 67 so this is wontfix for our current release.
97% of the crashes in this signature are in 10.15 - so I think the other 4 or so crashes on Win and Android probably aren't the same issue. Many of the comments reference crashing when downloading files. I suspect that is what the newly reported Bug 1556607 is as well.
Comment 6•6 years ago
|
||
Chiming in on this one, one of my coworkers updated to Catalina and experienced this. When trying to download in Safari, it's notable that it seems to trigger a systems permission prompt to allow downloads during the first download attempt after upgrading, whereas Firefox seems to segfault on download. I can't find anything in the release notes that would intimate such a change, but it might be worth digging into.
FWIW, I attempted to grant firefox full disk permissions via System Preferences, however this did not fix the issue.
Reporter | ||
Comment 8•6 years ago
|
||
Discussed during Channel meeting. Moving this to Downloads for now since all of the comments mention crashing while downloading.
Updated•6 years ago
|
Comment 9•6 years ago
|
||
heads-up to Haik and Stephen as breakage from macos 10.15.
Updated•6 years ago
|
Reporter | ||
Updated•6 years ago
|
Reporter | ||
Comment 10•6 years ago
|
||
Currently this is the #5 top overall crash in the 67.0.1 release.
Comment 11•6 years ago
|
||
Adding Timothy as he was changing this in bug 1530190. I think this is an imglib bug.
Comment 12•6 years ago
•
|
||
Honza, that bug affects Windows only code, but the crash volume is almost all Mac. We haven't touched the Mac icon channel implementation in quite some time, so if it is somehow related to how the Mac implementation uses the stream, it would be because something else changed....
(I will investigate from the imagelib side in the meantime.)
Comment 13•6 years ago
|
||
My code landed in 65 I just happen to be the last change on mercurial log.
The log looks like it crashes somewhere in AsyncRead and perhaps on the mStream code, I can't see any changes on:
nsresult rv = mStream->IsNonBlocking(&nonBlocking);
It looks to me a little like mStream isn't being set perhaps?
Comment 14•6 years ago
|
||
I wonder if writing to the pipe somehow failed if we got some garbage for the file size from the OS. We don't actually handle that properly. Here is a speculative fix which might help:
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2eec7117f7d49409cee64b2131f573323bceedd8
Comment 15•6 years ago
|
||
If anybody running 10.15 is willing to try, the debug build just finished up: https://queue.taskcluster.net/v1/task/RTcyriZaTmSwTbAYHZN9WQ/runs/0/artifacts/public/build/target.dmg
If it doesn't fix the crash, please save the debug output and attach it to the bug. Hopefully some warnings got triggered to give us a hint.
Reporter | ||
Comment 16•6 years ago
|
||
Adding the 2 individuals who said they saw crashes. If you could please try the build in Comment 15 and report back, we would greatly appreciate it.
Comment 17•6 years ago
|
||
Running 10.15 (Build: 19A471t)
Opening about:preferences
on the build above crashes without writing to wherever about:crashes
writes to. Would you like the entire crash report from Console?
Apologies if it's a stupid question, I am new!
Comment 18•6 years ago
|
||
(In reply to Hayden Hong from comment #17)
Running 10.15 (Build: 19A471t)
Openingabout:preferences
on the build above crashes without writing to whereverabout:crashes
writes to. Would you like the entire crash report from Console?Apologies if it's a stupid question, I am new!
Yes. It should have spat out a bunch of warnings to the console as well, not quite sure how stderr manifests on Mac, but that would be useful as well.
Comment 19•6 years ago
|
||
(In reply to Andrew Osmond [:aosmond] from comment #18)
Yes. It should have spat out a bunch of warnings to the console as well, not quite sure how stderr manifests on Mac, but that would be useful as well.
The output is ~1500 lines from the Console.app on macOS, much larger than I could attach in a comment here. Does that length sound right, or am I looking for logs in the wrong place? Thanks!
Assignee | ||
Comment 20•6 years ago
|
||
(In reply to Hayden Hong from comment #19)
The output is ~1500 lines from the Console.app on macOS, much larger than I
could attach in a comment here. Does that length sound right, or am I
looking for logs in the wrong place? Thanks!
That sounds right. You can attach it to this bug as a text file.
Comment 21•6 years ago
|
||
Crash Log for https://queue.taskcluster.net/v1/task/RTcyriZaTmSwTbAYHZN9WQ/runs/0/artifacts/public/build/target.dmg when accessing about:preferences
on 10.15 beta 19A471t.
Comment 22•6 years ago
|
||
Also experiencing this issue when attempting to download files from any sites (gmail/facebook) using Firefox 67.0.1 on MacOS 10.15 Beta (19A471t).
In case of a small file (77 KB .jpg) the file successfully downloaded to Mac Downloads folder before the Firefox crash, but retained it's incorrect temporary name (5weWAWg5.jpg.part). When manually renamed to .jpg in Finder, the file worked fine and was complete and not corrupted.
For other files - e.g. the 83.3 MB target.dmg link in comment #15 and comment #21 above, a 1.3 MB stub was left in the Mac Downloads folder which was not usable - q0wTVAvQ.dmg.part
Likewise, when downloading a 6.2 MB pdf from gmail, a zero-size .pdf.part empty file was left in the Mac Downloads folder.
Assignee | ||
Comment 23•6 years ago
|
||
(In reply to Hayden Hong from comment #21)
Created attachment 9070112 [details]
CrashlogCrash Log for
https://queue.taskcluster.net/v1/task/RTcyriZaTmSwTbAYHZN9WQ/runs/0/
artifacts/public/build/target.dmg when accessingabout:preferences
on
10.15 beta 19A471t.
Thanks for that. There is also a different type of output that can be captured if you run the build via the terminal. If you mounted the build and didn't install it in applications you would run "/Volumes/Firefox\ Nightly/Firefox\ NightlyDebug.app/Contents/MacOS/firefox" in the terminal, and then cut paste the output there. Let me know if you need help.
Comment 24•6 years ago
|
||
Output from running Firefox\ NightlyDebug.app/Contents/MacOS/firefox &> log.txt
Assignee | ||
Comment 25•6 years ago
|
||
(In reply to Hayden Hong from comment #24)
Created attachment 9070131 [details]
stderr outputOutput from running
Firefox\ NightlyDebug.app/Contents/MacOS/firefox &> log.txt
Thank you!
We are failing this line
Assignee | ||
Comment 26•6 years ago
|
||
And the problem is that we don't check the return value of MakeInputStream here even though it looks like we do
Confirming that, then I'll post a patch to fix that here.
We'll need a follow up to because this likely means that we won't be able to draw system icons on 10.15, but that's better than crashing.
Assignee | ||
Comment 27•6 years ago
|
||
Bug 1520868 accidentally removed this check.
Assignee | ||
Comment 28•6 years ago
|
||
I made the same line fail in my local build by changing the source and I got a similar looking crash, so this appears to be the issue.
Assignee | ||
Comment 29•6 years ago
|
||
Anyone, feel free to land this once it gets review (it'll prob be a while after it gets review before I'm able to land it myself).
Comment 30•6 years ago
|
||
Lest's annotate the method with MOZ_MUST_USE
to prevent this from happening in the future.
Comment 31•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 32•6 years ago
|
||
Comment on attachment 9070137 [details]
Bug 1556076. Restore checking the return value of MakeInputStream in nsIconChannel on mac. r?aosmond
Beta/Release Uplift Approval Request
- User impact if declined: On macOS 10.15 we crash during several common operations like opening about:preferences, downloading a file, opening the downloads window.
- Is this code covered by automated tests?: Yes
- 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): Restores longstanding behaviour that bug 1520868 accidentally regressed of checking a return value and early returning if it is a failure. We just got lucky that we didn't fail on macOS 10.14 and below.
- String changes made/needed: none
Assignee | ||
Comment 33•6 years ago
|
||
(In reply to Timothy Nikkel (:tnikkel) from comment #26)
We'll need a follow up to because this likely means that we won't be able to
draw system icons on 10.15, but that's better than crashing.
Bug 1557218 is this followup.
Assignee | ||
Comment 34•6 years ago
|
||
(In reply to Masatoshi Kimura [:emk] from comment #30)
Lest's annotate the method with
MOZ_MUST_USE
to prevent this from
happening in the future.
Oops, just saw this.
-> bug 1557225
Comment 35•6 years ago
|
||
Comment on attachment 9070137 [details]
Bug 1556076. Restore checking the return value of MakeInputStream in nsIconChannel on mac. r?aosmond
macos crash fix, for 68.0b8 (hopefully)
Comment 36•6 years ago
|
||
bugherder uplift |
Comment 37•6 years ago
|
||
bugherder |
Comment 38•6 years ago
|
||
Comment on attachment 9070137 [details]
Bug 1556076. Restore checking the return value of MakeInputStream in nsIconChannel on mac. r?aosmond
Approved for mozilla-release and 67.0.2
Comment 39•6 years ago
|
||
bugherder uplift |
Updated•6 years ago
|
Updated•6 years ago
|
Updated•3 years ago
|
Description
•