Closed Bug 200058 Opened 19 years ago Closed 19 years ago
standalone images and plugins leak (the whole document leaks)
This is a regression from bug 90256. The stream listener holds a strong ref to the document (via nsRefPtr) and the document holds a strong ref to the listener (also via nsRefPtr). As a result the whole shebang (listener, document, all contents of document, etc) leaks. For nsImageDocument the fix is trivial -- just don't hold on to the listener at all. For nsPluginDocument, things are a little more complicated, since it uses the listener... Perhaps the document should drop its ref to the listener somewhere? e.g. in OnStopRequest? Or perhaps one of the refs can be made weak? Setting critical, since this is a pretty severe memory leak... Further, this regresses a similar leak which was recently fixed for nsImageDocument.
Here's a patch to clear and remove the reference to mDocument in nsMediaDocumentListener::OnStopRequest once we are done with it. This prevents the circular reference and the leak.
Comment on attachment 119076 [details] [diff] [review] patch v.1 (clears mDocument in onStop) Hmm.... to be safe, could you also drop the document's pointer to the listener when the script global object is set to null? That would guard against broken channel impls that fail to call OnStopRequest...
okay, here I clear out mStreamListener in SetScriptGlobalObject(0).
need to have |mStreamListener = nsnull| outside the |if (mImageRequest)|
Attachment #119113 - Attachment is obsolete: true
Comment on attachment 119116 [details] [diff] [review] patch v2.1 sr=jst (bz, feel free to sr too if you want to have one last look).
Attachment #119116 - Flags: superreview?(bzbarsky) → superreview+
Comment on attachment 119116 [details] [diff] [review] patch v2.1 r=me, though I still wonder why the image document is holding on to the listener at all...
Attachment #119116 - Flags: review?(jkeiser) → review+
good catch bz! The image document doesn't really need a pointer to the stream listener. I made that change and checked in the fix, thanks!
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.