Crash in [@ gfxFontGroup::FindFontForChar]
Categories
(Core :: Layout: Text and Fonts, defect, P3)
Tracking
()
People
(Reporter: whimboo, Assigned: jfkthame)
References
(Regressed 1 open bug)
Details
(Keywords: crash)
Crash Data
Attachments
(3 files)
(deleted),
text/html
|
Details | |
(deleted),
application/octet-stream
|
Details | |
(deleted),
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details |
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:69.0) Gecko/20100101 Firefox/69.0 ID:20190623215020
This bug is for crash report bp-15b512de-721e-4c00-b5b5-789f60190627.
Top 10 frames of crashing thread:
0 XUL gfxFontGroup::FindFontForChar gfx/thebes/gfxTextRun.cpp:3013
1 XUL void gfxFontGroup::InitScriptRun<char16_t> gfx/thebes/gfxTextRun.cpp:2510
2 XUL gfxFontGroup::MakeTextRun gfx/thebes/gfxTextRun.cpp:2304
3 XUL BuildTextRunsScanner::FlushFrames layout/generic/nsTextFrame.cpp:1633
4 XUL BuildTextRunsScanner::ScanFrame layout/generic/nsTextFrame.cpp:1957
5 XUL BuildTextRunsScanner::ScanFrame layout/generic/nsTextFrame.cpp:2011
6 XUL nsTextFrame::EnsureTextRun layout/generic/nsTextFrame.cpp:2891
7 XUL nsTextFrame::AddInlineMinISizeForFlow layout/generic/nsTextFrame.cpp:8084
8 XUL nsTextFrame::AddInlineMinISize layout/generic/nsTextFrame.cpp:8267
9 XUL nsContainerFrame::DoInlineIntrinsicISize layout/generic/nsContainerFrame.cpp:759
I hit this crash only once while kinda fast scrolling through a Slack channel.
Updated•5 years ago
|
We've seen a few of these in Nightly over the last few weeks, and some in Release as well.
Updated•4 years ago
|
Updated•4 years ago
|
Assignee | ||
Comment 2•4 years ago
|
||
Some Nightly reports of this look like they may have been bug 1667977, for which a fix recently landed; not all reports look like that, though, so there may be multiple underlying issues. It'll be interesting to see how the crash rate looks in post-1667977 builds...
(Nightly and Release are not at all comparable here, because the gfx.e10s.font-list.shared pref is currently enabled in Nightly but disabled in Release, and so there may be quite different code backing this method. But the pref is set to ride to Release in Fx89, so that factor will be going away.)
Steps to reproduce:
- Open this page.
- Installing/removing the Flags Color World font several times.
See https://youtu.be/LX_AUVt9dGQ
Crash report: bp-3c15fc5d-ae47-422b-9ecb-7c87f0210421
Reason: SIGSEGV /SEGV_MAPERR
Top 10 frames of crashing thread:
0 libxul.so gfxFontGroup::FindFontForChar gfx/thebes/gfxTextRun.cpp:3119
1 libxul.so gfxFontGroup::MakeTextRun gfx/thebes/gfxTextRun.cpp:2470
2 libxul.so BuildTextRunsScanner::BuildTextRunForFrames layout/generic/nsTextFrame.cpp:2562
3 libxul.so BuildTextRunsScanner::FlushFrames layout/generic/nsTextFrame.cpp:1660
4 libxul.so BuildTextRunsScanner::ScanFrame layout/generic/nsTextFrame.cpp:2023
5 libxul.so nsTextFrame::EnsureTextRun layout/generic/nsTextFrame.cpp:2999
6 libxul.so nsTextFrame::ReflowText layout/generic/nsTextFrame.cpp:9264
7 libxul.so nsLineLayout::ReflowFrame layout/generic/nsLineLayout.cpp:878
8 libxul.so nsBlockFrame::DoReflowInlineFrames layout/generic/nsBlockFrame.cpp:4336
9 libxul.so nsBlockFrame::Reflow layout/generic/nsBlockFrame.cpp:1375
Updated•4 years ago
|
Assignee | ||
Comment 4•4 years ago
|
||
(In reply to blinky from comment #3)
Steps to reproduce:
- Open this page.
- Installing/removing the Flags Color World font several times.
Did you also choose the Flags font as the default in Firefox preferences, or something like that? (With default settings, I don't see why it would get used at all.)
No, I did not choose the Flags font as the default in Firefox preferences.
I can reproduce the crash with other fonts, but with this font, reproduce it more easily.
Assignee | ||
Comment 6•4 years ago
|
||
Oh, sorry - I misunderstood what I was seeing, it's not a plaintext document but has CSS that specifies the font. OK, that makes sense. I'll try to reproduce and pin down why it's breaking. Thanks!
Comment 7•4 years ago
|
||
This crash signature's volume has jumped in Nightly recently: from 2 crashes in Nightly 85.0a1 (and 0 in 86.0a1) to 157 in 89.0a1 and 62 in 90.0a1. So perhaps related to the fix for SearchAllFontsForChar crash bug 1667977 landing in 89.0a1?
But not many users are affected: the 157 crashes from 89.0a1 are only from 18 users and the 62 crashes from 90.0a1 are from only 5 users, so far.
Comment 8•4 years ago
|
||
Attaching blinky's test files to this bug for posterity.
Comment 9•4 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Comment 10•3 years ago
|
||
I think the patch just landing in bug 1717595 may also fix this, though I've been having some trouble reproducing just now so not 100% sure.
I have triggered Mac and Linux64 builds on https://treeherder.mozilla.org/jobs?repo=try&revision=79cfc3d309538f10c71cc04b9a4f042b16933d01 which includes that patch; blinky, if you could check whether you can still reproduce this crash with the patched build, that would be great. Thanks!
Assignee | ||
Comment 11•3 years ago
|
||
Never mind; I have been able to reproduce this on Linux with a build that includes that patch, so it's clearly not resolved yet.
Comment 12•3 years ago
|
||
I can reproduce on Windows 10, I don't have Ubuntu right now.
Comment 13•3 years ago
|
||
You can try to reproduce on Linux with the steps below.
- Open attachment 9221882 [details].
- Open this page in new tab.
- Installing/removing the Flags Color World font several times.
Assignee | ||
Comment 14•3 years ago
|
||
I figured out at least one scenario where this can happen: if the content process receives a Vsync message that leads to a reflow after the parent has replaced the font list, but before the corresponding RebuildFontList message has been handled. Making the font-list message higher priority, so that it takes precedence over Vsync, seems to enable us to avoid this on my Linux system, at least.
There's a try build in progress at https://treeherder.mozilla.org/jobs?repo=try&revision=d483c1974e7def0169fa8f49b5e1f4cae4ad3462; as always, testing appreciated once it's available.
Assignee | ||
Comment 15•3 years ago
|
||
Updated•3 years ago
|
Comment 16•3 years ago
|
||
Assignee | ||
Comment 17•3 years ago
|
||
Comment on attachment 9229007 [details]
Bug 1561868 - Give the RebuildFontList message a raised priority, so that it is processed ahead of Vsync. r=lsalzman
Beta/Release Uplift Approval Request
- User impact if declined: Possible crash if the user modifies their OS font configuration while the browser is running
- 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): Trivial patch just raising priority of the rebuild-font-list IPC message. (This is only used at the (rare) moments when the system font configuration changes dynamically, so there is no relevance to normal browsing activity.)
- String changes made/needed:
Comment 18•3 years ago
|
||
I can not reproduce the crash with the build in comment 14. Thanks.
Updated•3 years ago
|
Comment 19•3 years ago
|
||
bugherder |
Updated•3 years ago
|
Comment 20•3 years ago
|
||
Comment on attachment 9229007 [details]
Bug 1561868 - Give the RebuildFontList message a raised priority, so that it is processed ahead of Vsync. r=lsalzman
approved for 90 rc1
Comment 21•3 years ago
|
||
bugherder uplift |
Updated•3 years ago
|
Comment 22•3 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #14)
I figured out at least one scenario where this can happen: if the content process receives a Vsync message that leads to a reflow after the parent has replaced the font list, but before the corresponding RebuildFontList message has been handled. Making the font-list message higher priority, so that it takes precedence over Vsync, seems to enable us to avoid this on my Linux system, at least.
Hmm, is this the right fix? Reflows can happen outside vsync events. For example, if the child process is already running a long-running piece of JavaScript code, and then the parent replaces the font list, and then the child JS forces a sync reflow - wouldn't that encounter the same problem?
Assignee | ||
Comment 23•3 years ago
|
||
I think you're basically right, this is a mitigation that reduces the chances of encountering problems, but there is still an underlying issue.
In principle, the child process should be holding a reference to the (old) font list until such time as it handles the update message, and so the memory mapping should remain valid as long as that reference is held (shouldn't it?), but what I think I've observed (but can't readily reproduce/debug) is that the shared memory is unmapped before the child has released its handle.
Updated•3 years ago
|
Description
•