1648354 - Crash in [@ mozilla::ipc::MessageChannel::Send | mozilla::ipc::IProtocol::ChannelSend]

Status: RESOLVED FIXED

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

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 data:text/html;charset=utf-8,<u>🏴󠁵󠁳󠁣󠁡󠁿
6. Open https://archive.org/ in new tab.
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.
9. Wait two minutes.

Actual results:

Browser crashes.

Crash report: bp-f39f22ec-c0cc-4e32-b941-060240200625

Top 10 frames of crashing thread:

0 xul.dll mozilla::ipc::MessageChannel::Send ipc/glue/MessageChannel.cpp
1 xul.dll mozilla::ipc::IProtocol::ChannelSend ipc/glue/ProtocolUtils.cpp:477
2 xul.dll mozilla::dom::PContentChild::SendGetFontListShmBlock ipc/ipdl/PContentChild.cpp:5139
3 xul.dll mozilla::fontlist::FontList::GetBlockFromParent gfx/thebes/SharedFontList.cpp:582
4 xul.dll mozilla::fontlist::FontList::UpdateShmBlocks gfx/thebes/SharedFontList.cpp:602
5 xul.dll mozilla::fontlist::FontList::FindFamily gfx/thebes/SharedFontList.cpp:825
6 xul.dll gfxPlatformFontList::FindAndAddFamilies gfx/thebes/gfxPlatformFontList.cpp:1075
7 xul.dll gfxDWriteFontList::FindAndAddFamilies gfx/thebes/gfxDWriteFontList.cpp:1764
8 xul.dll gfxFontGroup::AddPlatformFont gfx/thebes/gfxTextRun.cpp:1825
9 xul.dll gfxFontGroup::BuildFontList gfx/thebes/gfxTextRun.cpp:1740

Comment 1

[Andrew McCreight [:mccr8]]

This looks like a thread safety assertion from a Stylo thread sending a message via PContentChild. That sounds bad.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1648354 - Handle the fact that ToPtr() may return null in the child process. r?jwatt

Normally, if IsNull() is false, we'd expect ToPtr() to return a valid pointer,
but for code that may run on a stylo thread in the child process this is not
necessarily true: if resolving the pointer requires accessing a new shared-mem
block, we can't make that IPC call from the stylo thread. So in this case, we
let ToPtr() return null, and the caller needs to handle this safely.

Comment 3

[Jonathan Kew [:jfkthame]]

(In reply to Andrew McCreight [:mccr8] from comment #1)

This looks like a thread safety assertion from a Stylo thread sending a message via PContentChild.

We have MOZ_RELEASE_ASSERT there so we detect this and safely crash the content process, and this depends on the user installing/uninstalling fonts locally while the browser is running, it's not something a site can trigger by itself.

So yes, that's what is going on here; but it's not clear to me how this would be a sec-high issue; seems to me like it would be sec-moderate at most?

Comment 4

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

:jfkthame, since this bug is a regression, could you fill (if possible) the regressed_by field?
For more information, please visit auto_nag documentation.

Comment 5

[Andrew McCreight [:mccr8]]

Ah, I didn't realize that this was a release assert and not just something in Nightly.

Comment 6

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

https://hg.mozilla.org/integration/autoland/rev/3e78b6a57f062559963f618bb12275dc1431f36b
https://hg.mozilla.org/mozilla-central/rev/3e78b6a57f06