1694791 - Assertion failure: !sourceListener->Stopped(), at /builds/worker/checkouts/gecko/dom/media/MediaManager.cpp:2993

Status: VERIFIED FIXED

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

Description

[Jason Kratzer [:jkratzer]]

Attachment: testcase.html

Testcase found while fuzzing mozilla-central rev b3eb91f0b5a7 (built with --enable-debug).

Assertion failure: !sourceListener->Stopped(), at /builds/worker/checkouts/gecko/dom/media/MediaManager.cpp:2993

    #0 0x7ff66518ff88 in mozilla::MediaManager::EnumerateDevicesImpl(nsPIDOMWindowInner*, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, mozilla::MediaSinkEnum, mozilla::MediaManager::DeviceEnumerationType, mozilla::MediaManager::DeviceEnumerationType, bool, RefPtr<mozilla::media::Refcountable<nsTArray<RefPtr<mozilla::MediaDevice> > > > const&)::$_87::operator()(bool) const /builds/worker/checkouts/gecko/dom/media/MediaManager.cpp:2993:13
    #1 0x7ff66518f366 in InvokeMethod<(lambda at /builds/worker/checkouts/gecko/dom/media/MediaManager.cpp:2981:11), RefPtr<mozilla::MozPromise<bool, RefPtr<mozilla::MediaMgrError>, true> > ((lambda at /builds/worker/checkouts/gecko/dom/media/MediaManager.cpp:2981:11)::*)(bool) const, bool> /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:630:12
    #2 0x7ff66518f366 in InvokeCallbackMethod<true, (lambda at /builds/worker/checkouts/gecko/dom/media/MediaManager.cpp:2981:11), RefPtr<mozilla::MozPromise<bool, RefPtr<mozilla::MediaMgrError>, true> > ((lambda at /builds/worker/checkouts/gecko/dom/media/MediaManager.cpp:2981:11)::*)(bool) const, bool, RefPtr<mozilla::MozPromise<bool, RefPtr<mozilla::MediaMgrError>, true>::Private> > /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:646:14
    #3 0x7ff66518f366 in mozilla::MozPromise<bool, RefPtr<mozilla::MediaMgrError>, true>::ThenValue<mozilla::MediaManager::EnumerateDevicesImpl(nsPIDOMWindowInner*, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, mozilla::MediaSinkEnum, mozilla::MediaManager::DeviceEnumerationType, mozilla::MediaManager::DeviceEnumerationType, bool, RefPtr<mozilla::media::Refcountable<nsTArray<RefPtr<mozilla::MediaDevice> > > > const&)::$_87, mozilla::MediaManager::EnumerateDevicesImpl(nsPIDOMWindowInner*, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, mozilla::MediaSinkEnum, mozilla::MediaManager::DeviceEnumerationType, mozilla::MediaManager::DeviceEnumerationType, bool, RefPtr<mozilla::media::Refcountable<nsTArray<RefPtr<mozilla::MediaDevice> > > > const&)::$_88>::DoResolveOrRejectInternal(mozilla::MozPromise<bool, RefPtr<mozilla::MediaMgrError>, true>::ResolveOrRejectValue&) /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:846:9
    #4 0x7ff665177352 in mozilla::MozPromise<bool, RefPtr<mozilla::MediaMgrError>, true>::ThenValueBase::ResolveOrRejectRunnable::Run() /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:487:21
    #5 0x7ff66198847f in mozilla::RunnableTask::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:472:16
    #6 0x7ff6619869f0 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:760:26
    #7 0x7ff6619857b4 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:611:15
    #8 0x7ff661985967 in mozilla::TaskController::ProcessPendingMTTask(bool) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:395:36
    #9 0x7ff66198c296 in operator() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:133:37
    #10 0x7ff66198c296 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_3>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:534:5
    #11 0x7ff66199d787 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1158:16
    #12 0x7ff6619a3dda in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:548:10
    #13 0x7ff6622bf446 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:87:21
    #14 0x7ff66222a893 in MessageLoop::RunInternal() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:335:10
    #15 0x7ff66222a7ad in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:328:3
    #16 0x7ff66222a7ad in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:310:3
    #17 0x7ff66608b948 in nsBaseAppShell::Run() /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:137:27
    #18 0x7ff6678d7e53 in XRE_RunAppShell() /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:902:20
    #19 0x7ff6622c032c in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:237:9
    #20 0x7ff66222a893 in MessageLoop::RunInternal() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:335:10
    #21 0x7ff66222a7ad in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:328:3
    #22 0x7ff66222a7ad in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:310:3
    #23 0x7ff6678d7a28 in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:733:34
    #24 0x556a68c4afa6 in content_process_main /builds/worker/checkouts/gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
    #25 0x556a68c4afa6 in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:309:18
    #26 0x7ff67734e0b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16

