1548813 - Crash in [@ gfxFontFamily::FindFontForStyle]

Status: VERIFIED FIXED

(Core :: Graphics, defect, P3)

Description

[Wayne Mery (:wsmwk)]

New crash, but not frequent [1]

This bug is for crash report bp-6e5b6941-f820-4c43-813f-d19660190409.

Top 10 frames of crashing thread:

0 xul.dll gfxFontFamily::FindFontForStyle gfx/thebes/gfxFontEntry.cpp:1242
1 xul.dll gfxPlatformFontList::SystemFindFontForChar gfx/thebes/gfxPlatformFontList.cpp:573
2 xul.dll gfxFontGroup::FindFontForChar gfx/thebes/gfxTextRun.cpp:2885
3 xul.dll gfxFontGroup::ComputeRanges<char16_t> gfx/thebes/gfxTextRun.cpp:2962
4 xul.dll gfxFontGroup::InitScriptRun<char16_t> gfx/thebes/gfxTextRun.cpp:2415
5 xul.dll gfxFontGroup::InitTextRun<char16_t> gfx/thebes/gfxTextRun.cpp:2337
6 xul.dll gfxFontGroup::MakeTextRun gfx/thebes/gfxTextRun.cpp:2209
7 xul.dll BuildTextRunsScanner::BuildTextRunForFrames layout/generic/nsTextFrame.cpp:2435
8 xul.dll BuildTextRunsScanner::FlushFrames layout/generic/nsTextFrame.cpp:1658
9 xul.dll nsTextFrame::EnsureTextRun layout/generic/nsTextFrame.cpp:2871

[1] so far all Windows 10. Not startup crashes
bp-ca0a0513-cd58-4978-bce1-e648d0190430 2019-04-30 13:49:29 20190421221535 0xe5e5e5f5 2019-04-23 04:48:21 10.0.17134 32137
bp-e1c25956-c3fd-438f-9da6-9610a0190430 2019-04-30 10:46:50 20190426100024 0xffffffffffffffff 2019-04-29 02:22:53 10.0.17763 114114
bp-3785a070-a8ef-4d44-b747-883130190417 2019-04-17 06:29:10 20190416083948 0xffffffffffffffff 2019-04-17 03:08:41 10.0.18875 11308
bp-6914e519-441a-439b-bb0e-6b7280190409 2019-04-09 12:03:09 20190408084104 0xffffffffffffffff 2019-04-08 18:18:27 10.0.17763 57771
bp-6e5b6941-f820-4c43-813f-d19660190409 2019-04-09 07:08:50 20190408084104 0xffffffffffffffff 2019-04-08 12:33:20 10.0.17763 13592

Comment 1

[Wayne Mery (:wsmwk)]

No THunderbird crashes of consequence since 2019-04-30 so => incomplete

Comment 2

[Wayne Mery (:wsmwk)]

Firefox 69.0a1 bp-94de3d35-0f05-4462-a009-1b5930190607

0 libxul.so gfxFontFamily::FindFontForStyle(gfxFontStyle const&, bool) gfx/thebes/gfxFontEntry.cpp:1384 context
1 libxul.so gfxPlatformFontList::SystemFindFontForChar(unsigned int, unsigned int, mozilla::unicode::Script, gfxFontStyle const*) gfx/thebes/gfxPlatformFontList.cpp:730 cfi
2 libxul.so gfxFontGroup::FindFontForChar(unsigned int, unsigned int, unsigned int, mozilla::unicode::Script, gfxFont*, FontMatchType*) gfx/thebes/gfxTextRun.cpp:3003 cfi
3 libxul.so void gfxFontGroup::InitScriptRun<char16_t>(mozilla::gfx::DrawTarget*, gfxTextRun*, char16_t const*, unsigned int, unsigned int, mozilla::unicode::Script, gfxMissingFontRecorder*) gfx/thebes/gfxTextRun.cpp:2500 cfi
4 libxul.so void gfxFontGroup::InitTextRun<char16_t>(mozilla::gfx::DrawTarget*, gfxTextRun*, char16_t const*, unsigned int, gfxMissingFontRecorder*) gfx/thebes/gfxTextRun.cpp:2422 cfi
5 libxul.so gfxFontGroup::MakeTextRun(char16_t const*, unsigned int, gfxTextRunFactory::Parameters const*, mozilla::gfx::ShapedTextFlags, nsTextFrameUtils::Flags, gfxMissingFontRecorder*) gfx/thebes/gfxTextRun.cpp:2294 cfi
6 libxul.so BuildTextRunsScanner::BuildTextRunForFrames(void*) layout/generic/nsTextFrame.cpp:2445 cfi
7 libxul.so BuildTextRunsScanner::FlushFrames(bool, bool) layout/generic/nsTextFrame.cpp:1642 cfi
8 libxul.so nsTextFrame::EnsureTextRun(nsTextFrame::TextRunType, mozilla::gfx::DrawTarget*, nsIFrame*, nsLineList_iterator const*, unsigned int*) layout/generic/nsTextFrame.cpp:2900 cfi
9 libxul.so nsTextFrame::ReflowText(nsLineLayout&, int, mozilla::gfx::DrawTarget*, mozilla::ReflowOutput&, nsReflowStatus&) layout/generic/nsTextFrame.cpp:8906 cfi
10 libxul.so nsLineLayout::ReflowFrame(nsIFrame*, nsReflowStatus&, mozilla::ReflowOutput*, bool&) layout/generic/nsLineLayout.cpp:880 cfi
11 libxul.so nsBlockFrame::ReflowInlineFrame(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) layout/generic/nsBlockFrame.cpp:4336 cfi
12 libxul.so nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) layout/generic/nsBlockFrame.cpp:3055 cfi
13 libxul.so nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) layout/generic/nsBlockFrame.cpp:1334 cfi
14 libxul.so nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) layout/generic/nsBlockReflowContext.cpp:297 cfi
15 libxul.so nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) layout/generic/nsBlockFrame.cpp:3052 cfi

