1659013 - Crash in [@ mozilla::BrowserTabsRemoteAutostart] when MOZ_FORCE_DISABLE_E10S is set

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, defect)

Description

[Andrew McCreight [:mccr8]]

This bug is for crash report bp-f5b6e702-98ea-4520-9f7c-c8a8e0200809.

Top 10 frames of crashing thread:

0 xul.dll mozilla::BrowserTabsRemoteAutostart toolkit/xre/nsAppRunner.cpp:5167
1 xul.dll mozilla::PDMFactory::EnsureInit::<unnamed-tag>::operator const dom/media/platforms/PDMFactory.cpp:178
2 xul.dll mozilla::detail::RunnableFunction<`lambda at /builds/worker/checkouts/gecko/dom/media/platforms/PDMFactory.cpp:171:24'>::Run xpcom/threads/nsThreadUtils.h:577
3 xul.dll mozilla::SyncRunnable::Run xpcom/threads/SyncRunnable.h:106
4 xul.dll mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal xpcom/threads/TaskController.cpp:512
5 xul.dll mozilla::detail::RunnableFunction<`lambda at /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:86:7'>::Run xpcom/threads/nsThreadUtils.h:577
6 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1234
7 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:109
8 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:327
9 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:309

This is a null deref on this line: if (forceDisable && !strcmp(forceDisable, gAppData->version)) {. That line was changed in bug 1653384, which landed on August 6, and the first build it showed up in is 20200806215439, so this is the set of change sets in first in that build: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6e35e01646d7c465893a172a0b4fb116c2293d2a&tochange=95cbd137913873b3e3dddda4b4d895ce28e04048

I'm going to call it a regression from that. These crashes are all or mostly happening from mozilla::PDMFactory::EnsureInit, and are all in the GPU process.

It looks like Mossop is out for awhile, so I can look at fixing this.

Looking around, I guess we don't initialize gAppData in non-parent processes, but arguably the bigger issue is that BrowserTabsRemoteAutostart bails out early if XRE_IsContentProcess() is true, but that doesn't hold for the rest of our lovely family of non-parent processes, so I think the early return should instead happen if !XRE_IsParentProcess().

Reading the code, I think we can only hit this if MOZ_FORCE_DISABLE_E10S is set, which is not a supported release configuration, so that makes it less important to back port.

Comment 1

[Andrew McCreight [:mccr8]]

The crashes are all on Windows for some reason. Maybe OSX doesn't have a GPU process normally? I was able to trigger a crash by commenting out the check for XRE_IsContentProcess(), then running with MOZ_FORCE_DISABLE_E10S=1 ./mach run and opening example.com, then opening a new tab and closing the original one.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: Bug 1659013 - Fix some issues with running BrowserTabsRemoteAutostart() in non-parent processes.

We should do an early return in every kind of non-parent process,
not just content processes.

If we somehow get past this check in a non-parent process, and
MOZ_FORCE_DISABLE_E10 is set, then gAppData will be null, so
we hit a null deref. Adding a null check shouldn't be necessary,
but we might as well just add it while we're here.

Finally, fix a minor capitalization error in the comment.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e4bcc1ce643e
Fix some issues with running BrowserTabsRemoteAutostart() in non-parent processes. r=froydnj

Comment 4

[Dorel Luca [:dluca]]

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