Assertion failure: mStreams.Contains(aStream), at /builds/worker/workspace/build/src/dom/media/MediaStreamGraph.cpp:3535

RESOLVED FIXED in Firefox 68

Status

()

defect
P2
normal
RESOLVED FIXED
2 months ago
11 days ago

People

(Reporter: jkratzer, Assigned: pehrsons)

Tracking

(Blocks 1 bug, Regression, {assertion, testcase})

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

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox67 wontfix, firefox68 fixed)

Details

Attachments

(3 attachments)

Reporter

Description

2 months ago
Posted file testcase.html

Testcase found while fuzzing mozilla-central rev bbca68b2af26.

Assertion failure: mStreams.Contains(aStream), at /builds/worker/workspace/build/src/dom/media/MediaStreamGraph.cpp:3535

rax = 0x000056450a1f4e20 rdx = 0x00007f42b1b8f680
rcx = 0x0000000000000b40 rbx = 0x00007f42a2c11400
rsi = 0x00007f42bcc1c8b0 rdi = 0x00007f42bcc1b680
rbp = 0x00007f42a26b90f0 rsp = 0x00007f42a26b90d0
r8 = 0x00007f42bcc1c8b0 r9 = 0x00007f42a26ba700
r10 = 0x0000000000000000 r11 = 0x0000000000000000
r12 = 0x00007f42a26b90d8 r13 = 0x00007f42a282bfb0
r14 = 0x0000000000000006 r15 = 0x0000000000000000
rip = 0x00007f42add23bc8
OS|Linux|0.0.0 Linux 4.18.0-17-generic #18~18.04.1-Ubuntu SMP Fri Mar 15 15:27:12 UTC 2019 x86_64
CPU|amd64|family 6 model 94 stepping 3|1
GPU|||
Crash|SIGSEGV /SEGV_MAPERR|0x0|35
35|0|libxul.so|mozilla::MediaStreamGraphImpl::IncrementSuspendCount(mozilla::MediaStream*)|hg:hg.mozilla.org/mozilla-central:dom/media/MediaStreamGraph.cpp:bbca68b2af262ffbbf2e3a2d2e77a16c999f479a|3541|0x40
35|1|libxul.so|mozilla::MediaStreamGraphImpl::SuspendOrResumeStreams(mozilla::dom::AudioContextOperation, nsTArray<mozilla::MediaStream*> const&)|hg:hg.mozilla.org/mozilla-central:dom/media/MediaStreamGraph.cpp:bbca68b2af262ffbbf2e3a2d2e77a16c999f479a|3575|0x5
35|2|libxul.so|mozilla::MediaStreamGraphImpl::ApplyAudioContextOperationImpl(mozilla::MediaStream*, nsTArray<mozilla::MediaStream*> const&, mozilla::dom::AudioContextOperation, void*, mozilla::dom::AudioContextOperationFlags)|hg:hg.mozilla.org/mozilla-central:dom/media/MediaStreamGraph.cpp:bbca68b2af262ffbbf2e3a2d2e77a16c999f479a|3629|0x5
35|3|libxul.so|mozilla::MediaStreamGraphImpl::RunMessagesInQueue()|hg:hg.mozilla.org/mozilla-central:dom/media/MediaStreamGraph.cpp:bbca68b2af262ffbbf2e3a2d2e77a16c999f479a|1169|0x11
35|4|libxul.so|mozilla::MediaStreamGraphImpl::OneIterationImpl(long)|hg:hg.mozilla.org/mozilla-central:dom/media/MediaStreamGraph.cpp:bbca68b2af262ffbbf2e3a2d2e77a16c999f479a|1396|0x8
35|5|libxul.so|mozilla::AudioCallbackDriver::DataCallback(float const*, float*, long)|hg:hg.mozilla.org/mozilla-central:dom/media/GraphDriver.cpp:bbca68b2af262ffbbf2e3a2d2e77a16c999f479a|902|0xc
35|6|libxul.so|<futures::future::lazy::Lazy<F, R> as futures::future::Future>::poll|hg:hg.mozilla.org/mozilla-central:third_party/rust/futures/src/future/lazy.rs:bbca68b2af262ffbbf2e3a2d2e77a16c999f479a|82|0x73
35|7|libxul.so|std::panicking::try::do_call|hg:hg.mozilla.org/mozilla-central:third_party/rust/futures/src/future/catch_unwind.rs:bbca68b2af262ffbbf2e3a2d2e77a16c999f479a|49|0x5
35|8|libxul.so|__rust_maybe_catch_panic|||0x9
35|9|libxul.so|<futures_cpupool::MySender<F, core::result::Result<<F as futures::future::Future>::Item, <F as futures::future::Future>::Error>> as futures::future::Future>::poll|hg:hg.mozilla.org/mozilla-central:third_party/rust/futures-cpupool/src/lib.rs:bbca68b2af262ffbbf2e3a2d2e77a16c999f479a|325|0x96
35|10|libxul.so|futures::task_impl::std::set|hg:hg.mozilla.org/mozilla-central:third_party/rust/futures/src/future/mod.rs:bbca68b2af262ffbbf2e3a2d2e77a16c999f479a|113|0xa
35|11|libxul.so|futures::task_impl::std::Run::run|hg:hg.mozilla.org/mozilla-central:third_party/rust/futures/src/task_impl/std/mod.rs:bbca68b2af262ffbbf2e3a2d2e77a16c999f479a|450|0x68
35|12|libxul.so|futures_cpupool::Inner::work|hg:hg.mozilla.org/mozilla-central:third_party/rust/futures-cpupool/src/lib.rs:bbca68b2af262ffbbf2e3a2d2e77a16c999f479a|257|0x42
35|13|libxul.so|std::sys_common::backtrace::__rust_begin_short_backtrace|git:github.com/rust-lang/rust:src/libstd/sys_common/backtrace.rs:91856ed52c58aa5ba66a015354d1cc69e9779bdf|135|0x35
35|14|libxul.so|std::panicking::try::do_call|git:github.com/rust-lang/rust:src/libstd/thread/mod.rs:91856ed52c58aa5ba66a015354d1cc69e9779bdf|469|0x21
35|15|libxul.so|__rust_maybe_catch_panic|||0x9
35|16|libxul.so|<F as alloc::boxed::FnBox<A>>::call_box|git:github.com/rust-lang/rust:src/liballoc/boxed.rs:91856ed52c58aa5ba66a015354d1cc69e9779bdf|749|0xa4
35|17|libxul.so|thread_start|git:github.com/rust-lang/rust:src/libstd/sys/unix/thread.rs:91856ed52c58aa5ba66a015354d1cc69e9779bdf|81|0x84
35|18|libpthread-2.27.so||||0x76db
35|19|libc-2.27.so|clone|||0x3f

