SIGSEGV crash when running too many convolver nodes

RESOLVED FIXED in mozilla29

Status

()

defect
P1
normal
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: karlt, Assigned: padenot)

Tracking

25 Branch
mozilla29
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

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.
Moving to Paul per our meeting yesterday.
Assignee: karlt → paul
(Assignee)

Comment 2

5 years ago
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.
Attachment #8357764 - Flags: review?(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.
Attachment #8357764 - Flags: review?(karlt) → review-
(Assignee)

Updated

5 years ago
Attachment #8357764 - Attachment is obsolete: true
(Reporter)

Updated

5 years ago
Attachment #8358398 - Flags: review?(karlt) → review+
(Assignee)

Comment 6

5 years ago
(I've checked, and this is fixed in blink, so we don't have to ping them)
https://hg.mozilla.org/mozilla-central/rev/6a3a28e9c83d
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.