Closed
Bug 256822
Opened 20 years ago
Closed 20 years ago
JS window objects leaked when opening / closing windows
Categories
(Toolkit :: Find Toolbar, defect, P1)
Toolkit
Find Toolbar
Tracking
()
RESOLVED
FIXED
mozilla1.7.4
People
(Reporter: dbaron, Assigned: dbaron)
Details
(Keywords: fixed-aviary1.0, memory-leak, regression, Whiteboard: [patch])
Attachments
(1 file)
1.38 KB,
patch
|
jst
:
review+
brendan
:
superreview+
chofmann
:
approval-aviary+
|
Details | Diff | Splinter Review |
Jesse, who was apparently running a Firefox branch debug build, pointed out to me that the DOMWINDOW numbers printed out in debug builds were increasing by two when opening new windows (as normal) but only decreasing by one when closing them. This is a regression and indicates a leak of the sort that will slow down the browser, since it's a leak that involves entraining lots more stuff in the JS garbage collector and thus slowing down each GC cycle. The fix is a trivial one liner.
Assignee | ||
Comment 1•20 years ago
|
||
Assignee | ||
Updated•20 years ago
|
Severity: normal → critical
Flags: blocking-aviary1.0PR?
Keywords: mlk,
regression
Priority: -- → P1
Target Milestone: --- → Firefox1.0beta
Assignee | ||
Updated•20 years ago
|
Attachment #156960 -
Flags: superreview?(brendan)
Attachment #156960 -
Flags: review?(firefox)
Attachment #156960 -
Flags: approval-aviary?
Assignee | ||
Comment 2•20 years ago
|
||
For the record, I found this by defining GC_MARK_DEBUG in jsgc.h, setting js_DumpGCHeap to stdout in jsgc.c, rebuilding in js/src/ and js/src/xpconnect/, and then starting FF and opening and closing a large number of windows. Then I could look at the traces from the GCs before the last bunch of DOMWINDOW printfs (and be able to distinguish the relevant leak trace by its repeated appearance -- once per window). The trace that showed up many times was: 0a0c7ed0 object 0x9efe980 ChromeWindow via nsXPCWrappedJS::mJSObj[nsIObserver,0xa6a17a8,0xa0d3610](Object).__proto__(Object).__parent__(ChromeWindow). Following the 0a0d3610 pointer (which comes from the special string passed to JS_AddNamedRoot in nsXPCWrappedJS::AddRef) and finding the first occurrence in the log yielded: 0a0d3610 object 0xa0e399c Object via nsXPCWrappedJS::mJSObj[nsIDOMEventListener,0xa6ab478,0xa0e7690](Function).__parent__(ChromeWindow).gTypeAheadFind(Object). which showed the name of the variable in which the object causing the entrainment lived. The entrainment was simply pref service -> observer implemented in JS -> observer's JS global object.
Assignee | ||
Comment 3•20 years ago
|
||
(In reply to comment #2) > (and be able to distinguish the relevant leak trace by its repeated appearance > -- once per window). The trace that showed up many times was: That should read: (and was able to distinguish the relevant leak trace by its repeated appearance -- once per window). The trace that showed up many times (with different addresses for each window) was:
Comment 4•20 years ago
|
||
Comment on attachment 156960 [details] [diff] [review] patch sr=brendan@mozilla.org, in advance of r=. /be
Attachment #156960 -
Flags: superreview?(brendan) → superreview+
Comment 5•20 years ago
|
||
Marking blocking-aviary1.0PR. /be
Flags: blocking-aviary1.0PR? → blocking-aviary1.0PR+
Comment 6•20 years ago
|
||
Comment on attachment 156960 [details] [diff] [review] patch a=chofmann for aviary branch
Attachment #156960 -
Flags: approval-aviary? → approval-aviary+
Comment 7•20 years ago
|
||
Comment on attachment 156960 [details] [diff] [review] patch r=jst
Attachment #156960 -
Flags: review?(firefox) → review+
Assignee | ||
Comment 8•20 years ago
|
||
Fix checked in to AVIARY_1_0_20040515_BRANCH, 2004-08-25 10:01 -0700. Leaving open pending trunk checkin of find toolbar.
Keywords: fixed-aviary1.0
Whiteboard: [patch]
Assignee | ||
Comment 9•20 years ago
|
||
And actually, before I did the stuff in comment 2, I set: XPCOM_MEM_BLOAT_LOG=bloat.log XPCOM_MEM_ALLOC_LOG=GlobalWindowImpl.alloc XPCOM_MEM_LOG_CLASSES=GlobalWindowImpl and ran a similar window opening test to verify (in GlobalWindowImpl.alloc) that the references held too long were actually coming from JS (which was my initial expectation).
Comment 10•20 years ago
|
||
Please change the Target Milestone from "Firefox1.0beta" to "Firefox1.1".
Assignee | ||
Comment 11•20 years ago
|
||
Presumably fixed when aviary branch landed on trunk, although the code moved in: ---------------------------- revision 1.296.2.3.2.123 date: 2004/10/16 06:24:33; author: blakeross%telocity.com; state: Exp; lines: +4 -507 Fix find toolbar bugs 250414, 251891, 250279. r=bryner a=me
Assignee | ||
Updated•20 years ago
|
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Product: Firefox → Toolkit
You need to log in
before you can comment on or make changes to this bug.
Description
•