1134977 - Sound playback stops after playing game for a while, and Firefox.exe process remains forever after quit browser

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[Alice0775 White]

+++ This bug was initially created as a clone of Bug #1134956 +++

User Agent:
m-c tinderbox build which is including Bug 1133190 patch.
https://hg.mozilla.org/mozilla-central/rev/56f090df5480
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Firefox/38.0 ID:20150219155341

Sound playback stops after playing game for a while, and Firefox.exe process remains forever after quit browser.


This does not happen on Aurora37.0a2 and Beta36.0b10.
https://hg.mozilla.org/releases/mozilla-aurora/rev/bfbf0f731f52
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:37.0) Gecko/20100101 Firefox/37.0 ID:20150219175713
https://hg.mozilla.org/releases/mozilla-beta/rev/9e2391999085
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0 ID:20150217132925

Steps to reproduce:

1. Open https://developer.mozilla.org/en-US/demos/detail/bananabread and Launch Game
2. Click [PLAY:LEVEL1], Click [LOW RESOLUTION]
3. Play for a few minutes

Actual Results:
Sound playback stops
Firefox.exe process remains forever after quit browser

Actual Results:
Sound should play
Firefox.exe process should disappear after quit browser

(Note: browser will crash on Nightly(2015-02-19) due to Bug 1133190)

Comment 1

[Alice0775 White]

Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9e46d10862f7&tochange=9efd544f112f

Regressed by: Bug 698079

Comment 2

[Luke]

I'm also experiencing this issue on Win 7 with FF 38.0a1 (2015-02-21). All sounds stop playing until I restart FF.

Comment 3

[Luke]

Because of Bug 1134956 it's tough to mozregression, but here's what I came up with:

MainThread Bisector INFO Narrowed nightly regression window from [
2015-01-17, 2015-01-20] (3 days) to [2015-01-19, 2015-01-20] (1 days) (~0 steps
left)
MainThread Bisector INFO Got as far as we can go bisecting nightli
es...
MainThread Bisector INFO Last good revision: 6446c26b45f9
MainThread Bisector INFO First bad revision: 93f526d85b47
MainThread Bisector INFO Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6446c26b45f9&tocha
nge=93f526d85b47

Comment 4

[Alice0775 White]

Regression due to Bug 698079, See comment #1

Comment 5

[Matthew Gregan [:kinetik]]

I can't repro in a current debug build of m-i on my Windows 8.1 machine.  How long are you playing for before you lose audio?  I played 2-3 5 min sessions without hitting any issues, but the game does run pretty slowly in a debug build, so that may prevent bug from presenting itself.

Any chance you can grab a stack trace of all threads when the shutdown hang occurs?

Comment 6

[Alice0775 White]

Press left mouse button and hold it, the sound problem occurs within 10-15 sec.
And 100% reproducible.

Stack from crashfirefox.exe:
bp-2c056c1b-9e08-42a6-b0e3-123aa2150223
Reproducing the shutdown hang is slightly difficult on today's Nightly(2015-02-22).

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


I tried on windows8.1, however, not yet succeeded to reproduce the sound problem.

Comment 7

[Alice0775 White]

Attachment: log.zip

NSPR_LOG log

NSPR_LOG_MODULES=timestamp,MediaDecoder:5,MediaSource:5,MediaPromise:5,MP4Demuxer:5

Comment 8

[Alice0775 White]

Attachment: log_stdrd.txt

stdard out/error log

Comment 9

[Paul Adenot (:padenot)]

This is weird: 

> Assertion failure: mState == FAILED || mState == STARTED || mState == CLONED (State invariants 
> violated), at c:\builds\moz2_slave\m-cen-w32-d-000000000000000000\build\src\widget\windows
> \AudioSession.cpp:346

Comment 10

[Matthew Gregan [:kinetik]]

Anthony's got a Windows 7 machine in the office and offer to try reproducing this.

Comment 11

[Luke]

At the end of every sound effect, there's a popping or clicking noise. For example go to any level without bots and compare the jumping noise in Firefox to Chrome. Chrome sounds exactly like Sauerbraten without a pop at the end. Is this related to this bug, Bug 1134263, or a separate issue?

Comment 12

[Paul Adenot (:padenot)]

