Closed Bug 609814 Opened 12 years ago Closed 12 years ago

SVGDocumentWrapper never unregisters as a shutdown observer (causing leaks, effectively)

Categories

(Core :: SVG, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b8
Tracking Status
blocking2.0 --- final+

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

(Keywords: memory-leak)

Attachments

(1 file)

SVGDocumentWrapper objects register as a shutdown observers, so that they can sever some ties just before shutdown (since they get torn down as part of imagelib, but they own stuff that's in gklayout, and I think imagelib gets torn down after gklayout).

However -- bz just pointed out to me that SVGDocumentWrapper objects never *un*-register as shutdown observers.  We need them to unregister, so that if their images are cleaned up before shutdown (the usual case), they can be GC'd correctly.  Right now I guess the observer service holds a strong reference to them, which is keeping them alive forever.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
blocking2.0: --- → ?
Attached patch fixSplinter Review
This patch makes SVGDocumentWrapper unregister as a shutdown observer in its destructor.

To do that, we need to make it register as an observer using a weak reference (by passing PR_TRUE as the third param to AddObserver), so that it won't just live forever by virtue of being a shutdown-observer.
Attachment #488564 - Flags: review?(roc)
Keywords: mlk
Thanks for the review! Now just waiting on blocking-2.0+ before I can land this.  (roc, mind granting that?)
blocking2.0: ? → final+
Landed: http://hg.mozilla.org/mozilla-central/rev/3e6c3cf12c4b
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Depends on: 610748
It'd be cool if there were a way to test this kind of stuff.
Flags: in-testsuite?
I think that would require a new test framework (or an extension to an existing framework).  That may be coming soon, though (?) -- it sounds like Jesse has some ideas here:
 http://www.squarefree.com/2010/11/14/detecting-leak-until-shutdown-bugs/
You need to log in before you can comment on or make changes to this bug.