1911632 - Crash in [@ mozilla::media::MustGetShutdownBarrier]

Status: NEW

(Core :: WebRTC: Audio/Video, defect)

Description

[Chris Peterson [:cpeterson]]

Crash report: https://crash-stats.mozilla.org/report/index/c51be92e-b570-434c-b7c9-64f4f0240803

About 70% of these crash reports are from Android, but we also have reports from Windows and macOS.

MOZ_CRASH Reason: MOZ_RELEASE_ASSERT(barrier)

Top 10 frames:

0  libxul.so  mozilla::media::MustGetShutdownBarrier()  dom/media/systemservices/MediaUtils.cpp:35
1  libxul.so  mozilla::MediaManager::Get()  dom/media/MediaManager.cpp:2476
2  libxul.so  mozilla::dom::MediaDevices::MaybeResumeDeviceExposure()  dom/media/MediaDevices.cpp:203
3  libxul.so  mozilla::dom::MediaDevices::WindowResumed()  dom/media/MediaDevices.h:99
3  libxul.so  nsGlobalWindowInner::Resume(bool)  dom/base/nsGlobalWindowInner.cpp:5645
4  libxul.so  mozilla::dom::BrowsingContextGroup::UpdateToplevelsSuspendedIfNeeded()  docshell/base/BrowsingContextGroup.cpp:280
5  libxul.so  mozilla::dom::BrowsingContext::DidSet(std::__ndk1::integral_constant<unsigned...  docshell/base/BrowsingContext.cpp:2658
5  libxul.so  mozilla::dom::syncedcontext::Transaction<mozilla::dom::BrowsingContext>::Appl...  docshell/base/SyncedContextInlines.h:224
5  libxul.so  mozilla::dom::syncedcontext::FieldValues<mozilla::dom::BrowsingContext::BaseF...  docshell/base/SyncedContext.h:168
5  libxul.so  mozilla::dom::syncedcontext::FieldValues<mozilla::dom::BrowsingContext::BaseF...  docshell/base/SyncedContext.h:154

Comment 1

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

The severity field is not set for this bug.
:jimm, could you have a look please?

For more information, please visit BugBot documentation.

Comment 2

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

The severity field is not set for this bug.
:jimm, could you have a look please?

For more information, please visit BugBot documentation.

Comment 3

[Andreas Pehrson [:pehrsons]]

Normally I'd expect GetShutdownBarrier to fail only if we're in shutdown. But main thread stacks clearly say we are not (I'd have expected ContentProcess::CleanUp on the stack).

Also, we crash on a release assert due to GetShutdownBarrier() failing, which is after passing a release assert for getting a barrier out of nsIAsyncShutdownService.

This means services::GetAsyncShutdownService() returned nullptr.

Either something must have stomped on gXPCOMShuttingDown (seems unlikely), or do_GetService("@mozilla.org/async-shutdown-service;1") failed. The paper trail for the latter goes via either CallGetService failing, gComponentManager being null, or an issue deeper in nsComponentManager (a non-trivial amount of failure modes so seems more likely).

Moving to XPCOM for further diagnosis.

Comment 4

[Nika Layzell [:nika] (ni? for response)]

The async shutdown service is implemented in JS, so technically any interaction with it is always possible to fail (e.g. due to a JS OOM exception).

Bug 1760855 is for making this not be the case eventually, but there are no plans to do this right now, as it's a pretty substantial process.

Comment 5

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

The severity field is not set for this bug.
:nika, could you have a look please?

For more information, please visit BugBot documentation.

Comment 6

[Nika Layzell [:nika] (ni? for response)]

I should also note that this bug is in a content process, so the profileBeforeChange barrier (https://searchfox.org/mozilla-central/rev/f2c181a7ab3bfea4d2266521e6eac713630479b3/dom/media/systemservices/MediaUtils.cpp#21-22) is guaranteed to not be present (https://searchfox.org/mozilla-central/rev/f2c181a7ab3bfea4d2266521e6eac713630479b3/toolkit/components/asyncshutdown/AsyncShutdown.sys.mjs#1102), and xpcomWillShutdown is never reached by a content process in a non-leakchecking build.

Comment 7

[Andreas Pehrson [:pehrsons]]

We'll let this crash for now.

Me and karlt has discussed this a bit.
The best fix here would be to use an infallible shutdown blocker, so that's option 1. Marking bug 1760855 as a dependency because of this.
Option 2 would be to ignore the lack of a shutdown barrier where it doesn't matter. I.e in content processes of non-leakchecking builds. But we also noted MediaManager may be used in the parent process (e.g. gDM permission prompt previews), and the fast-shutdown phase in the parent process is after xpcom-shutdown-threads which MediaManager likely relies on.
Option 3 would be to gracefully handle the failure case of getting a shutdown barrier by e.g. making MediaManager unavailable, but this seems like opening a can of worms. The added complexity may reach far and is hard to overview.

Also note that this is not just MediaManager. See for instance media::ShutdownBlockingTicket which assumes the same barrier availability.

Should the crash-rate go up so we want to do something short-term here, option 2 seems most reasonable.