463999 - Leaks opening browser, loading URL with <audio> in it, then closing

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Jeff Walden [:Waldo]]

Attachment: Leak logs for the two URLs

Hours-old pull, last substantive change was 8-bit audio support.  Two different URLs giving leaks here, one more than the other, but both probably the same thing, so listing both here.  The URLs in question, however, are:

data:text/html,<audio%20controls%20src="http://ftp.mozilla.org/pub/mozilla/libraries/bonus-tracks/rheet.wav"%20style="width:100px;height:100px;background:lime;"></audio>

data:text/html,<audio%20controls%20src="ftp://ftp.mozilla.org/pub/mozilla.org/mozilla/libraries/bonus-tracks/rheet.wav"%20autoplay></audio>

The first leaks a lot more things, but at a glance I think that's just HTTP entraining more stuff.

Unfortunately Bugzilla wrapping completely destroys leak-log output, so I'm dumping the logs in an attachment for easier perusal, rather than leaving it in the comment for bugmail to pick it up.

Comment 1

[Matthew Gregan [:kinetik]]

Strange, both of the leaks include an nsOggDecoder.

Comment 2

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

I've run into this leak already tonight, it is caused by the nsOggDecoder being shutdown before its instantiated its mPlayer field. When the nsOggDecoder is shutdown, it assumes that mPlayer will free the channel reader, but if the decoder is shutdown before its decoded the meta data and instantiated the mPlayer field, there will be nothing to free the channel reader will, and it will leak.

We instansiate nsOggDecoder when the media element is added to the document, here's a stack:
>	gklayout.dll!nsOggDecoder::nsOggDecoder()  Line 1132	C++
gklayout.dll!nsHTMLMediaElement::PickMediaElement()  Line 622 + 0x24 bytes	C++
gklayout.dll!nsHTMLMediaElement::LoadWithChannel(nsIChannel * aChannel=0x00000000, nsIStreamListener * * aListener=0x00000000)  Line 200 + 0x8 bytes	C++
gklayout.dll!nsHTMLMediaElement::Load()  Line 161	C++
gklayout.dll!nsHTMLAudioElement::Load()  Line 64 + 0xc bytes	C++
gklayout.dll!nsHTMLMediaElement::DoneAddingChildren(int aHaveNotified=0x00000000)  Line 879	C++
gklayout.dll!SinkContext::CloseContainer(nsHTMLTag aTag=eHTMLTag_audio, int aMalformed=0x00000000)  Line 1016	C++
gklayout.dll!HTMLContentSink::CloseContainer(nsHTMLTag aTag=eHTMLTag_audio)  Line 2385 + 0x11 bytes	C++
gkparser.dll!CNavDTD::CloseContainer(nsHTMLTag aTag=eHTMLTag_audio, int aMalformed=0x00000000)  Line 2800 + 0x26 bytes	C++
gkparser.dll!CNavDTD::CloseContainersTo(int anIndex=0x00000002, nsHTMLTag aTarget=eHTMLTag_audio, int aClosedByStartTag=0x00000000)  Line 2848 + 0xe bytes	C++
gkparser.dll!CNavDTD::CloseContainersTo(nsHTMLTag aTag=eHTMLTag_audio, int aClosedByStartTag=0x00000000)  Line 2992 + 0x14 bytes	C++
gkparser.dll!CNavDTD::HandleEndToken(CToken * aToken=0x0499af90)  Line 1764 + 0xe bytes	C++
gkparser.dll!CNavDTD::HandleToken(CToken * aToken=0x0499af90, nsIParser * aParser=0x0480ea38)  Line 760 + 0xc bytes	C++
gkparser.dll!CNavDTD::BuildModel(nsIParser * aParser=0x0480ea38, nsITokenizer * aTokenizer=0x049afea8, nsITokenObserver * anObserver=0x00000000, nsIContentSink * aSink=0x047d2c7c)  Line 332 + 0x16 bytes	C++
gkparser.dll!CNavDTD::BuildNeglectedTarget(nsHTMLTag aTarget=eHTMLTag_body, eHTMLTokenTypes aType=eToken_start, nsIParser * aParser=0x0480ea38, nsIContentSink * aSink=0x047d2c7c)  Line 378	C++
gkparser.dll!CNavDTD::DidBuildModel(unsigned int anErrorCode=0x00000000, int aNotifySink=0x00000001, nsIParser * aParser=0x0480ea38, nsIContentSink * aSink=0x047d2c7c)  Line 401	C++
gkparser.dll!nsParser::DidBuildModel(unsigned int anErrorCode=0x00000000)  Line 1527 + 0x35 bytes	C++
gkparser.dll!nsParser::ResumeParse(int allowIteration=0x00000001, int aIsFinalChunk=0x00000001, int aCanInterrupt=0x00000001)  Line 2288	C++
gkparser.dll!nsParser::OnStopRequest(nsIRequest * request=0x00e00c10, nsISupports * aContext=0x00000000, unsigned int status=0x00000000)  Line 2927 + 0x17 bytes	C++
docshell.dll!nsDocumentOpenInfo::OnStopRequest(nsIRequest * request=0x00e00c10, nsISupports * aCtxt=0x00000000, unsigned int aStatus=0x00000000)  Line 324	C++
necko.dll!nsBaseChannel::OnStopRequest(nsIRequest * request=0x06dbef20, nsISupports * ctxt=0x00000000, unsigned int status=0x00000000)  Line 678	C++
necko.dll!nsInputStreamPump::OnStateStop()  Line 577	C++
necko.dll!nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream * stream=0x063275b0)  Line 401 + 0xb bytes	C++
xpcom_core.dll!nsInputStreamReadyEvent::Run()  Line 112	C++
xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=0x00000001, int * result=0x0026f12c)  Line 511	C++

That then goes onto create an nsChannelReader, which will leak as described above.

nsHTMLMediaElement::PickMediaElement() is getting a src attribute of "" and instantiating the ogg decoder by default. We should probably not do that, as well as fixing the shutdown problem outlined above.

Comment 3

[Matthew Gregan [:kinetik]]

Oh right, of course.  Since we don't do media type switching for the src attribute, we end up in PickMediaElement, which always instantiates an nsOggDecoder.

(This is also why the file doesn't play back, we're trying to play it using the Ogg backend.  You need to use a <source> child with type="audio/x-wav", for now.)

Comment 4

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

(In reply to comment #2)
> We should probably not do that, as
> well as fixing the shutdown problem outlined above.

Shutdown stuff is already filed in bug 462878.

Comment 5

[Matthew Gregan [:kinetik]]

Now that bug 462878 has landed, this leak should be fixed.

I just tried, and can't reproduce it.  Jeff, can you please retest and confirm?  Thanks.

Comment 6

[Jeff Walden [:Waldo]]

Loading either link and immediately shutting down no longer leaks for me, good stuff.

We should add a crashtest, but I'm not sure how such tests work when they might play sounds -- don't think that's so great for a test suite, in an ideal world.