1584143 - Crash in [@ nsObserverService::NotifyObservers]

Status: VERIFIED FIXED

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

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 https://twitter.com/comic_natalie/status/1175062260156264448.
6. Open the Font Loader, Click on the Add Fonts button, Select the font file Franklin Gothic Book Regular.ttf then click Open.
7. Click on the Load button then Unload.

Actual results:

The tab crashed.

Crash report: bp-e1df90f5-81f3-45e6-b911-f43cd0190926

Top 10 frames of crashing thread:

0 xul.dll nsObserverService::NotifyObservers xpcom/ds/nsObserverService.cpp:277
1 xul.dll gfxPlatformFontList::InitFontList gfx/thebes/gfxPlatformFontList.cpp:408
2 xul.dll gfxFontGroup::GetDefaultFont gfx/thebes/gfxTextRun.cpp:1996
3 xul.dll gfxFontGroup::GetFirstValidFont gfx/thebes/gfxTextRun.cpp:2082
4 xul.dll nsFontMetrics::XHeight gfx/src/nsFontMetrics.cpp:163
5 xul.dll Gecko_GetFontMetrics layout/style/GeckoBindings.cpp:1639
6 xul.dll static struct style::font_metrics::FontMetrics style::gecko::wrapper::{{impl}}::query servo/components/style/gecko/wrapper.rs:1027
7 xul.dll static struct style::values::computed::length::CSSPixelLength style::values::specified::length::FontRelativeLength::to_computed_value servo/components/style/values/specified/length.rs:114
8 xul.dll static struct style::values::computed::length::CSSPixelLength style::values::computed::length::{{impl}}::to_computed_value servo/components/style/values/computed/length.rs:39
9 xul.dll static void style::properties::longhands::max_width::cascade_property i686-pc-windows-msvc/release/build/style-46263c804b1d9f71/out/longhands/position.rs:2721

Comment 1

[Jonathan Kew [:jfkthame]]

Ah, that's a great crash report. This shows is that when the font configuration changes (and therefore we discard and re-create the platform font list), it's possible for gfxPlatformFontList::InitFontList to end up getting called from a stylo thread, which is bad news - it's always assumed that it'll be running on the main thread.

Comment 2

[Jonathan Kew [:jfkthame]]

Try build with a patch that I believe should resolve this (it no longer crashes in my local testing):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cc522fa5d10c89be439159680d53ba491a93ecf6

When the build is available, if you could confirm whether it also fixes the crash on your Win7 system, that would be great - thanks!

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1584143 - Don't try to reinitialize platform font list from GetDefaultFont() if we're not on the main thread. r=jwatt

This can happen if we need to use gfxFontGroup::GetDefaultFont() during stylo traversal,
but we initially failed to create the required font because the font list is stale.
In this case, use a "last-resort" default font entry as a stopgap until the font list
update is completed.

Comment 4

[blinky]

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

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/019d2332ab4b
Don't try to reinitialize platform font list from GetDefaultFont() if we're not on the main thread. r=jwatt

Comment 6

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/019d2332ab4b

Comment 7

[Alin Ilea, Desktop QA]

I have managed to reproduce the issue with an old Nightly build 71.0a1 (Build id: 20190926094200) using Windows 7 and Windows 10.
Verified - fixed on Nightly 71.0a1 (Build id: 20191003164251), latest Nightly 72.0a1 (2019-11-20) and latest Beta 71.0b11.