1525323 - Assertion failure: false (A non-finished SourceMediaStream wasn't fed enough data by NotifyPull), at /builds/worker/workspace/build/src/dom/media/MediaStreamGraph.cpp:1254

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P2)

Description

[Jason Kratzer [:jkratzer]]

Attachment: testcase.html

Assertion failure: false (A non-finished SourceMediaStream wasn't fed enough data by NotifyPull), at /builds/worker/workspace/build/src/dom/media/MediaStreamGraph.cpp:1254

Testcase found while fuzzing mozilla-central rev 52b73d447c52. Testcase must be served via a local webserver in order to reproduce.

==7337==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x7f05fda2a577 bp 0x7f05dfe0dd90 sp 0x7f05dfe0dae0 T38)
==7337==The signal is caused by a WRITE memory access.
==7337==Hint: address points to the zero page.
#0 0x7f05fda2a576 in mozilla::MediaStreamGraphImpl::UpdateGraph(long) /builds/worker/workspace/build/src/dom/media/MediaStreamGraph.cpp:1252:13
#1 0x7f05fda2d8b2 in mozilla::MediaStreamGraphImpl::OneIteration(long) /builds/worker/workspace/build/src/dom/media/MediaStreamGraph.cpp:1395:3
#2 0x7f05fd6f374a in mozilla::ThreadedDriver::RunThread() /builds/worker/workspace/build/src/dom/media/GraphDriver.cpp:306:41
#3 0x7f05fd7251a2 in mozilla::MediaStreamGraphInitThreadRunnable::Run() /builds/worker/workspace/build/src/dom/media/GraphDriver.cpp:204:14
#4 0x7f05f515e1c6 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1162:14
#5 0x7f05f5165f8d in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:474:10
#6 0x7f05f6410431 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:303:20
#7 0x7f05f62fb83e in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
#8 0x7f05f62fb83e in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
#9 0x7f05f62fb83e in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
#10 0x7f05f5156663 in nsThread::ThreadFunc(void*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:449:11
#11 0x7f061a2dd666 in _pt_root /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:201:5
#12 0x7f0619f216da in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76da)
#13 0x7f0618eff88e in clone /build/glibc-OTsEL5/glibc-2.27/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /builds/worker/workspace/build/src/dom/media/MediaStreamGraph.cpp:1252:13 in mozilla::MediaStreamGraphImpl::UpdateGraph(long)
Thread T38 (MediaStreamGrph) created by T36 (AudioIPC1) here:
#0 0x556f2a8306bd in __interceptor_pthread_create /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:210:3
#1 0x7f061a2da395 in _PR_CreateThread /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:433:14
#2 0x7f061a2d9f7e in PR_CreateThread /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:518:12
#3 0x7f05f51595b9 in nsThread::Init(nsTSubstring<char> const&) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:655:8
#4 0x7f05f5164c45 in nsThreadManager::NewNamedThread(nsTSubstring<char> const&, unsigned int, nsIThread**) /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp:410:12
#5 0x7f05f5169b44 in NS_NewNamedThread(nsTSubstring<char> const&, nsIThread**, nsIRunnable*, unsigned int) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:127:57
#6 0x7f05fd6f255c in NS_NewNamedThread<16> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:71:10
#7 0x7f05fd6f255c in mozilla::ThreadedDriver::Start() /builds/worker/workspace/build/src/dom/media/GraphDriver.cpp:222
#8 0x7f05fd6f1695 in mozilla::GraphDriver::SwitchToNextDriver() /builds/worker/workspace/build/src/dom/media/GraphDriver.cpp:103:17
#9 0x7f05fd6fbc94 in mozilla::AudioCallbackDriver::DataCallback(float const*, float*, long) /builds/worker/workspace/build/src/dom/media/GraphDriver.cpp:942:5
#10 0x7f06062f7a98 in $LT$audioipc_client..stream..CallbackServer$u20$as$u20$audioipc..rpc..server..Server$GT$::process::$u7b$$u7b$closure$u7d$$u7d$::h28dd4e3894b83022 /builds/worker/workspace/build/src/media/audioipc/client/src/stream.rs:110:24
#11 0x7f06062f7a98 in _$LT$futures..future..lazy..Lazy$LT$F$C$$u20$R$GT$$GT$::get::h104328d61243c4d6 /builds/worker/workspace/build/src/third_party/rust/futures/src/future/lazy.rs:64
#12 0x7f06062f7a98 in $LT$futures..future..lazy..Lazy$LT$F$C$$u20$R$GT$$u20$as$u20$futures..future..Future$GT$::poll::hd23a29becfa61583 /builds/worker/workspace/build/src/third_party/rust/futures/src/future/lazy.rs:82
#13 0x7f06062f7a98 in futures::future::catch_unwind::$LT$impl$u20$futures..future..Future$u20$for$u20$std..panic..AssertUnwindSafe$LT$F$GT$$GT$::poll::h0850ffa648a463c4 /builds/worker/workspace/build/src/third_party/rust/futures/src/future/catch_unwind.rs:49
#14 0x7f06062f7a98 in $LT$futures..future..catch_unwind..CatchUnwind$LT$F$GT$$u20$as$u20$futures..future..Future$GT$::poll::$u7b$$u7b$closure$u7d$$u7d$::h6c033e1ce6ef5b31 /builds/worker/workspace/build/src/third_party/rust/futures/src/future/catch_unwind.rs:32
#15 0x7f06062f7a98 in std::panicking::try::do_call::h3a4127862ee7f453 /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libstd/panicking.rs:310
#16 0x7f06062f7a98 in __rust_maybe_catch_panic /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libpanic_abort/lib.rs:39
#17 0x7f06062f7a98 in std::panicking::try::h09f95521e16c9c6b /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libstd/panicking.rs:289
#18 0x7f06062f7a98 in std::panic::catch_unwind::h7b9bd9b48aa68a0a /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libstd/panic.rs:398
#19 0x7f06062f7a98 in _$LT$futures..future..catch_unwind..CatchUnwind$LT$F$GT$$u20$as$u20$futures..future..Future$GT$::poll::h32cba86a96c825b5 /builds/worker/workspace/build/src/third_party/rust/futures/src/future/catch_unwind.rs:32
#20 0x7f06062f7a98 in _$LT$futures_cpupool..MySender$LT$F$C$$u20$core..result..Result$LT$$LT$F$u20$as$u20$futures..future..Future$GT$..Item$C$$u20$$LT$F$u20$as$u20$futures..future..Future$GT$..Error$GT$$GT$$u20$as$u20$futures..future..Future$GT$::poll::ha949fea50d0b6a22 /builds/worker/workspace/build/src/third_party/rust/futures-cpupool/src/lib.rs:325

