Closed
Bug 4127
Opened 26 years ago
Closed 25 years ago
[BLOCK] MLK: We are leaking the entire nsWebShellWindow
Categories
(Core :: XUL, defect, P1)
Core
XUL
Tracking
()
VERIFIED
FIXED
M5
People
(Reporter: bruce, Assigned: scc-obsolete)
References
Details
(Whiteboard: QA BLOCKER)
Apprunner leaks the entire WebShell just by starting up and shutting down.
Viewer doesn't. This causes nearly everything in apprunner to leak. I can
reproduce this with both Linux and Solaris builds for some time now (today is
March 21, 1999). I assume this is XP, but can't verify that for myself.
Reporter | ||
Comment 1•26 years ago
|
||
Actually, we leak nsWebShellWindow (xpfe/appshell/src/nsWebShellWindow.cpp).
The destructor on that object is never called, nor is the Close() method.
Re-assigned to danm@netscape.com, added others to the cc list, and changed
component to XP Toolkit/Widgets.
Dan, let me know if you need some help with this ...
Reporter | ||
Updated•26 years ago
|
Summary: MLK: We are leaking the entire WebShell → MLK: We are leaking the entire nsWebShellWindow
Reporter | ||
Comment 3•26 years ago
|
||
Look at
http://cvs-mirror.mozilla.org/webtools/bonsai/cvsblame.cgi?file=mozilla/xpfe/app
shell/src/nsAppShellService.cpp ... lines 177-207. rods@netscape.com checked in
what looked like a decent version of uninitialization code which called
CloseTopLevelWindow(). This being disabled looks like a strong candidate for
the leak. That code didn't compile for me as it is now, but looks a whole lot
better than the code in there now.
Reporter | ||
Comment 5•26 years ago
|
||
Fascinatingly enough ... if you close the window via the close button provided
by the window manager in X, it frees this memory up! This showed that fixing
this will reduce the startup/shutdown memory leak from 280k to about 70k, with
the majority of the remainder outside of the XPToolkit land.
Comment 6•26 years ago
|
||
This bug is causing crashes in Editor, because the caret, whick blinks on a
timer, relies on the pres shell being destroyed to stop it blinking.
Comment 7•26 years ago
|
||
I'm seeing that the webshell has a huge refCount (~298) when the window is
closed. It seems that most of these come from the menus code, since each menu
item appears to hold a reference to the wehshell. Chris, are the menus cleaning
up after themselves when closing windows?
Comment 8•26 years ago
|
||
Oh, wait. It's the menu delegates that are holding webshell refs.
Comment 9•26 years ago
|
||
What's the status on this one? I'm getting bugs filed (e.g. 3914) that are
caused by this.
Updated•26 years ago
|
Assignee: danm → saari
Comment 10•26 years ago
|
||
Reassigning to myself...
Comment 11•26 years ago
|
||
Simon,
What were you using to find the leaks? I'm changing things now, hopefully
improving the situation.
Comment 12•26 years ago
|
||
I exploded the addref and release macros in a few places (e.g. webshell), and
just debugged with breakpoints.
Updated•26 years ago
|
Target Milestone: M4 → M5
Comment 13•26 years ago
|
||
I'm latering to M5 because of risk. I've been steadily knocking down the
refcount, but it is touching a lot of files, and I'm starting to see cascading
side effects. I've got the refcount down to 4 in my tree when the window closes,
but after that things start going down hill fast.
Fixing all of the menu references did not fix this problem.
Comment 14•25 years ago
|
||
*** Bug 4288 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 15•25 years ago
|
||
How is this coming along? The big leaks are to the point where it is nearly
pointless to even bother looking at a leak report from Purify (which is why I've
not been reporting leaks much for a while).
Updated•25 years ago
|
Summary: MLK: We are leaking the entire nsWebShellWindow → [BLOCK] MLK: We are leaking the entire nsWebShellWindow
Comment 16•25 years ago
|
||
What is the status here? Bruce is about to go on strike and we seriously need his
work if the goal of M5 is small footprint. This bug is a blocker for Bruce
(marking as such in the summary).
Anyone? Anyone? Beuller?
Reporter | ||
Comment 17•25 years ago
|
||
saari, did all of your fixes make it into the tree so that the refcount is down
around 4 now hopefully? I *NEED* this fixed or there can be no further
meaningful memory leak detection, end of story. This drags down so much noise
that any real leak is totally lost in the noise. It has been over a week and no
response to my last ping (April 13), does _anyone_ care about this? It is
blocking programatic window closing and the editor team from having a properly
working caret. I guess I'm on strike.
Comment 18•25 years ago
|
||
I care about this, and I'll drop everything to look into it tomorrow unless
someone else figures it out before then.
/be
Comment 19•25 years ago
|
||
This is a top priority for me, my numero uno m5 bug, sorry I havn't got to this
yet... I should be working on it tomorrow.
I care quite a bit about this, I just don't have all the answers yet. The menu
fixes are helpful, but you can simulate them by commenting out the menus in
navigator.xul. With any luck I'll get them in tomorrow, but it involves changing
the ownership model (more like creating one to being with) across all platforms
in the first place.
I'm cc'ing Scott Collins as he is going to assist with this.
Updated•25 years ago
|
Assignee: saari → scc
Comment 20•25 years ago
|
||
assigning to scc per our discussion in the house party meeting. chris, you're too
doomed to have to worry about this one ;) ;)
adding myself to the cc list since one of my bugs is caused by this (i think).
Comment 21•25 years ago
|
||
I'm not doomed, just mostly incapacitated!
Comment 22•25 years ago
|
||
So where are we today? Brendan, did you find anything? M5 _cannot_ ship with
this bug, IMHO, so we need to get it nailed soon or I'm going to ask the build
guys to keep the tree closed until we do.
Updated•25 years ago
|
Priority: P2 → P1
Comment 23•25 years ago
|
||
raising priority to P1
Assignee | ||
Updated•25 years ago
|
Status: NEW → ASSIGNED
Reporter | ||
Comment 24•25 years ago
|
||
Bug #3258 is probably caused by this bug as well.
Comment 25•25 years ago
|
||
as is bug #4883
Comment 26•25 years ago
|
||
Done some more investigating:
We leak the webshell in viewer as well. The final refcnt of the webshell is 1. So
this has nothing to do with xptoolkit.
Comment 27•25 years ago
|
||
OK, it seems like we don't know where the bug is, and we don't have an owner, so
I've asked leaf and cyeh to hold the tree closed this morning until someone
steps up to own it ``for real''. (No offence, scc, but pink says you're not
going to be able to work on it in the near term.)
Comment 28•25 years ago
|
||
Of course, it's always possible that I'm a moron. I seem to have misunderstood
pink, so if scc is really On This, then I'll just crawl back to my hole.
Comment 29•25 years ago
|
||
We also leak, in viewer, EVERY SINGLE WIDGET EVER CREATED, INCLUDING THE TOPLEVEL
WIDGET.
Opening a new bug for that one. Sigh....
Comment 30•25 years ago
|
||
Why would this suddenly warrant holding the tree closed? How would having
someone claim 'real' ownership suddenly make the tree worth opening? Isn't
setting the status to 'assigned' enough anymore?
Comment 31•25 years ago
|
||
Touche', I guess. There are other Critical, P1 bugs that have been in assigned
state since, oh, January, so I guess my cynicism got the best of me. My bad,
thanks to scc for taking it on, no offence intended, already back in my hole,
etc., etc.
All that said, we haven't had really any traction on this bug in the month+
since it was filed, and lots of people have claimed that it was really important
to them. It's almost certainly not their fault that other stuff came up, but it
is frustrating to see.
Comment 32•25 years ago
|
||
Some time back, pinkerton wrote:
"We leak the webshell in viewer as well. The final refcnt of the webshell is 1.
So this has nothing to do with xptoolkit."
Mmm. In apprunner, the last release of the webshell leaves the refcount at
303. In viewer, it's 1. I'd say that that difference was significant, and until
things look better in apprunner (mostly menu probs), then don't pass the buck.
Comment 33•25 years ago
|
||
It's a hard bug. There has been progress, but only incremental. SCC is on it
full time as of today.
Comment 34•25 years ago
|
||
saari needs to checkin changes for menus (he has yet to do so). This will bring
it back to a normal number and why we know that saari isn't the culprit here. Why
he hasn't checked these in is unclear to me.
Since this happens in viewer AND it happens even when there are no menus in
apprunner....i stand by my previous statement. Whatever.
Comment 35•25 years ago
|
||
BTW, I'm not trying to pass the buck, but let everyone know the scope of this
problem. Since it happens in viewer, it's not something we did in xptoolkit so
there's no point focussing there first. Maybe I'm not being helpful with this
info, if not, I'll stop commenting.
Comment 36•25 years ago
|
||
*** Bug 5483 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 37•25 years ago
|
||
Made significant progress based on advice from troy. This cleans up viewer.
Still more work to do in AppRunner. Some objects seem to be still in use after
destruction. Comments from beard indicate that this has been the case for some
time.
Assignee | ||
Comment 38•25 years ago
|
||
Made significant progress based on advice from troy. This cleans up viewer.
Still more work to do in AppRunner. Some objects seem to be still in use after
destruction. Comments from beard indicate that this has been the case for some
time.
Assignee | ||
Comment 39•25 years ago
|
||
Made significant progress based on advice from troy. This cleans up viewer.
Still more work to do in AppRunner. Some objects seem to be still in use after
destruction. Comments from beard indicate that this has been the case for some
time.
Comment 40•25 years ago
|
||
scc and I just spent 10 hours on this and we're very close. A huge checkin
followed. We have two small problems and everything will be solved, documented,
and ready for the next person to screw up. Hopefully those can be fixed by end of
the day friday.
Comment 41•25 years ago
|
||
*** Bug 5483 has been marked as a duplicate of this bug. ***
Updated•25 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Comment 42•25 years ago
|
||
Fixed. WebShells are now being disposed in apprunner.
Comment 43•25 years ago
|
||
Hi Bruce, could you check this out and mark Verified if all is well? Thanks!
Reporter | ||
Updated•25 years ago
|
Status: RESOLVED → VERIFIED
Reporter | ||
Comment 44•25 years ago
|
||
This is resolved for now.
You need to log in
before you can comment on or make changes to this bug.
Description
•