1256510 - Async shutdown timeout/crash "MediaStreamGraph shutdown: blocking on msg thread" from fuzzing

Status: RESOLVED FIXED

(Core :: Audio/Video: MediaStreamGraph, defect, P1)

Description

[Jesse Ruderman]

Attachment: testcase (leads to a shutdown hang)

(I tested without e10s on Mac)

1. Load the testcase
2. Quit


WARNING: Audio Buffer is not full by the end of the callback.: 'Available() == 0 || mSampleWriteOffset == 0', file dom/media/AudioBufferUtils.h, line 87

WARNING: At least one completion condition is taking too long to complete. Conditions: [{"name":"MediaStreamGraph shutdown: blocking on msg thread","state":"(none)","filename":"dom/media/MediaStreamGraph.cpp","lineNumber":2934,"stack":"MediaStreamGraph shutdown"}] Barrier: profile-before-change

FATAL ERROR: AsyncShutdown timeout in profile-before-change Conditions: [{"name":"MediaStreamGraph shutdown: blocking on msg thread","state":"(none)","filename":"dom/media/MediaStreamGraph.cpp","lineNumber":2934,"stack":"MediaStreamGraph shutdown"}] At least one completion condition failed to complete within a reasonable amount of time. Causing a crash to ensure that we do not leave the user with an unresponsive process draining resources.

###!!! ABORT: file dom/media/MediaStreamGraph.cpp, line 2934

Comment 1

[Jesse Ruderman]

Attachment: stack (minidump_stackwalk)

Comment 2

[Jesse Ruderman]

Attachment: stack (lldb) (with JS JITs disabled)

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Ok, the stakcs don't help (they're from the crash-when-you-take-too-long code).   I have reproed this locally (linux64 inbound) easily (Thanks Jesse!)

We have two MediaStreamGraphs in gGraphs.  Both are realtime.  One has 3 ports in mPortCount (the first, 0x...04d80).  The second has 0 ports (0x...04f40).  Only the first (04d80) gets shut down; the second never does.

Noteworthy: the first has this:
  mFirstCycleBreaker = 3, 
  mStateComputedTime = 216704, 
  mProcessedTime = 216704, 
  mLastMainThreadUpdate = {
    mValue = 2805456806066054
  }, 

the second has this:
  mFirstCycleBreaker = 3857049061, // 0xe5e5e5e5 aka uninitialized
  mStateComputedTime = 0, 
  mProcessedTime = 0, 
  mLastMainThreadUpdate = {
    mValue = 2805456806212637
  }, 

It appears the second has never run, and that would explain why it never processes the shutdown message.  Looking at it in rr...

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Attachment: MozReview Request: Bug 1256510: When shutting down MediaStreamGraphs, if the graph hasn't started, let it start and shut down r?padenot

Review commit: https://reviewboard.mozilla.org/r/39971/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39971/

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

Setting tracking flags (presumed since jib's code landed in 44)

Comment 6

[Paul Adenot (:padenot)]

Comment on attachment 8730527 [details]
MozReview Request: Bug 1256510: When shutting down MediaStreamGraphs, if the graph hasn't started, let it start and shut down r?padenot

https://reviewboard.mozilla.org/r/39971/#review36621

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d2144af8a3fe

Comment 8

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b82d97221ce4

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/405894fb1dd5

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/405894fb1dd5

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8730527 [details]
MozReview Request: Bug 1256510: When shutting down MediaStreamGraphs, if the graph hasn't started, let it start and shut down r?padenot

Approval Request Comment
[Feature/regressing bug #]: regressed in 44

[User impact if declined]: Shutdown hangs if you use certain sequences in WebAudio

[Describe test coverage new/current, TreeHerder]: a crashtest hits this (which caused the orange/backout initially), though crashtests apparently don't flag shutdown hangs that resolve.

[Risks and why]: quite low risk - a stopped MSG won't ever process the shutdown request, and starting it is a safe way to get it to do so (and avoids any races caused by trying to shut it down without starting it).

[String/UUID change made/needed]: none

Comment 12

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8730527 [details]
MozReview Request: Bug 1256510: When shutting down MediaStreamGraphs, if the graph hasn't started, let it start and shut down r?padenot

I confirmed with Jesup that he tested with Jesse's test case to ensure the shutdown hang is fixed. Aurora46+, Beta45+

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f95f7c876b15

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9ed563fd0535

Comment 15

[Liz Henry (:lizzard) (relman/hg->git project)]

This didn't make it into beta 2. but it should be in beta 4 to ship tomorrow.  
Here's hoping it will fix the topcrash in 1255737!