Comment 3

[Jonathan Kew [:jfkthame]]

Note that the crash in comment 2 is from Linux, whereas the original reports here were all Win10.

Looking at crash-stats, the recent Firefox crashes all seem to be Linux, while the Thunderbird ones are all Windows. Don't know if there's any significance to that.

Comment 4

[Marcia Knous [:marcia]]

This is still happening on 70 and 69, but in very low volume.

Comment 5

[Wayne Mery (:wsmwk)]

#11 crash for Thunderbird 68.0. bp-6914e519-441a-439b-bb0e-6b7280190409 is the earliest crash found - 68.0a1 buildid 20190408084104

Comment 6

[Jonathan Kew [:jfkthame]]

Ah, I see a possible cause for this: if content includes U+FFFD (Unicode REPLACEMENT CHARACTERs) for some reason -- e.g. encoding errors -- we cache the font family used to render this codepoint, to avoid an expensive search every time it occurs. But if the font-list then gets reinitialized, e.g. because a font is installed or removed on the system, that cached pointer will be invalidated and we need to clear it. Failing to do that could result in a crash that looks like this, I think.

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1548813 - Ensure cached mReplacementCharFallbackFamily is cleared if the font list is reinitialized. r=jrmuizel

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8e5c9493f893
Ensure cached mReplacementCharFallbackFamily is cleared if the font list is reinitialized. r=jrmuizel

Comment 9

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/8e5c9493f893

Comment 10

[Jorg K (CEST = GMT+2)]

Wayne, should we take this one-liner to TB 68.x? Looks like FF don't plan to fix it on their ESR.

Comment 11

[Jonathan Kew [:jfkthame]]

I think we should also consider taking the fix for FF 70beta/68esr, given its trivial nature, and that it fixes a crash seen in the wild (even though it's low-volume).

Comment 12

[Jonathan Kew [:jfkthame]]

Comment on attachment 9091739 [details]
Bug 1548813 - Ensure cached mReplacementCharFallbackFamily is cleared if the font list is reinitialized. r=jrmuizel

Beta/Release Uplift Approval Request

• User impact if declined: Potential crash if installed fonts are changed while the browser is running
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• 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): Extremely trivial patch to simply clear a cached value when reinitializing the font list
• String changes made/needed:

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Trivial fix for a potential crash (seen in low volume in the wild, including on ESR)
• User impact if declined:
• Fix Landed on Version: 71
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Extremely trivial patch to simply clear a cached value when reinitializing the font list
• String or UUID changes made by this patch:

Comment 13

[Wayne Mery (:wsmwk)]

+1 to both ideas

Comment 15

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9091739 [details]
Bug 1548813 - Ensure cached mReplacementCharFallbackFamily is cleared if the font list is reinitialized. r=jrmuizel

Crash fix, looks pretty simple, let's take it for beta 7 and for ESR as well.

Comment 16

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/releases/mozilla-beta/rev/56a8e96ba56c

Comment 17

[Andreea Pavel [:apavel]]

Tried to uplift this to esr68 and got a conflict:

grafting 566497:8e5c9493f893 "Bug 1548813 - Ensure cached mReplacementCharFallbackFamily is cleared if the font list is reinitialized. r=jrmuizel"
merging gfx/thebes/gfxPlatformFontList.cpp
warning: conflicts while merging gfx/thebes/gfxPlatformFontList.cpp! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')

File looks like: https://irccloud.mozilla.com/file/PPNGeWSH/image.png

Jonathan can you take a look?

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Liz, is this supposed to land for ESR 68.1.2 and 68.2.0 or just 68.2.0?

Comment 19

[Jonathan Kew [:jfkthame]]

Attachment: patch rebased to esr68

Rebased for ESR68 (due to bug 1575315 having touched the immediately-preceding context).

Comment 20

[Liz Henry (:lizzard) (relman/hg->git project)]

Just for 68.2.0, I forgot to mark the tracking flag for 70+. Thanks Aryx!

Comment 21

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-esr68/rev/c93c2b91c3f5

Comment 22

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/releases/mozilla-esr68/rev/f2d0fc74353b5bec140f62171abcf92c10bdff28 on THUNDERBIRD_68_VERBRANCH for TB 68.1.2.

Comment 23

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 24

[Wayne Mery (:wsmwk)]

gfxFontFamily::FindFontForStyle is gone for Thunderbird and Firefox