Comment 1

[Michael Froman [:mjf]]

Jan-Ivar, do you have any thoughts here?

Comment 2

[Bugmon [:jkratzer for issues]]

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20210406152948-b85e871f6a8d.
The bug appears to have been introduced in the following build range:

Start: 30e17829a10eea6db37c0d92eb8ad98750ee18d3 (20210219041916)
End: f2cb3ed27e689f45cea83bdd6bfc490c21ef74e1 (20210219052948)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=30e17829a10eea6db37c0d92eb8ad98750ee18d3&tochange=f2cb3ed27e689f45cea83bdd6bfc490c21ef74e1

Comment 3

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Marking this as regressed by bug 1692385 since it added the MOZ_ASSERT(!sourceListener->Stopped()) that's failing here, even though there's a chance this merely moved the problem into the line of fire of getUserMedia and this testcase.

Karl, any thoughts? Is the assert warranted? Before that patch we were calling Stop() (which I think is idempotent) but the assertion that it is not already stopped seems new.

It looks like OnNavigation is called on the doc tree to stop everything at once, but IsWindowStillActive() somehow isn't catching it? The test case uses iframe.contentWindow.navigator.mediaDevices.getUserMedia but I think we're using the relevant window exclusively here, so I'm not able to follow why that's a problem.

I suspect this is a difference between placeholder listeners and real ones. Stopping serves a real function of stopping actual media, and I suppose placeholder listeners don't delay that happening, whereas they do delay IsWindowStillActive() from returning false, is that it?

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Might make sense to ping Andreas first for timezone reasons.

Comment 5

[Karl Tomlinson (:karlt)]

The reasoning expected here was:

1. When navigation occurs, the window gets marked as inactive through removal of all its window listeners and Stop()ing of the source/device listeners.
2. After navigation, creation of new listeners on the window would be prevented by the fully active test in getUserMedia().

However there are two situations where 2 does not hold:
A. enumerateDevices() creates listeners, but does not have a similar fully active test.
B. After navigation from an initial about:blank document, the document is replaced but the same (inner) Window is retained and remains fully active.

I can reproduce the assertion failure if the testcase is installed locally.
I haven't succeeded in reproducing under rr, with or without --chaos.
Similarly with video:true.

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: Bug 1694791 check placeholderListener instead of window listener for enumerateDevices() abort r?jib

An enumerateDevices() call before a navigation possibly should sometimes
resolve its promise even after the navigation, but that shouldn't depend on
whether another enumerateDevices() call is made with the same Window after the
navigation but before the first would resolve.

The current behavior is that listeners are removed on Document navigation, but
listeners can be added after navigation. Testing the window listener is
checking for any listener but testing the placeholderListener checks the
listener associated with this particular operation.

Comment 7

[Karl Tomlinson (:karlt)]

Attachment: Bug 1694791 Test enumerateDevices() calls either side of browsing context discard r?jib

Depends on D114317

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f78ced1f5c40
check placeholderListener instead of window listener for enumerateDevices() abort r=jib DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/5a5ad655b63b
Test enumerateDevices() calls either side of browsing context discard r=jib DONTBUILD

Comment 9

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/f78ced1f5c40
https://hg.mozilla.org/mozilla-central/rev/5a5ad655b63b

Comment 10

[Bugmon [:jkratzer for issues]]

Bugmon Analysis:
Verified bug as fixed on rev mozilla-central 20210507095221-1a24ffb3930b.
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.