leak pres context of second window (and later windows)

RESOLVED FIXED in mozilla0.9.7

Status

()

Core
XUL
P1
normal
RESOLVED FIXED
17 years ago
10 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

({memory-leak})

Trunk
mozilla0.9.7
x86
Linux
memory-leak
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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
Keywords: mlk
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.
Hrm.  And we're leaking |nsFrame|s
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.
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.
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla0.9.7
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.
Created attachment 58797 [details] [diff] [review]
patch for review
Attachment #58787 - Attachment is obsolete: true

Comment 8

17 years ago
sr=hyatt

Updated

17 years ago
Attachment #58797 - Flags: superreview+
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

17 years ago
Someone stop him!! He's outta control!

Comment 12

17 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.
jrgm pointed out that the obvious explanation for the speedup is less time in
js_GC because there's less to mark.
Fix checked in 2001-11-22 13:44 PDT, in case it wasn't mentioned already.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
dbaron: we love you man!

/be

Updated

10 years ago
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.