1680310 - Assertion failure: GetManagerThread() && GetManagerThread()->IsOnCurrentThread(), at src/dom/media/ipc/RemoteDecoderManagerChild.cpp:432

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P2)

Description

[Tyson Smith [:tsmith]]

Assertion failure: GetManagerThread() && GetManagerThread()->IsOnCurrentThread(), at src/dom/media/ipc/RemoteDecoderManagerChild.cpp:432

#0 0x696a6e0617b4 in mozilla::RemoteDecoderManagerChild::OpenForGPUProcess(mozilla::ipc::Endpoint<mozilla::PRemoteDecoderManagerChild>&&) src/dom/media/ipc/RemoteDecoderManagerChild.cpp:432:3
#1 0x696a6e068bf7 in void details::CallFunction<0ul, void (*)(mozilla::ipc::Endpoint<mozilla::PRemoteDecoderManagerChild>&&), mozilla::ipc::Endpoint<mozilla::PRemoteDecoderManagerChild> >(std::integer_sequence<unsigned long, 0ul>, void (*)(mozilla::ipc::Endpoint<mozilla::PRemoteDecoderManagerChild>&&), mozilla::Tuple<mozilla::ipc::Endpoint<mozilla::PRemoteDecoderManagerChild> >&) src/ipc/chromium/src/base/task.h:37:3
#2 0x696a6e068b64 in void DispatchTupleToFunction<void (*)(mozilla::ipc::Endpoint<mozilla::PRemoteDecoderManagerChild>&&), mozilla::ipc::Endpoint<mozilla::PRemoteDecoderManagerChild> >(void (*)(mozilla::ipc::Endpoint<mozilla::PRemoteDecoderManagerChild>&&), mozilla::Tuple<mozilla::ipc::Endpoint<mozilla::PRemoteDecoderManagerChild> >&) src/ipc/chromium/src/base/task.h:53:3
#3 0x696a6e068776 in RunnableFunction<void (*)(mozilla::ipc::Endpoint<mozilla::PRemoteDecoderManagerChild>&&), mozilla::Tuple<mozilla::ipc::Endpoint<mozilla::PRemoteDecoderManagerChild> > >::Run() src/ipc/chromium/src/base/task.h:324:20
#4 0x696a68070f76 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1196:14
#5 0x696a680774b6 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:513:10
#6 0x696a691dbdaf in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:302:20
#7 0x696a690620c6 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:334:10
#8 0x696a69062044 in MessageLoop::RunHandler() src/ipc/chromium/src/base/message_loop.cc:327:3
#9 0x696a69062002 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:309:3
#10 0x696a6806d2f9 in nsThread::ThreadFunc(void*) src/xpcom/threads/nsThread.cpp:441:10
#11 0xd4d04fc7444 in _pt_root src/nsprpub/pr/src/pthreads/ptthread.c:201:5
#12 0x57073fa636da in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76da)
#13 0x7ff09d108a3e in clone /build/glibc-2ORdQG/glibc-2.27/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Comment 1

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

I suspect this is a product of recent changes around decoding processes. Is there a specific test case that triggers this we could use to confirm the regression range?

Comment 2

[Tyson Smith [:tsmith]]

A Pernosco session is available here: https://pernos.co/debug/8mEXd1P3hRG9PXe6jCrq1g/index.html

Comment 3

[Tyson Smith [:tsmith]]

I don't have a test case for this issue. I got this rr trace while trying to capture one for bug 1680314 (which is very unreliable and unreduced, still trying to get a rr trace).

Comment 4

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

:jya, this looks like a result of 1595994, can you take a look? It looks like GetManagerThread() is returning a null ptr.

Comment 5

[Jean-Yves Avenard [:jya]]

We're in the middle of shutdown when RemoteDecoderManagerChild::InitForGPUProcess got called.

Would have been an existing issue before 1595994.

Don't have bandwidth to look into this one at the moment; it's an easy fix especially with the pernosco trace being available.

:bryce can you take this one?

Comment 6

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Attachment: Bug 1680310 - Gracefully handle functions running after we Shutdown in RemoteDecoderManagerChild. r?mattwoodrow

RemoteDecoderManagerChild has a number of functions that may be called
asynchronously. Many such functions assert that they can get the manager thread
and that they are on that thread. However, if we've already shutdown, the thread
they fetch will be null. Since we can enter shutdown while async executions of
these functions are pending, we may fail our asserts.

To avoid this, we instead check if the manager thread is null in these
functions, if so, we assume we're in shutdown and bail. If the thread is not
null, we continue as before and assert we are running on the thread as expected.

I haven't modified RemoteDecoderManagerChild::GetSingleton in this way because
it appears the function is always called synchronously, but have amended the
assert message to make that expectation clear.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8da83f327982
Gracefully handle functions running after we Shutdown in RemoteDecoderManagerChild. r=jya

Comment 8

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/8da83f327982

Comment 9

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

The patch landed in nightly and beta is affected.
:bryce, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 10

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Comment on attachment 9193335 [details]
Bug 1680310 - Gracefully handle functions running after we Shutdown in RemoteDecoderManagerChild. r?mattwoodrow

Beta/Release Uplift Approval Request

• User impact if declined: Unlikely to affect end users, but interferes with fuzzing by having over eager asserts. This fixes asserts firing in normal edge cases (shutdown).
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patch tweaks asserts to avoid them firing during shutdown. The pattern followed already exists in the file but was not used everywhere. The changes are small and are bringing in line a few cases where the pattern was not followed.
• String changes made/needed: None.

Comment 11

[Julien Cristau [:jcristau]]

Comment on attachment 9193335 [details]
Bug 1680310 - Gracefully handle functions running after we Shutdown in RemoteDecoderManagerChild. r?mattwoodrow

approved for 85.0b7

Comment 12

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/475cf26e7ec2