Assertion failure: mInitDone, at /builds/worker/workspace/build/src/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp:190

RESOLVED FIXED in Firefox 64

Status

()

defect
P3
normal
RESOLVED FIXED
10 months ago
9 months ago

People

(Reporter: jkratzer, Assigned: pehrsons)

Tracking

(Blocks 1 bug, {assertion, testcase})

Trunk
mozilla64
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr60 wontfix, firefox62 wontfix, firefox63 wontfix, firefox64 fixed)

Details

(crash signature)

Attachments

(4 attachments)

Reporter

Description

10 months ago
Posted file testcase.html
Testcase found while fuzzing mozilla-central rev 26990836dc5c.

Testcase must be served via a local webserver in order to reproduce.

rax = 0x0000000000000000   rdx = 0x0000000000000000
rcx = 0x0000000000000b40   rbx = 0x00007f959eefcc10
rsi = 0x00007f95ba3478b0   rdi = 0x00007f95ba346680
rbp = 0x00007f959f57c1e0   rsp = 0x00007f959f57bdf0
r8 = 0x00007f95ba3478b0    r9 = 0x00007f959f57d700
r10 = 0x00000000ffffffee   r11 = 0x0000000000000000
r12 = 0x00007f95af6e6200   r13 = 0x00007f95a0463850
r14 = 0x00007f959eef9838   r15 = 0x00007f959eef9870
rip = 0x00007f95aa7e6421
OS|Linux|0.0.0 Linux 4.15.0-33-generic #36-Ubuntu SMP Wed Aug 15 16:00:05 UTC 2018 x86_64
CPU|amd64|family 6 model 78 stepping 3|1
GPU|||
Crash|SIGSEGV /SEGV_MAPERR|0x0|23
23|0|libxul.so|mozilla::MediaEngineRemoteVideoSource::Allocate(mozilla::dom::MediaTrackConstraints const&, mozilla::MediaEnginePrefs const&, nsTString<char16_t> const&, mozilla::ipc::PrincipalInfo const&, mozilla::AllocationHandle**, char const**)|hg:hg.mozilla.org/mozilla-central:dom/media/webrtc/MediaEngineRemoteVideoSource.cpp:26990836dc5cc3cd1b8027392b79210e71094dc3|191|0x18
23|1|libxul.so|mozilla::MediaDevice::Allocate(mozilla::dom::MediaTrackConstraints const&, mozilla::MediaEnginePrefs const&, mozilla::ipc::PrincipalInfo const&, char const**)|hg:hg.mozilla.org/mozilla-central:dom/media/MediaManager.cpp:26990836dc5cc3cd1b8027392b79210e71094dc3|1111|0x13
23|2|libxul.so|mozilla::GetUserMediaTask::Run()|hg:hg.mozilla.org/mozilla-central:dom/media/MediaManager.cpp:26990836dc5cc3cd1b8027392b79210e71094dc3|1850|0x12
23|3|libxul.so|nsThread::ProcessNextEvent(bool, bool*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:26990836dc5cc3cd1b8027392b79210e71094dc3|1161|0x15
23|4|libxul.so|NS_ProcessNextEvent(nsIThread*, bool)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:26990836dc5cc3cd1b8027392b79210e71094dc3|519|0x11
23|5|libxul.so|mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:26990836dc5cc3cd1b8027392b79210e71094dc3|334|0xa
23|6|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:26990836dc5cc3cd1b8027392b79210e71094dc3|325|0x17
23|7|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:26990836dc5cc3cd1b8027392b79210e71094dc3|318|0x8
23|8|libxul.so|base::Thread::ThreadMain()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/thread.cc:26990836dc5cc3cd1b8027392b79210e71094dc3|181|0x8
23|9|libxul.so|ThreadFunc|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/platform_thread_posix.cc:26990836dc5cc3cd1b8027392b79210e71094dc3|44|0xa
23|10|libpthread-2.27.so||||0x76db
23|11|libc-2.27.so||||0x12188f
Flags: in-testsuite?
Reporter

Updated

10 months ago
Crash Signature: [@mozilla::MediaEngineRemoteVideoSource::Allocate], [@mozilla::AudioMixer::FinishMixing]
ni? Jan-Ivar since Andreas is on PTO and I'm not sure who wants to own this
Flags: needinfo?(jib)
Priority: -- → P3
Assignee

Comment 2

10 months ago
So mInitDone can be legitimately false if CamerasChild::GetCaptureDevice in Init() failed. We'll want to change this assert back to the guard we had before.
Assignee: nobody → apehrson
Flags: needinfo?(jib)
Assignee

Comment 3

10 months ago
(In reply to Andreas Pehrson [:pehrsons] from comment #2)
> So mInitDone can be legitimately false if CamerasChild::GetCaptureDevice in
> Init() failed. We'll want to change this assert back to the guard we had
> before.

That is actually not what is happening here. When reproducing this I see for the failing MediaEngineRemoteVideoSource the following sequence:

- Init()
- Shutdown()
- Allocate()

No wonder mInitDone is false. Perhaps it's still the best solution to re-instate the runtime guard for mInitDone, but first we have to understand this race properly.
Status: NEW → ASSIGNED
Assignee

Comment 4

9 months ago
We are basically continuing with Allocate() after an async operation despite having started shutting down in the meantime.

Better window validity checks after async operations fixes this.

I'll also revert the runtime guards for mInitDone since GetCaptureDevice could legitimately fail.
Comment on attachment 9009931 [details]
Bug 1488832 - Change init assert to runtime guard. r?jib

Jan-Ivar Bruaroey [:jib] (needinfo? me) has approved the revision.
Attachment #9009931 - Flags: review+
Comment on attachment 9009930 [details]
Bug 1488832 - Improve window validity checks after async gUM steps. r?jib

Jan-Ivar Bruaroey [:jib] (needinfo? me) has approved the revision.
Attachment #9009930 - Flags: review+
Assignee

Comment 9

9 months ago
I've made some attempts at an automated test but to no avail. I'll push it to try to make sure but locally it hasn't reproduced.

I ended up with a mochitest (for loopback devices, thus the real device backends) that loads the testcase in an iframe and is done after a minute (should be enough to verify that it works).

gUM requests are working and all, but no crashes.
Assignee

Comment 10

9 months ago
The good news is that try can repro the issue with my mochitest. The bad news that I still can't locally :-)

I'll just reduce the timeout, validate it, and put the patch up.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e15535acae84843e7171edefd446895e6b05856c
Comment on attachment 9010614 [details]
Bug 1488832 - Add mochitest. r?jib

Jan-Ivar Bruaroey [:jib] (needinfo? me) has approved the revision.
Attachment #9010614 - Flags: review+

Comment 14

9 months ago
Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/c808e4d6fb53
Add mochitest. r=jib
https://hg.mozilla.org/integration/autoland/rev/2ea7113db27c
Improve window validity checks after async gUM steps. r=jib
https://hg.mozilla.org/integration/autoland/rev/fe9563002b6f
Change init assert to runtime guard. r=jib
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.