Flags: in-testsuite?

Andreas, do you have any thoughts on what is happening here?

Flags: needinfo?(apehrson)
Priority: -- → P2
Assignee

Comment 2

2 months ago

Not immediately, but if I can reproduce.

Assignee: nobody → apehrson
Flags: needinfo?(apehrson)
Assignee

Comment 3

2 months ago

This seems to be triggered by the next CC after running the testcase.

Assignee

Comment 4

2 months ago

NB that autoplay blocking must be disabled for the testcase to work.

Assignee

Updated

2 months ago
Status: NEW → ASSIGNED
Assignee

Comment 5

2 months ago

What's leading to this assertion failure is the sequence below (all for the same MediaStream):

MediaStream::mSuspendedCount = 1;
MSGImpl::DecrementSuspendCount() (mSuspendedCount = 0)
MSGImpl::IncrementSuspendCount() (mSuspendedCount = 1)
MSGImpl::DecrementSuspendCount() (mSuspendedCount = 0)
MSGImpl::DecrementSuspendCount() (mSuspendedCount = -1) (!!!)
MSGImpl::IncrementSuspendCount() (mSuspendedCount = 0)
MSGImpl::DecrementSuspendCount() (mSuspendedCount = -1)
MSGImpl::IncrementSuspendCount() (BOOM)

Before the suspended count goes from -1 to 0, IncrementSuspendCount() checks whether we were not suspended, and if so assumes we now became suspended. In this case this is wrong, and this is the first bug.

Because it assumed we became suspended, the stream was moved from mStreams to mSuspendedStreams. On the next DecrementSuspendCount nothing happens because the suspend state didn't change. However, on the following IncrementSuspendCount it is again assumed that the stream became suspended, but this time it cannot be moved from mStreams to mSuspendedStreams because it is already in mSuspendedStreams; it's a double-suspend, and the move has already happened.

In the sequence above, the double-DecrementSuspendCount that I marked with (!!!) is the second bug. This seems however fixed on m-c since this was filed. I'll focus on landing the test and fixing the first bug as a protection against future bugs that do something similar to the second bug I found here.

Assignee

Comment 7

2 months ago

We don't currently know any ways in which external code can make the
suspendCount negative, but this will keep us safe should a future bug enable it.

Depends on D28805

Comment 8

2 months ago
Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/158a9ef03153
Add crashtest. r=padenot
https://hg.mozilla.org/integration/autoland/rev/c0e459d13624
Avoid trying to double-suspend a MediaStream if external code makes the suspendCount negative. r=padenot

Comment 9

2 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.