1125804 - crash in mozilla::AudioMixer::FinishMixing()

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Alice0775 White]

This bug was filed from the Socorro interface and is 
report bp-43ac012e-b8c5-4ea3-b798-f1c032150126.
=============================================================

Reproducible: 80% probability

Steps to reproduce:
1. Open http://moztw.org/foxmosa/game/pairs/ 
2. Start Play and complete the game
3. Open http://chrome.angrybirds.com/?version=standard&renderer=canvas

Actual Results:
Browser crashes

Comment 1

[Alice0775 White]

[Tracking Requested - why for this release]:

Comment 2

[Alice0775 White]

Slightly difficult to reproduce the crash.
But, the following regression window is probably correct because I tried 3 times each Step 2 and Step3 of comment#0.

Regression window
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=568cefaf88a9&tochange=1cd5483ff2fe

Triggered by: 1cd5483ff2fe	Paul Adenot — Bug 1059899 - When cubeb_stream_init fails, fallback to using a SystemClockDriver. r=jesup

Comment 3

[Alice0775 White]

(In reply to Alice0775 White from comment #2)
> Slightly difficult to reproduce the crash.
> But, the following regression window is probably correct because I tried 3
> times each Step 2 and Step3 of comment#0.
> 

Err:

Slightly difficult to reproduce the crash.
But, the following regression window is probably correct because I tried 3 times each Step1, Step2 and Step3 of comment#0.

Comment 4

[Lawrence Mandel [:lmandel] (use needinfo)]

Maire/Anthony - I'm not sure which of your teams should investigate. Can you help figure this one out?

Comment 5

[Maire Reavy [:mreavy]]

This one is mine.  It's MSG/web audio.  Padenot is the best person to look at this but he's at a web audio conference this week. Can this investigation wait until early next week when he's back?

Comment 6

[Lawrence Mandel [:lmandel] (use needinfo)]

(In reply to Maire Reavy [:mreavy] (Plz needinfo me) from comment #5)
> This one is mine.  It's MSG/web audio.  Padenot is the best person to look
> at this but he's at a web audio conference this week. Can this investigation
> wait until early next week when he's back?

Yes, although we're already up to Beta 6 next Monday so we don't have very much time to get a fix into Firefox 36.

Comment 7

[Paul Adenot (:padenot)]

This does not happen at all on 38, so I might have landed something that fixes it ? I'll keep the needinfo and investigate.

Comment 8

[Alice0775 White]

I can still reproduce the crash with the STR of comment #0 on latest Nightly38.
bp-ac807752-fab8-4082-b310-909352150129

https://hg.mozilla.org/mozilla-central/rev/6bfc0e1c4b29
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Firefox/38.0 ID:20150129030202

Comment 9

[Paul Adenot (:padenot)]

Attachment: patch

jesup, you reviewed the original patch, you get to review this one :-)

Basically, what happens here is that WASAPI sometimes fail to allocate a new stream if a _lot_ streams are currently active. The first game is likely to use <audio> for its sound effects, and each <audio> has a cubeb_stream (so that's a lot of cubeb_streams on the first game). When the angry birds game starts, it uses Web Audio API, so it tries to open a new cubeb_stream to run the MediaStreamGraph.

When running a graph off an audio thread, first thing the graph does is to add the AudioCallbackDriver as a mixer callback (on the main thread).

Then we fire the cubeb operation thread to ::Init the cubeb_stream (because it's a slow operation that can bock). In this particular case, this fails, and we fall back to a regular SystemClockDriver directly from the cubeb operation thread, but at this point we forgot to remove the MixerCallbackReceiver in the AudioMixer callback list. Then, the MSG tries to mix in audio data, and derefs an invalid pointer, because the AudioCallbackDriver is gone.

This patch simply removes the AudioCallbackDriver from the mixer list when it detects that it's going to fall back on a SystemClockDriver.

Comment 10

[Paul Adenot (:padenot)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0813db72fe3e

Comment 11

[Paul Adenot (:padenot)]

Comment on attachment 8559784 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: MSG refactoring (848954) +  Bug 1059899
[User impact if declined]: crash
[Describe test coverage new/current, TreeHerder]: unchanged
[Risks and why]: the fix is obvious, and is useful only on certain configuration, this is just a quite obvious programming error
[String/UUID change made/needed]: none

Comment 12

[Carsten Book [:Tomcat]]

sorry had to back this out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=6407224&repo=mozilla-inbound

Comment 13

[Paul Adenot (:padenot)]

Relanded, this was a warning-as-errors things.

Comment 14

[Paul Adenot (:padenot)]

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

Comment 15

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8559784 [details] [diff] [review]
patch

Taking it to make sure it is in beta 8

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a71f66ed990a

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/13bf0a02fa62

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1584db7257a6

Comment 19

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/projects/cypress/rev/6260a7224a2d