Thread T36 (AudioIPC1) created by T0 (file:// Content) here:
#0 0x556f2a8306bd in __interceptor_pthread_create /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:210:3
#1 0x7f060690e5a6 in std::sys::unix::thread::Thread::new::h6179c0ba07009a42 /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libstd/sys/unix/thread.rs:78:18

==7337==ABORTING

Comment 1

[Nils Ohlmeier [:drno]]

Andreas could you please have a look if this is related to recent refactoring, or a long standing issue?

Comment 2

[Andreas Pehrson [:pehrsons]]

It's probably from recent fixes. I'll take it.

Comment 3

[Andreas Pehrson [:pehrsons]]

This needs media.navigator.permission.disabled set to true to reproduce.

Comment 4

[Andreas Pehrson [:pehrsons]]

It's a pulled track that doesn't have a listener attached. In this case I keep hitting it for the fake audio track, but that's probably irrelevant and could happen on any gUM-track. Highly likely that it's from a recent change.

Comment 5

[Andreas Pehrson [:pehrsons]]

For controlling these streams and tracks we send messages from main thread.

Here we have a race because initialization looks like this (thread in parens) (two paths):
GUMRunnable (main) -> InitializeAsync() (media) -> SetPullingEnabled(true) (main)
GUMRunnable (main) -> Activate() (main) -> AddTrackListener() (no thread hop)

While the shutdown sequence triggering this looks like (two paths):
OnNavigation() (main) -> Stop() (media) -> SetPullingEnabled(false) (main)
OnNavigation() (main) -> Remove() (main) -> RemoveTrackListener()/SetPullingEnabled(false) (main) (no thread hop)

So the order of these events when we crash is (times are from my run's log):

• GUMRunnable
• Activate (10:37:09.691115)
  • AddTrackListener
• OnNavigation (10:37:09.708155)
• Remove (10:37:09.709060)
  • RemoveTrackListener
  • SetPullingEnabled(false) (futile)
• InitializeAsync (10:37:12.244697)
• SetPullingEnabled(true)
• Boom

Comment 6

[Andreas Pehrson [:pehrsons]]

I think the best way forward here is to give control of the track listeners to MediaEngineSources -- and only the ones that need it.

It would be nice to wait for bug 1423253 since it removes the need for pulling (i.e., the need for the listener altogether) from all sources except the microphone one. Perhaps we don't need to pull the microphone either, but it does help us to assert that we're adding the expected amount of data.

Comment 7

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1525323 - Remove SourceTrackListener from MediaManager. r?padenot

This moves the responsibility of forwarding NotifyPull() from the graph thread
to MediaEngineSources out of MediaManager and into the sources themselves. This
is better aligned with how the sources work, since not all sources need pulling.
This also clarifies lifetime management of these listeners in relation to when
pulling is enabled for a track, since the sources are already handling enabling
pulling themselves.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1f79ed91a20b
Remove SourceTrackListener from MediaManager. r=padenot

Comment 9

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/1f79ed91a20b

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Is it possible to land this testcase as a crashtest?

Comment 11

[Andreas Pehrson [:pehrsons]]

I tried but this testcase is one of those that need to spin at location.reload() for several seconds before hitting the crash, and I have never managed to convert one of those to a crash test. I think I even tried to put the testcase in an iframe so it could reload itself and have the parent window cancel it after a timeout, but either it failed to repro or I didn't get that cancellation to work.

Comment 12

[Andreas Pehrson [:pehrsons]]

I found this diagnostic assert on crash-stats existing in the wild. That crash is the non-debug version. Shows for 67 and earlier only.