72019 - [MLK] Massive memory leak of buffers in ns4xPluginStreamListener::OnDataAvailable()

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Patrick C. Beard]

Every buffer that gets allocated in ns4xPluginStreamListener::OnDataAvailable() 
leaks. When testing the cache with the Quicktime plugin on the Mac, this 
generated a HUGE memory leak as it displayed a Quicktime movie. I have a patch, 
and this should probably go in to Mozilla 0.8.1, IMHO.

Comment 1

[Patrick C. Beard]

Attachment: Fix for memory leak and some additional error handling.

Comment 2

[Håkan Waara]

Some comments on the patch:

* maybe you should switch from PR_Free() to nsCRT::free() and PR_Malloc to
nsCRT::malloc()?

* please avoid goto's if possible

otherwise the patch looks good to me.

Comment 3

[Patrick C. Beard]

The goto is legitimate, to implement a common exit point for early returns. If we 
had a generic class for managing dynamically allocated buffers, this could be 
used and early returns wouldn't require a goto. I was interested in creating a 
minimal patch.

Comment 4

[Brian Nesse (gone)]

Personally, I'd use nsMemory instead of nsCRT. What it comes down to is we have 
too many ways to manipulate memory (IMO). And yes, breaks could be used instead 
of gotos, but the final code will be essentially the same.

The patch is solid and a distinct improvement over the rather severe leakage 
currently happening. r=bnesse.

Comment 5

[chris hofmann]

lets do it as soon as leaf is ready for checkin's

Comment 6

[av (gone)]

a=av

Comment 7

[Chris Waterson]

nice! sr=waterson

Comment 8

[Asa Dotzler [:asa]]

let's get this checked in  a=asa (on behalf of drivers)

Comment 9

[Patrick C. Beard]

So, who's checking this in to the trunk? Am I?

Comment 10

[av (gone)]

I've done this. Marking fixed.

Comment 11

[shrirang khanzode]

verif(rubberstamp)