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

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Daniel Holbert [:dholbert]]

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.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix

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.

Comment 2

[Daniel Holbert [:dholbert]]

Thanks for the review! Now just waiting on blocking-2.0+ before I can land this.  (roc, mind granting that?)

Comment 3

[Daniel Holbert [:dholbert]]

Landed: http://hg.mozilla.org/mozilla-central/rev/3e6c3cf12c4b

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

It'd be cool if there were a way to test this kind of stuff.

Comment 5

[Daniel Holbert [:dholbert]]

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/