1716713 - Startup Crash in [@ mozilla::mscom::ProcessRuntime::InitInsideApartment]

Status: RESOLVED FIXED

(Core :: IPC: MSCOM, defect)

Description

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

3 affected installations so far, all crashes with Windows 10

Crash report: https://crash-stats.mozilla.org/report/index/185d0679-3620-414a-8351-5e8b30210616

MOZ_CRASH Reason: MOZ_DIAGNOSTIC_ASSERT((((HRESULT)(mInitResult)) >= 0))

Top 7 frames of crashing thread:

0 xul.dll mozilla::mscom::ProcessRuntime::InitInsideApartment ipc/mscom/ProcessRuntime.cpp:241
1 xul.dll mozilla::mscom::ProcessRuntime::ProcessRuntime ipc/mscom/ProcessRuntime.cpp:131
2 xul.dll XRE_InitChildProcess toolkit/xre/nsEmbedFunctions.cpp:661
3 firefox.exe wmain toolkit/xre/nsWindowsWMain.cpp:131
4 firefox.exe __scrt_common_main_seh /builds/worker/workspace/obj-build/browser/app/f:/dd/vctools/crt/vcstartup/src/startup/exe_common.inl:288
5 kernel32.dll BaseThreadInitThunk 
6 ntdll.dll RtlUserThreadStart 

Comment 1

[James Teh [:Jamie]]

I'm guessing this is caused by the skeleton UI appearing with an accessibility client present. See bug 1714212. Previously, this was a debug assert, but now that it's a diagnostic assert, we're seeing this crash.

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

This is in content though, which is weird.

Comment 3

[(No longer employed by Mozilla) Aaron Klotz]

I'll keep an eye on it, but given that it's only across a small number of affected installations, I'm more interested in learning about why that code is failing than I am about the assertion itself.

Comment 4

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Bug 1716713: Make mscom::ProcessRuntime construction occur prior to XPCOM init in mozilla::dom::ContentProcess; r=Jamie

This is a speculative fix: Typically we instantiate ProcessRuntime prior
to XPCOM initialization to ensure that we init COM before XPCOM calls any
APIs that might trigger an implicit init. In this case we were instantiating
after XPCOM init, which might not be helpful.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/156e2478cc0b
Make mscom::ProcessRuntime construction occur prior to XPCOM init in mozilla::dom::ContentProcess; r=Jamie

Comment 6

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/156e2478cc0b

Comment 7

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: Bug 1716713 - Remove HAVE_SIDEBAR build condition r=smaug

Comment 8

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

Comment 7 is intended for bug 1717613.

Comment 9

[Kagami Rosylight [:saschanaz] (they/them)]

😱 sorry!

Comment 10

[Phabricator Automation]

Comment on attachment 9228591 [details]
Bug 1716713 - Remove HAVE_SIDEBAR build condition r=smaug

Revision D118613 was moved to bug 1717613. Setting attachment 9228591 [details] to obsolete.

Comment 11

[Andrew McCreight [:mccr8]]

Still seems to be happening: bp-75baf12e-7112-471a-b745-5d8030210624

39 crashes from a single installation on the 20210623214552 build.

Comment 12

[(No longer employed by Mozilla) Aaron Klotz]

As long as the number of installs is very small, I'm not concerned about this -- these are pre-release builds, after all. The diagnostic assert is meant to catch any systemic failures caused as a result of changes in Gecko itself.

eg If we already had this diagnostic assert in place when the skeleton UI changes landed, we probably would have caught the a11y issues immediately.

I see injected DLLs in the crash report from comment 11, so it might be interesting to take a look at our third-party modules ping and learn which injection vector is being used to get those DLLs into our processes so early.

I'm going to move this to "External Software affecting Firefox," with the understanding that as long as the number of distinct installs is very small, I consider this to be a WONTFIX from the IPC: MSCOM perspective.

Comment 13

[(No longer employed by Mozilla) Aaron Klotz]

My bad, I forgot that I had already landed a patch in this bug. I'll file a new one for the injected DLLs.