Closed
Bug 109671
Opened 23 years ago
Closed 23 years ago
leak pres context of second window (and later windows)
Categories
(Core :: XUL, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla0.9.7
People
(Reporter: dbaron, Assigned: dbaron)
References
Details
(Keywords: memory-leak)
Attachments
(1 file, 1 obsolete file)
4.05 KB,
patch
|
bugs
:
review+
brendan
:
superreview+
|
Details | Diff | Splinter Review |
We leak the pres context of the second navigator window that's opened, and I think all later windows. (If one leaks a pres context and then focuses anything inside that pres context, the document is leaked. This means we leak the entire XUL document if the window gets focus.) Steps to reproduce: 1) ./mozilla -P Mozilla 2) Click in blank area of homepage 3) Ctrl-N [optional: click in blank area of homepage to see entire XUL document leak] 4) X in corner of 2d window 5) X in corner of 1st window
Assignee | ||
Comment 1•23 years ago
|
||
The nsPresContext is being leaked from an imgRequestProxy. The leaked reference is put into the nsCOMPtr here: nsCOMPtr_base::assign_assuming_AddRef(nsISupports *)+0x00000037 [/builds/cssorg/obj/debug/dist/bin/libxpcom.so +0x0013FD8F] nsCOMPtr_base::assign_with_AddRef(nsISupports *)+0x0000003C [/builds/cssorg/obj/debug/dist/bin/libxpcom.so +0x0013132C] nsCOMPtr<nsISupports>::operator=(nsISupports *)+0x00000021 [/builds/cssorg/obj/debug/dist/bin/libxpcom.so +0x0013FAB5] imgRequestProxy::Init(imgRequest *, nsILoadGroup *, imgIDecoderObserver *, nsISupports *)+0x0000012A [/builds/cssorg/obj/debug/dist/bin/components/libimglib2.so +0x0001D182] imgLoader::CreateNewProxyForRequest(imgRequest *, nsILoadGroup *, imgIDecoderObserver *, nsISupports *, unsigned int, imgIRequest *, imgIRequest **)+0x00000143 [/builds/cssorg/obj/debug/dist/bin/components/libimglib2.so +0x000194D7] imgLoader::LoadImage(nsIURI *, nsILoadGroup *, imgIDecoderObserver *, nsISupports *, unsigned int, nsISupports *, imgIRequest *, imgIRequest **)+0x0000132C [/builds/cssorg/obj/debug/dist/bin/components/libimglib2.so +0x00018CBC] nsImageBoxFrame::UpdateImage(nsIPresContext *, int &)+0x00000564 [/builds/cssorg/obj/debug/dist/bin/components/libgklayout.so +0x0029F904] nsImageBoxFrame::DidSetStyleContext(nsIPresContext *)+0x00000026 [/builds/cssorg/obj/debug/dist/bin/components/libgklayout.so +0x0029FC92] nsIFrame::SetStyleContext(nsIPresContext *, nsIStyleContext *)+0x00000078 [/builds/cssorg/obj/debug/dist/bin/components/libgklayout.so +0x0034E1CC] nsFrame::Init(nsIPresContext *, nsIContent *, nsIFrame *, nsIStyleContext *, nsIFrame *)+0x000000FB [/builds/cssorg/obj/debug/dist/bin/components/libgklayout.so +0x0017E803] nsLeafBoxFrame::Init(nsIPresContext *, nsIContent *, nsIFrame *, nsIStyleContext *, nsIFrame *)+0x00000031 [/builds/cssorg/obj/debug/dist/bin/components/libgklayout.so +0x002AA425] nsImageBoxFrame::Init(nsIPresContext *, nsIContent *, nsIFrame *, nsIStyleContext *, nsIFrame *)+0x00000105 [/builds/cssorg/obj/debug/dist/bin/components/libgklayout.so +0x0029F195] This reminds me of bug 87040.
Assignee | ||
Comment 2•23 years ago
|
||
Hrm. And we're leaking |nsFrame|s
Assignee | ||
Comment 3•23 years ago
|
||
The root of the leaked subtree of frames is an nsMenuPopupFrame that is a child of an nsBoxFrame. That nsBoxFrame has one other sibling, an nsPlaceHolderFrame.
Assignee | ||
Comment 4•23 years ago
|
||
Could it be being leaked because the nsRootBoxFrame has a null mPopupSetFrame? It is null -- the question is whether the Destroy through the popup set the only way Destroy is supposed to be called on popups.
Assignee | ||
Updated•23 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Updated•23 years ago
|
Priority: -- → P1
Target Milestone: --- → mozilla0.9.7
Blocks: 110144
Assignee | ||
Comment 5•23 years ago
|
||
The basic problem here seems to be ordering: GetPopupSetFrame, this=0x8938a18, popupset=(nil). ###!!! ASSERTION: unexpected null pointer: 'popupSetFrame != 0', file /builds/trunk/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp, line 5675 ###!!! Break: at file /builds/trunk/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp, line 5675 ^GWEBSHELL+ = 5 SetPopupSetFrame, this=0x8938a18, popupset=0x89e1308. GetPopupSetFrame, this=0x8938a18, popupset=0x89e1308.
Assignee | ||
Comment 6•23 years ago
|
||
Assignee | ||
Comment 7•23 years ago
|
||
Attachment #58787 -
Attachment is obsolete: true
Comment 8•23 years ago
|
||
sr=hyatt
Updated•23 years ago
|
Attachment #58797 -
Flags: superreview+
Comment 9•23 years ago
|
||
Comment on attachment 58797 [details] [diff] [review] patch for review r=ben@netscape.com
Attachment #58797 -
Flags: review+
Assignee | ||
Comment 10•23 years ago
|
||
This patch seems to have speed up the window opening test on tinderbox (Txul) significantly (~1790ms to ~1700ms). Not sure why, but I'll take it.
Comment 11•23 years ago
|
||
Someone stop him!! He's outta control!
Comment 12•23 years ago
|
||
I have a test that opens and times 100 windows, some opened singly, some in groups of three, and some 10 at a time. After running this test with dbaron's leak fix, task manager on win2k reports that I'm using 23.8MB (26.6MB peak). By comparison, without the fix, I use 44.3MB (46.0MB peak). Gee. A mere 20MB savings. Is that all :-] The times also show that this is now, on average, about 9% faster through the first 40 windows, and because it no longer hits the same pathological states near the end of the test, about 17% faster overall through 100 windows.
Assignee | ||
Comment 13•23 years ago
|
||
jrgm pointed out that the obvious explanation for the speedup is less time in js_GC because there's less to mark.
Assignee | ||
Comment 14•23 years ago
|
||
Fix checked in 2001-11-22 13:44 PDT, in case it wasn't mentioned already.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 15•23 years ago
|
||
dbaron: we love you man! /be
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: jrgmorrison → xptoolkit.widgets
You need to log in
before you can comment on or make changes to this bug.
Description
•