1875407 - Crash in [@ mozilla::MediaTrackGraphImpl::Notify]

Status: RESOLVED FIXED

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

Description

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

Crash report: https://crash-stats.mozilla.org/report/index/4567ea4d-c31e-460a-a7f6-70a960240118

MOZ_CRASH Reason: MOZ_DIAGNOSTIC_ASSERT(!mShutdownBlocker) (MediaTrackGraph took too long to shut down!)

Top 10 frames of crashing thread:

0  xul.dll  mozilla::MediaTrackGraphImpl::Notify  dom/media/MediaTrackGraph.cpp:1715
1  xul.dll  nsTimerImpl::Fire::<lambda_12>::operator const  xpcom/threads/nsTimerImpl.cpp:677
1  xul.dll  mozilla::detail::VariantImplementation<unsigned char, 1, nsCOMPtr<nsITimerCallback>, nsCOMPtr<nsIObserver>, nsTimerImpl::FuncCallback, nsTimerImpl::ClosureCallback>::matchN  mfbt/Variant.h:309
1  xul.dll  mozilla::detail::VariantImplementation<unsigned char, 0, nsTimerImpl::UnknownCallback, nsCOMPtr<nsITimerCallback>, nsCOMPtr<nsIObserver>, nsTimerImpl::FuncCallback, nsTimerImpl::ClosureCallback>::matchN  mfbt/Variant.h:318
1  xul.dll  mozilla::Variant<nsTimerImpl::UnknownCallback, nsCOMPtr<nsITimerCallback>, nsCOMPtr<nsIObserver>, nsTimerImpl::FuncCallback, nsTimerImpl::ClosureCallback>::matchN  mfbt/Variant.h:902
1  xul.dll  mozilla::Variant<nsTimerImpl::UnknownCallback, nsCOMPtr<nsITimerCallback>, nsCOMPtr<nsIObserver>, nsTimerImpl::FuncCallback, nsTimerImpl::ClosureCallback>::match  mfbt/Variant.h:857
1  xul.dll  nsTimerImpl::Fire  xpcom/threads/nsTimerImpl.cpp:675
1  xul.dll  nsTimerEvent::Run  xpcom/threads/TimerThread.cpp:515
2  xul.dll  mozilla::RunnableTask::Run  xpcom/threads/TaskController.cpp:578
2  xul.dll  mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal  xpcom/threads/TaskController.cpp:905

By querying Nightly crashes reported within the last 2 months, here are some insights about the signature:

• First crash report: 2024-01-18
• Process type: Content
• Is startup crash: No
• Has user comments: No
• Is null crash: Yes - 1 out of 4 crashes happened on null or near null memory address

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Core::XPCOM' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Andrew McCreight [:mccr8]]

FYI, Karl. It looks like you added this diagnostic assert in bug 1872787.

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: Bug 1875407 downgrade MediaTrackGraph shutdown diagnostic assertion to MOZ_ASSERT() r?padenot

We have plenty of crashes from Nightly users to investigate.
Leaving the MOZ_ASSERT() in place will catch any instances in CI.

Comment 4

[Karl Tomlinson (:karlt)]

For https://crash-stats.mozilla.org/report/index/4567ea4d-c31e-460a-a7f6-70a960240118#allthreads (Windows 11),
the MainThread is not in any nested event loop (including not in ShutdownXPCOM(), which is not called in shipped builds),
the two GraphRunner threads are in ThreadState::Wait, and
AudioIPC Client RPC and AudioIPC Client Callback threads are polling for another event.

Stacks I checked for other platforms were similar:

https://crash-stats.mozilla.org/report/index/84f20141-e0f1-48b7-80d4-cca790240120#allthreads (Ubuntu 22.04) had 4 GraphRunner threads

https://crash-stats.mozilla.org/report/index/7a0baea6-3389-4901-ab93-610800240120#allthreads (macOS) had 2 GraphRunner threads
and a MediaTrackGrph thread for a ThreadedDriver, waiting for the next iteration. We're missing the line number which would indicate whether the MediaTrackGrph thread is waiting for another message or just until more processing.

https://crash-stats.mozilla.org/report/index/72f1f3a3-2047-45e0-b914-94f4f0240121#allthreads (Android) had one GraphRunner thread named "Thread-2".

The MediaTrackGraph does not look busy and is not waiting for cubeb_stream_stop().

The only place that the graph is force shut down when the process is not shutting down is AudioContext::OnWindowDestroy().

Plausibly, the main thread might be busy for 20 seconds and a ScriptProcessorNode might even make that more likely, but I don't know whether that would be as common as indicated by crash reports.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ee72c01626bf
downgrade MediaTrackGraph shutdown diagnostic assertion to MOZ_ASSERT() r=padenot

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: Bug 1875407 downgrade MediaTrackGraph shutdown diagnostic assertion to MOZ_ASSERT() r?padenot

We have plenty of crashes from Nightly users to investigate.
Leaving the MOZ_ASSERT() in place will catch any instances in CI.

Original Revision: https://phabricator.services.mozilla.com/D199192

Comment 7

[Phabricator Automation]

Uplift Approval Request

• Fix verified in Nightly: no
• Is Android affected?: yes
• Code covered by automated testing: no
• Steps to reproduce for manual QE testing: n/a
• User impact if declined: Safe crashes in only early beta builds.
• Explanation of risk level: Reverts addition of crashing assertion.
• Needs manual QE test: no
• String changes made/needed: none.
• Risk associated with taking this patch: very low

Comment 8

[Atila Butkovits]

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

Comment 9

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

Set release status flags based on info from the regressing bug 1872787

Comment 10

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/547151893bb4