978647 - Firefox freezes and RAM consumption reaches 2-3 GB

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[Alice0775 White]

Reported http://forums.mozillazine.org/viewtopic.php?p=13390013#p13390013

Steps To Reproduce:
1. Open http://whalesand23rds.wordpress.com/

Actual Results:
Firefox freezes and RAM consumption reaches 2-3 GB


Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/9ac7ed427cd2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 ID:20131203142913
Bad:
http://hg.mozilla.org/mozilla-central/rev/8187818246ad
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 ID:20131204003601
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9ac7ed427cd2&tochange=8187818246ad


Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/b96d513cd89f
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 ID:20131203164653
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/ccb5e249ab11
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 ID:20131203164918
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b96d513cd89f&tochange=ccb5e249ab11

Triggered by:
  ccb5e249ab11	Chris Pearce — Bug 945947 - Re-enable DirectShow for MP3 playback on Vista and later. r=ajones We disabled MP3 playback on Windows using DirectShow as the duration was being incorrectly reported. While DirectShow was disabled, we supported MP3 playback via Windows Media Foundation, and we still used DirectShow on WinXP. Now that bug 918135 has landed the duration calculation has been fixed, so we can re-enable DirectShow for MP3 playback for all Windows versions. This enables us to avoid suffering from bug 882537 for MP3 on Windows.


Setting media.directshow.enabled = false helps.

Comment 1

[Alice0775 White]

Stack on hang: bp-14edad38-6a6f-4e1f-9dc8-e36432140303
               bp-8d5602ae-cafe-4ff7-be1c-342a72140303

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Thanks for reporting this Alice!

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

OK, the problem here is the page is loading a zero length MP3 file in an <audio> element, and ParseMp3Headers() in DirectShowReader isn't detecting EOS properly (MediaResource::ReadAt() returning NS_OK and zero bytes read), so ParseMp3Headers() is getting stuck in a loop. 

We're also calling DispatchBytesConsumed in ChannelMediaResource::ReadAt() even if we've read 0 bytes. This instantiates a Runnable event, which is why we're crashing; we're allocating so many of these events that we're running out of memory.

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

Note: This crash affects Firefox 28, which is in beta and ships week of March 18.

The fix is pretty trivial, so we could get it uplifted.

Or, we could disable the DirectShow support in Firefox 28 (we will still get MP3 via the Windows Media Foundation backend, as we do in Firefox 27), and fix this in Firefox 29.

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch: handle EOS when looking for MP3 headers

Assume if we reach EOS without deciding a stream is MP3, that the stream must not be MP3.

I will follow up with tests once they've run through Try and other platforms have been tested. In the meantime, let's get this landed.

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

Landed fix for Firefox 30:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9401fbb063cb

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Could you consider a quick uplift for 28 (and 29)? thanks

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/9401fbb063cb

Comment 9

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8384416 [details] [diff] [review]
Patch: handle EOS when looking for MP3 headers

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 945947, where we switched from using Windows Media Foundation to using DirectShow for MP3 decoding.
User impact if declined: Browser can be out-of-memory'd pretty easy.
Testing completed (on m-c, etc.): Tested locally. It landed on m-i yesterday.
Risk to taking this patch (and alternatives if risky): Risk is low. Alternative is to revert to using Windows Media Foundation for MP3 decoding, but this has reliability issues.
String or IDL/UUID changes made by this patch: None.

Comment 10

[Chris Pearce [:cpearce (Not reading bugmail)]]

Landed in Aurora for Firefox 29.
https://hg.mozilla.org/releases/mozilla-aurora/rev/9745afb973d2

Comment 11

[Paul Silaghi, QA [:pauly]]

Reproduced in nightly 2014-03-03.
The page works fine in nightly 30.0a1(2014-03-04), win 7 x64.

Comment 12

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 8384416 [details] [diff] [review]
Patch: handle EOS when looking for MP3 headers

Let's get this into the last FF28 beta today.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/37e06d6ff4f8

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/37e06d6ff4f8

Comment 15

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #14)
> https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/37e06d6ff4f8

FYI, this patch only affects code on Windows only. So B2G is unaffected. ;)

Comment 16

[Catalin Varga [QA][:VarCat]]

Verified as fixed using the following environment:

FF 28.0b9
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0
Build Id: 20140306171728
Os: Win 7 x 64

the bug is not reproducing anymore.

Comment 17

[Paul Silaghi, QA [:pauly]]

Verified fixed 29.0a2 (2014-03-10), win 7 x64