Closed Bug 1488832 Opened 2 years ago Closed 2 years ago

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

Categories

(Core :: WebRTC, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox-esr60 --- wontfix
firefox62 --- wontfix
firefox63 --- wontfix
firefox64 --- fixed

People

(Reporter: jkratzer, Assigned: pehrsons)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Crash Data

Attachments

(4 files)

Attached 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?
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
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)
(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
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+
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.
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+
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.