1742362 - [mostly macOS 10.12 in Firefox 94+] Fission crash in [@ nsDeviceContext::BeginPage]

Status: RESOLVED FIXED

(Core :: Printing: Output, defect, P1)

Description

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

Most of these crashes have been observed with macOS 10.12 and Firefox 94.0.1 (94.0 didn't ship to macOS 10.12 users due to bug 1737998). But the reports get frequent on 2021-11-11 while the 94.0.1 started to ship to all users a week before - is this a fission issue? All crash reports have 'dom fission enabled'.

Maybe Fission related. (DOMFissionEnabled=1)

Crash report: https://crash-stats.mozilla.org/report/index/b2edadf8-1bff-4922-99b0-469340211121

Reason: EXC_BAD_ACCESS / KERN_INVALID_ADDRESS

Top 10 frames of crashing thread:

0 XUL nsDeviceContext::BeginPage gfx/src/nsDeviceContext.cpp:346
1 XUL mozilla::layout::RemotePrintJobParent::PrintPage layout/printing/ipc/RemotePrintJobParent.cpp:160
2 XUL mozilla::layout::RemotePrintJobParent::FinishProcessingPage layout/printing/ipc/RemotePrintJobParent.cpp:146
3 XUL mozilla::MozPromise<nsRefCountedHashtable<nsIntegralHashKey<unsigned long long, 0>, RefPtr<mozilla::gfx::RecordedDependentSurface> >, nsresult, true>::ThenValue<mozilla::layout::RemotePrintJobParent::RecvProcessPage xpcom/threads/MozPromise.h:850
4 XUL mozilla::MozPromise<nsRefCountedHashtable<nsIntegralHashKey<unsigned long long, 0>, RefPtr<mozilla::gfx::RecordedDependentSurface> >, nsresult, true>::ThenValueBase::ResolveOrRejectRunnable::Run xpcom/threads/MozPromise.h:487
5 XUL mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal xpcom/threads/TaskController.cpp:770
6 XUL nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1148
7 XUL NS_ProcessPendingEvents xpcom/threads/nsThreadUtils.cpp:432
8 XUL nsAppShell::ProcessGeckoEvents widget/cocoa/nsAppShell.mm:501
9 CoreFoundation CoreFoundation@0xa4e50 

Comment 1

[Chris Peterson [:cpeterson]]

(In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #0)

Most of these crashes have been observed with macOS 10.12 and Firefox 94.0.1 (94.0 didn't ship to macOS 10.12 users due to bug 1737998). But the reports get frequent on 2021-11-11 while the 94.0.1 started to ship to all users a week before - is this a fission issue? All crash reports have 'dom fission enabled'.

Thanks. Yes, this does look like a Fission issue. I see some crash reports from versions before 94.0, but they also correlate with Fission. For example, there are a few crash reports from Release 92.0, when we ran our Fission Release channel experiment, but no reports from Release 93.0 (after the experiment ended). The first report with this signature was from Beta 90.0b12 (June 2021).

Looks like a nsDeviceContext object's mPrintTarget member variable is null here:

https://searchfox.org/mozilla-central/rev/702199bca53babc925e47fd8f86ed56487d42492/gfx/src/nsDeviceContext.cpp#346

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

These are parent process crashes. This is rather bad.

jwatt, could you take a look?

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Setting S1 since it seems like Firefox 94.0.1 has been shipped to Mac OS 10.12.

Comment 4

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1742362 - Improve error handling of nsDeviceContext. r=jwatt,dholbert

I think what's going on is that we get AbortDocument(), then
BeginPage(). It's not too unreasonable that this can happen more often
on macOS 12, since BeginPage() can run off a timer and we had bugs about
printing being slow in those systems (which presumably make users more
likely to cancel printing?).

Anyways add some null-checks and promote some assertions to
DIAGNOSTIC_ASSERTs so that we can catch the bad state ASAP.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f18f0c33b55b
Improve error handling of nsDeviceContext. r=jwatt

Comment 6

[Chris Peterson [:cpeterson]]

Emilio, do you think we should uplift your fix to Beta 96?

The crash rate in Release is pretty low: 213 reports from Fx 94.0.* and 162 from Fx 95.0.* so far. It might be nice to fix, but it's not urgent.

Comment 7

[Emilio Cobos Álvarez [:emilio]]

I'd probably give it some bake time if possible but I guess we're unlikely to have many 10.12 users in nightly... I don't think it's a particularly risky fix so it may make sense to uplift.

Comment 8

[Norisz Fay [:noriszfay]]

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

Comment 9

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 9256198 [details]
Bug 1742362 - Improve error handling of nsDeviceContext. r=jwatt,dholbert

Beta/Release Uplift Approval Request

• User impact if declined: Crashes in macOS 12
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce: We don't have concrete STR
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Relatively simple patch effectively adding a null-check for the purposes of this crash.
• String changes made/needed: none

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9256198 [details]
Bug 1742362 - Improve error handling of nsDeviceContext. r=jwatt,dholbert

96 is on release now.

Comment 11

[Dianna Smith [:diannaS]]

Comment on attachment 9256198 [details]
Bug 1742362 - Improve error handling of nsDeviceContext. r=jwatt,dholbert

Approved for 96.0rc2

Comment 12

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-release/rev/9cbf994f3299