1522417 - Crash in gfxUserFontEntry::LoadPlatformFont

Status: RESOLVED FIXED

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

Description

[[:philipp]]

This bug is for crash report bp-9f907b34-6b2c-46c5-b227-5a32d0190124.

Top 10 frames of crashing thread:

0 libxul.so gfxUserFontEntry::LoadPlatformFont gfx/thebes/gfxUserFontSet.cpp:747
1 libxul.so gfxUserFontEntry::FontDataDownloadComplete gfx/thebes/gfxUserFontSet.cpp:805
2 libxul.so nsFontFaceLoader::OnStreamComplete layout/style/nsFontFaceLoader.cpp:256
3 libxul.so mozilla::net::nsStreamLoader::OnStopRequest netwerk/base/nsStreamLoader.cpp:94
4 libxul.so nsCORSListenerProxy::OnStopRequest netwerk/protocol/http/nsCORSListenerProxy.cpp:615
5 libxul.so mozilla::extensions::ChannelWrapper::RequestListener::OnStopRequest toolkit/components/extensions/webrequest/ChannelWrapper.cpp:948
6 libxul.so mozilla::net::nsStreamListenerTee::OnStopRequest netwerk/base/nsStreamListenerTee.cpp:42
7 libxul.so mozilla::net::nsHttpChannel::ContinueOnStopRequest netwerk/protocol/http/nsHttpChannel.cpp:7752
8 libxul.so mozilla::net::nsHttpChannel::OnStopRequest netwerk/protocol/http/nsHttpChannel.cpp:7517
9 libxul.so nsInputStreamPump::OnStateStop netwerk/base/nsInputStreamPump.cpp:656

these crashes appear cross-platform since nightly build 20190121175139. so far they look more common on android, on windows they're manifesting themselves as content crashes. could this be related to the changes of bug 1519918?

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1522417 - Add diagnostics.

I haven't managed to find a way stuff should unexpectedly die around these
functions, so add a few diagnostics to catch this hopefully, before papering
over the bug / backing out / adding a kungfudeathgrip.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Will add some diagnostics for now, to try to catch the bug, since I couldn't via code inspection.

Comment 3

[Emilio Cobos Álvarez [:emilio]]

May be related to bug 1519918 or bug 1520062 in any case, yeah.

Comment 4

[Andrew McCreight [:mccr8]]

Bug 1520062 got backed out for causing a crash in a different OnStop method, FWIW.

Comment 5

[Andrew McCreight [:mccr8]]

It looks like there were a bunch of crashes on the 22 and not many since then, so maybe the backout did the trick?

Comment 6

[Emilio Cobos Álvarez [:emilio]]

Yeah, perhaps. Still the diagnostics don't hurt probably, let's see if they catch something. Otherwise let's WFM.

Comment 7

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

https://hg.mozilla.org/integration/autoland/rev/0fc47438d8372975cc9d75740e6a0f7a3723ed2d
https://hg.mozilla.org/mozilla-central/rev/0fc47438d837

Comment 8

[Calixte Denizet (:calixte)]

There are 13 crashes with signatures "nsFontFaceLoader::Cancel" and the crash reason is:
MOZ_RELEASE_ASSERT(!mInLoadTimerCallback)

Comment 11

[Emilio Cobos Álvarez [:emilio]]

I think bug 1523181 will fix this, I'll add more diagnostics otherwise.

Comment 13

[Chris Peterson [:cpeterson]]

Curiously, I see 16x more crash reports with this signature from ARM64 builds of Fennec 67 Nightly than from ARMv7 builds.

Comment 14

[[:philipp]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #11)

I think bug 1523181 will fix this, I'll add more diagnostics otherwise.

there still reports from nightly builds including the changes in bug 1523181:
bp-875266cb-7e7b-4966-9c13-cba5f0190206
bp-64a55734-b758-413a-95ad-81f370190206

Comment 15

[Emilio Cobos Álvarez [:emilio]]

Yup, I realized they were different in bug 1524246. I think that matches this bug better.

Comment 16

[Emilio Cobos Álvarez [:emilio]]

(And that landed just today)

Comment 17

[[:philipp]]

thanks!

Comment 18

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

Are you still planning to work on this for 66? Or do you think it's better aimed at 67? I can keep tracking for 66 and we can still potentially take an uplift.

Comment 19

[Emilio Cobos Álvarez [:emilio]]

Oh, I see why this is still crashing on beta, we forgot to back out the diagnostics patch, but they're MOZ_DIAGNOSTIC_ASSERTs so shouldn't crash on release. I can back out that patch from beta just like we backed out bug 1526252, if you want me to. Though again shouldn't crash release, so should be fine.

It seems there are no crashes on Nightly at all, which makes sense because this was fixed in all the dependent bugs.

Comment 20

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

Sure, if you can do the backout, it seems like a good idea. I get that it won't make any substantial difference but I like to have less noise in beta crashes. (And to have a good idea of the overall crash rate across different beta releases, too.) Thanks!

Comment 21

[Emilio Cobos Álvarez [:emilio]]

I guess that was supposed to go to me :)

Comment 22

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

Oops, yes.

Comment 23

[Emilio Cobos Álvarez [:emilio]]

Attachment: Revert diagnostics on beta.

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

Bug 1522417

User impact if declined

DIAGNOSTIC_ASSERT crashes on beta.

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)

Just reverting the diagnostics added here since the patches that introduced the regression were backed out from beta.

String changes made/needed

none

Comment 24

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

Comment on attachment 9045770 [details] [diff] [review]
Revert diagnostics on beta.

Backout of diagnostic crash stuff. 
OK for uplift for beta 11.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/59a0b8b45c1e