That might be because this game is using <audio>, and we're creating double the threads, now (one for the notification of plug/unplug, one for the actual audio). Actually, for each <audio>, we need like 3-4 threads or something, so it's not going to work well anyways.

A way forward would be to only have one IMMNotificationClient for all cubeb_streams. This has the problem of having to keep track of all the cubeb_streams somewhere, so we can tell them to close and reopen on device change, which means adding a global lock.

Anthony, I don't know which machine you're going to use to try to repro this, but try to get the slowest and least updated machine you can (old audio drivers and all). My experience is that the W520/30/40 thinkpads we have never seem to run out of CPU, and collect threads and audio streams (on the server side) super quickly, so it's hard to repro. I use an old dell machine to repro this, but I can't repro the shutdown hang, only the sound stopping. I also can't reproduce the popping.

Comment 13

[Luke]

The popping noise is a separate issue. Opened Bug 1136994 and did a mozregression. I don't think it's a CPU issue as I can reproduce it on both my i7 Desktop and i5 laptop.  I can't hear it clearly on my laptop speakers, so you do need headphones to reproduce it.

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=464bca437658&tochange=be076357691c

Comment 14

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

I can reproduce the sound stopping but I don't get the shutdown hang.

Comment 15

[Lukas Blakk [:lsblakk] use ?needinfo]

Tracking this regression, Paul will the assignee be you or Anthony?

Comment 16

[Paul Adenot (:padenot)]

I think we pretty much know how to fix this (second paragraph of comment 12), we just need to decide who is going to do it. I can probably take this, but feel free to steal it.

Comment 17

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

Marking this for verification once we have a fix.

Comment 20

[Matthew Gregan [:kinetik]]

(In reply to Paul Adenot (:padenot) from comment #16)
> I think we pretty much know how to fix this (second paragraph of comment
> 12), we just need to decide who is going to do it. I can probably take this,
> but feel free to steal it.

Missed this comment, but I'm stealing this in bug 1145038.

Comment 21

[Matthew Gregan [:kinetik]]

This is a simple bug and fix, but took a while to spot because we had already fixed it in bug 1109802 and thus I had a blind spot for it.  It turns out that part of the refactoring in bug 698079 reverted bug 1109802's fix.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8ae2f49d8829

Comment 22

[Matthew Gregan [:kinetik]]

Attachment: bug1134977_v0.patch

Going ahead with requesting uplift for this immediately, since the fix is tiny/trivial and had already been made in bug 1109802.

Approval Request Comment
[Feature/regressing bug #]: bug 698079
[User impact if declined]: leak audio resource every time an audio stream is closed
[Describe test coverage new/current, TreeHerder]: n/a
[Risks and why]: extremely low, we had this fix before but it was regressed in a refactoring
[String/UUID change made/needed]: none

Comment 23

[Paul Adenot (:padenot)]

Comment on attachment 8586540 [details] [diff] [review]
bug1134977_v0.patch

Review of attachment 8586540 [details] [diff] [review]:
-----------------------------------------------------------------

Hrm, this is embarrassing. I have half a patch to convert the wasapi backend to use auto pointers, I should probably just finish it.

Comment 24

[Matthew Gregan [:kinetik]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/513590a0b360

Comment 26

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/513590a0b360

Comment 27

[Sylvestre Ledru [:Sylvestre]]

Should be in 38 beta 2

Comment 28

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8e6e3826c2cd

Comment 29

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8348e6654b30

Comment 30

[Petruta Horea [:phorea], Desktop QA]

I could reproduce only the sound issue on Nightly 39.0a1, Win 7 64-bit. This didn't occur with Firefox 38 beta 3, the game .

I had the message "failed to load sample: packages/sounds/soundsnap/chainsaw_idle.ogg" on screen after playing for some time, but I don't think this is related.

Alice, can you check if the process is closed after quitting Firefox?

Comment 31

[Alice0775 White]

No the shutdown hang. WFM

Comment 32

[Petruta Horea [:phorea], Desktop QA]

Thank you!

Comment 33

[Camelia Badau [:cbadau], Desktop QA]

I could reproduce the sound issue on Nightly 39.0a1, Win 7 64bit. 
I've tested with Firefox 39 Beta 3 (build ID: 20150604162752) and Firefox 40.0a2 (buildID: 20150608004126) and the sound issue is fixed. No shutdown hang.