942022 - SIGSEGV crash when running too many convolver nodes

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P1)

Description

[Karl Tomlinson (:karlt)]

Attachment: convolved-chord-2.html

STR:

1. ulimit -u 500 (or lower to reproduce faster)
2. load testcase
3. wait about 80 seconds.

#0  0x00007fd2396bd454 in __pthread_mutex_lock () from /lib64/libpthread.so.0
#1  0x00007fd235961a5a in Acquire (this=0x158)
    at /home/karl/moz/dev/ipc/chromium/src/base/lock.h:16
#2  AutoLock (lock=..., this=<synthetic pointer>)
    at /home/karl/moz/dev/ipc/chromium/src/base/lock.h:43
#3  MessageLoop::PostTask_Helper (this=0x0, from_here=..., 
    task=task@entry=0x7fcb406a3bd0, delay_ms=delay_ms@entry=0, 
    nestable=nestable@entry=true)
    at /home/karl/moz/dev/ipc/chromium/src/base/message_loop.cc:300
#4  0x00007fd235961ae0 in MessageLoop::PostTask (this=<optimized out>, 
    from_here=..., task=task@entry=0x7fcb406a3bd0)
    at /home/karl/moz/dev/ipc/chromium/src/base/message_loop.cc:253
#5  0x00007fd235017493 in WebCore::ReverbConvolver::ReverbConvolver (
    this=0x7fce44aa9b60, impulseResponseData=0x7fcb404ecce8, 
    impulseResponseLength=16384, renderSliceSize=128, 
    maxFFTSize=<optimized out>, convolverRenderPhase=0, 
    useBackgroundThreads=true)
    at /home/karl/moz/dev/content/media/webaudio/blink/ReverbConvolver.cpp:130
#6  0x00007fd235016a3f in WebCore::Reverb::initialize (
    this=this@entry=0x7fcbc1226230, impulseResponseBuffer=..., 
    impulseResponseBufferLength=impulseResponseBufferLength@entry=16384, 
    renderSliceSize=renderSliceSize@entry=128, 
    maxFFTSize=maxFFTSize@entry=32768, 
    numberOfChannels=numberOfChannels@entry=2, useBackgroundThreads=true)
    at /home/karl/moz/dev/content/media/webaudio/blink/Reverb.cpp:123
#7  0x00007fd235016c11 in WebCore::Reverb::Reverb (this=0x7fcbc1226230, 
    impulseResponse=<optimized out>, impulseResponseBufferLength=16384, 
    renderSliceSize=128, maxFFTSize=32768, numberOfChannels=2, 
    useBackgroundThreads=true, normalize=true, sampleRate=1.1479437e-40)
    at /home/karl/moz/dev/content/media/webaudio/blink/Reverb.cpp:105
#8  0x00007fd23500681c in mozilla::dom::ConvolverNodeEngine::AdjustReverb (
    this=0x21bfcf0)
    at /home/karl/moz/dev/content/media/webaudio/ConvolverNode.cpp:101
#9  0x00007fd234fd73f7 in mozilla::AudioNodeStream::Message::Run (
    this=<optimized out>)
    at /home/karl/moz/dev/content/media/AudioNodeStream.cpp:161
#10 0x00007fd234ff3942 in mozilla::MediaStreamGraphImpl::RunThread (
    this=0x276ca60)
    at /home/karl/moz/dev/content/media/MediaStreamGraph.cpp:1108
#11 0x00007fd234ff3db3 in mozilla::(anonymous namespace)::MediaStreamGraphInitThreadRunnable::Run (this=0x140e1a0)
    at /home/karl/moz/dev/content/media/MediaStreamGraph.cpp:1324
#12 0x00007fd235925c20 in nsThread::ProcessNextEvent (this=0x10110f0, 
    mayWait=<optimized out>, result=0x7fd0efffee0f)
    at /home/karl/moz/dev/xpcom/threads/nsThread.cpp:610
#13 0x00007fd2358f6ced in NS_ProcessNextEvent (thread=<optimized out>, 
    thread@entry=0x10110f0, mayWait=mayWait@entry=true)
    at /home/karl/moz/dev/xpcom/glue/nsThreadUtils.cpp:251
#14 0x00007fd2359263f1 in nsThread::ThreadFunc (arg=0x10110f0)
    at /home/karl/moz/dev/xpcom/threads/nsThread.cpp:248
#15 0x00007fd2384caadd in _pt_root (arg=0x2cc4170)
    at /home/karl/moz/dev/nsprpub/pr/src/pthreads/ptthread.c:204
#16 0x00007fd2396bae14 in start_thread () from /lib64/libpthread.so.0
#17 0x00007fd2387ce79d in clone ()
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S:115

We should handle thread creation failure, but apparently we are also not cleaning up old threads fast enough.

Comment 1

[Maire Reavy [:mreavy]]

Moving to Paul per our meeting yesterday.

Comment 2

[Paul Adenot (:padenot)]

Attachment: Return safely when thread creation fails, in ReverbConvolver.

Cleaning up the old thread happens on CC when the MediaStream is release, so
it's less straightforward for me to improve.

This merely prevents the SIGSEGV, and safely turns the node into a no-op.

Comment 3

[Karl Tomlinson (:karlt)]

Comment on attachment 8357764 [details] [diff] [review]
Return safely when thread creation fails, in ReverbConvolver.

>+bool ReverbConvolver::isValid() const
>+{
>+  return m_useBackgroundThreads && m_backgroundThread.IsRunning();
>+}

A ReverbConvolver with m_useBackgroundThreads set won't have a background
thread if RealtimeFrameLimit is not reached, so this will have false negatives.

Is there a need to do anything more here than issue a warning an skip posting the task?

m_backgroundStages will simply not be used if there is no thread to process them.  Processing the graph thread stages to use the leading portion of the IR buffer may actually be slightly better than outputting null, if that mattered.
Whatever is the least amount of code is best for handling failed thread creation.

Comment 4

[Paul Adenot (:padenot)]

Attachment: Return safely when thread creation fails, in ReverbConvolver.

Comment 5

[Paul Adenot (:padenot)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6a3a28e9c83d

Comment 6

[Paul Adenot (:padenot)]

(I've checked, and this is fixed in blink, so we don't have to ping them)

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6a3a28e9c83d