Closed Bug 285065 Opened 15 years ago Closed 15 years ago

Firefox preferences window leaks global object due to radio.xml cycle

Categories

(Core :: XUL, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla1.8beta2

People

(Reporter: dbaron, Assigned: dbaron)

Details

(Keywords: memory-leak, Whiteboard: [patch])

Attachments

(2 files)

The Firefox preferences window leaks its global object (and thus that of the
window that opened it) because there's a cycle through C++ objects created by
radio.xml:  the |iterator| variable is in the scope that's visible to
|_filterRadioGroup|, which is owned by the iterator.

(I'm thinking somebody should do a memory leak audit of these widgets...)
Attached patch patchSplinter Review
Attachment #176533 - Flags: superreview?(bryner)
Attachment #176533 - Flags: review?(bryner)
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [patch]
Target Milestone: --- → mozilla1.8beta2
Attachment #176533 - Flags: superreview?(bryner)
Attachment #176533 - Flags: superreview+
Attachment #176533 - Flags: review?(bryner)
Attachment #176533 - Flags: review+
Fix checked in to trunk, 2005-03-06 20:36 -0800.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment on attachment 176533 [details] [diff] [review]
patch

>+          // XXX The function really should be global instead of local.
Do you mean as per attachment 134317 [details] [diff] [review]'s _filterRadioGroup method?
(In reply to comment #3)
> (From update of attachment 176533 [details] [diff] [review])
> >+          // XXX The function really should be global instead of local.
> Do you mean as per attachment 134317 [details] [diff] [review]'s _filterRadioGroup method?

Yes.
Attached patch Fix XXXSplinter Review
Attachment #176576 - Flags: superreview?(dbaron)
Attachment #176576 - Flags: review?(bryner)
Attachment #176576 - Flags: superreview?(dbaron) → superreview+
Attachment #176576 - Flags: review?(bryner) → review+
I actually patched revision 1.15 of tookit's radio.xml ;-)
See also bug 323534, "createTreeWalker can cause leaks due to cycles created by closures".
FWIW, bug 323534 removed the need for this patch.
You need to log in before you can comment on or make changes to this bug.