Closed Bug 1680310 Opened 7 months ago Closed 6 months ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
86 Branch
Tracking Status
firefox-esr78 --- wontfix
firefox84 --- wontfix
firefox85 --- fixed
firefox86 --- fixed

People

(Reporter: tsmith, Assigned: bryce)

References

(Blocks 1 open bug)

Details

(Keywords: assertion)

Attachments

(1 file)

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

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?

Component: Audio/Video → Audio/Video: Playback
Flags: needinfo?(twsmith)

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

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).

Flags: needinfo?(twsmith)

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

Flags: needinfo?(jyavenard)

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?

Assignee: nobody → bvandyk
Flags: needinfo?(jyavenard) → needinfo?(bvandyk)
Severity: -- → S3
Flags: needinfo?(bvandyk)
Priority: -- → P2

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.

Depends on: 1680314
Blocks: 1680314
No longer depends on: 1680314
Pushed by bvandyk@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8da83f327982
Gracefully handle functions running after we Shutdown in RemoteDecoderManagerChild. r=jya
Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → 86 Branch

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.

Flags: needinfo?(bvandyk)

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.
Flags: needinfo?(bvandyk)
Attachment #9193335 - Flags: approval-mozilla-beta?

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

approved for 85.0b7

Attachment #9193335 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.