1581715 - Crash in [@ mozilla::fontlist::Pointer::ToPtr]

Status: VERIFIED FIXED

(Core :: Layout: Text and Fonts, defect, P3)

Description

[blinky]

Steps to reproduce:

1. Set gfx.e10s.font-list.shared to true.
2. Restart Firefox.
3. Download Font Loader.
4. Download Franklin Gothic Book Regular.ttf.
5. Open http://linux.voyager.hr/grsec_last_stable/changelog-stable2.txt.
6. Press Shift-Reload.
7. Open the Font Loader, Click on the Add Fonts button, Select the font file Franklin Gothic Book Regular.ttf then click Open.
8. Click on the Load button.

Actual results:

The tab crashed.

Crash report: bp-dc902d43-a705-403b-87d6-749fd0190917

Top 10 frames of crashing thread:

0 xul.dll mozilla::fontlist::Pointer::ToPtr gfx/thebes/SharedFontList.cpp:40
1 xul.dll mozilla::fontlist::Family::SearchAllFontsForChar gfx/thebes/SharedFontList.cpp:304
2 xul.dll class gfxFont* gfxFontGroup::FindFallbackFaceForChar gfx/thebes/gfxTextRun.cpp:2714
3 xul.dll gfxFontGroup::FindFontForChar gfx/thebes/gfxTextRun.cpp
4 xul.dll gfxFontGroup::InitScriptRun<char16_t> gfx/thebes/gfxTextRun.cpp:2486
5 xul.dll static void gfxFontGroup::InitTextRun<char16_t> gfx/thebes/gfxTextRun.cpp:2408
6 xul.dll gfxFontGroup::MakeTextRun gfx/thebes/gfxTextRun.cpp:2280
7 xul.dll BuildTextRunsScanner::BuildTextRunForFrames layout/generic/nsTextFrame.cpp:2482
8 xul.dll BuildTextRunsScanner::FlushFrames layout/generic/nsTextFrame.cpp:1640
9 xul.dll BuildTextRunsScanner::ScanFrame layout/generic/nsTextFrame.cpp:1964

Comment 1

[blinky]

Regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=87be514024ac53ab6362ffc26610c063d50abe07&tochange=7de7d6a0be86d400ee23ca1ac806eb358555b28d

Comment 2

[[:philipp]]

hi, it looks like the same issue as filed under bug 1554819., so i'll dupe it over. can you post your str there as well?
thanks.

Comment 3

[blinky]

Reopen this. The fix for bug 1554819 doesn't fix this, See bug 1554819 comment 40.

Comment 4

[Jonathan Kew [:jfkthame]]

I can't currently reproduce this with any sort of reliability, which makes it difficult for me to test locally, but I've pushed a try build with a speculative patch that I'm hoping might prevent this crash:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ed01e29c7cf48e5414ecde65f9a1ed7be48f1132&selectedJob=276577474.

If you could give that a try and see whether it affects things on your system, that would be really helpful - thanks!

Comment 5

[blinky]

This is reproduced on Win10 with latest Nightly build.

Steps to reproduce:

1. Download Font Loader.
2. Download Franklin Gothic Book Regular.ttf.
3. Open this page.
4. Press Shift-Reload.
5. Open the Font Loader, Click on the Add Fonts button, Select the font file Franklin Gothic Book Regular.ttf then click Open.
6. Click on the Load button.
7. If it doesn't reproduce, close and open the page again.
8. Click on the Unload button.

Actual results:

The tab crashed.

Crash report: bp-1b0e7305-593d-4a52-807d-c45970191116

Top 10 frames of crashing thread:

0 xul.dll InvalidArrayIndex_CRASH xpcom/ds/nsTArray.cpp:27
1 xul.dll mozilla::fontlist::Pointer::ToPtr gfx/thebes/SharedFontList.cpp:58
2 xul.dll mozilla::fontlist::Family::SearchAllFontsForChar gfx/thebes/SharedFontList.cpp:304
3 xul.dll class gfxFont* gfxFontGroup::FindFallbackFaceForChar gfx/thebes/gfxTextRun.cpp:2717
4 xul.dll gfxFontGroup::FindFontForChar gfx/thebes/gfxTextRun.cpp
5 xul.dll static void gfxFontGroup::InitScriptRun<char16_t> gfx/thebes/gfxTextRun.cpp:2489
6 xul.dll static void gfxFontGroup::InitTextRun<char16_t> gfx/thebes/gfxTextRun.cpp:2411
7 xul.dll gfxFontGroup::MakeTextRun gfx/thebes/gfxTextRun.cpp:2283
8 xul.dll void BuildTextRunsScanner::FlushFrames layout/generic/nsTextFrame.cpp:1644
9 xul.dll void BuildTextRunsScanner::ScanFrame layout/generic/nsTextFrame.cpp:2012

Comment 6

[blinky]

I can not reproduce the crash with the build in comment 4. Thanks.

Comment 7

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

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

Comment 8

[Jonathan Kew [:jfkthame]]

Not really a regression; this is an issue with an in-progress feature that is not yet preffed on.

Comment 9

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1581715 - Ensure the fontgroup is refreshed before attempting to (re-)build textrun for a frame. r=jwatt

Comment 10

[Jonathan Kew [:jfkthame]]

What seems to be going on here is that after a change to the installed system fonts (which causes us to discard and re-create the font list), it's possible for us to get a reflow (e.g. triggered by the RefreshDriver firing an InterruptibleLayout flush) after the content process has updated its font-list to refer to the newly-created shared list, thus invalidating any pointers into the old list, but before the global reconstruction of all frames triggered by gfxPlatform::ForceGlobalReflow() has happened.

This causes problems because if we have existing textframes which have references existing fontgroups, those fontgroups now have Family/Face/SharedBitSet pointers that point to no-longer-mapped memory and cannot safely be touched. So we crash when trying to do font-matching using these objects. These fontgroups and their (obsolete) references are all due to be discarded and rebuilt as a result of ForceGlobalReflow(), but we can't guarantee that'll happen before any other attempt to use them.

To protect against this, we need to check whether the platform font list has been recreated before trying to use a fontgroup to build text runs, and if so, force the fontgroup to redo its BuildFontList so that it gets fresh, valid references to the various font objects.

This is similar to bug 1554819, and we can use the same font-list generation check as there, but it turns out the issue is wider than just canvas contexts.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d7b5314e7c8d
Ensure the fontgroup is refreshed before attempting to (re-)build textrun for a frame. r=jwatt

Comment 12

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/d7b5314e7c8d

Comment 13

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

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Comment 14

[Jonathan Kew [:jfkthame]]

This is still preffed off by default, so status for beta is disabled.

Comment 15

[Ciprian Georgiu, Desktop QA]

I was able to reproduce this crash using the STR from comment 0 and comment 5, on an affected Nightly build from 2019-11-06.

The crash is not reproducing anymore on 73.0, under Windows 10 x64 with a 32-bit Firefox build.