Last Comment Bug 285065 - Firefox preferences window leaks global object due to radio.xml cycle
: Firefox preferences window leaks global object due to radio.xml cycle
Status: RESOLVED FIXED
[patch]
: mlk
Product: Core
Classification: Components
Component: XUL (show other bugs)
: Trunk
: All All
: P1 normal (vote)
: mozilla1.8beta2
Assigned To: David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
: John Morrison
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-03-06 19:22 PST by David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
Modified: 2006-03-12 18:22 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (3.12 KB, patch)
2005-03-06 19:23 PST, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bryner: review+
bryner: superreview+
Details | Diff | Review
Fix XXX (4.08 KB, patch)
2005-03-07 08:48 PST, neil@parkwaycc.co.uk
bryner: review+
dbaron: superreview+
Details | Diff | Review

Description David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2005-03-06 19:22:39 PST
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...)
Comment 1 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2005-03-06 19:23:44 PST
Created attachment 176533 [details] [diff] [review]
patch
Comment 2 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2005-03-06 20:39:27 PST
Fix checked in to trunk, 2005-03-06 20:36 -0800.
Comment 3 neil@parkwaycc.co.uk 2005-03-07 02:18:04 PST
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?
Comment 4 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2005-03-07 07:41:22 PST
(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.
Comment 5 neil@parkwaycc.co.uk 2005-03-07 08:48:12 PST
Created attachment 176576 [details] [diff] [review]
Fix XXX
Comment 6 neil@parkwaycc.co.uk 2005-03-08 04:28:56 PST
I actually patched revision 1.15 of tookit's radio.xml ;-)
Comment 7 Jesse Ruderman 2006-01-15 15:51:18 PST
See also bug 323534, "createTreeWalker can cause leaks due to cycles created by closures".
Comment 8 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2006-01-18 18:47:33 PST
FWIW, bug 323534 removed the need for this patch.

Note You need to log in before you can comment on or make changes to this bug.