Last Comment Bug 56703 - leak global object, etc., in main mail window
: leak global object, etc., in main mail window
Status: RESOLVED FIXED
[MemShrink]
: mlk
Product: Core
Classification: Components
Component: XUL (show other bugs)
: Trunk
: x86 Linux
: P3 normal (vote)
: ---
Assigned To: David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2000-10-14 22:15 PDT by David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
Modified: 2011-08-30 17:55 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
leaked objects (5.17 KB, text/plain)
2000-10-14 22:18 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details
patch that fixes the message-compose leak, but not the main mail window leak (829 bytes, patch)
2001-01-28 14:35 PST, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Review
patch that may help main mail window leak, since it makes the crash in bug 65798 reliable (10.50 KB, patch)
2001-01-28 17:05 PST, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Review
proposed patch (1.70 KB, patch)
2001-02-07 17:00 PST, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Review
fix for leak in main mail window (1.26 KB, patch)
2001-02-17 19:43 PST, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Review
same patch with a comment explaining why (1.92 KB, patch)
2001-02-20 07:10 PST, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Review
old patch that I removed from my tree that doesn't help anything (?) (4.36 KB, patch)
2001-12-19 07:34 PST, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Review

Description David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2000-10-14 22:15:42 PDT
I'm filing this as a toolkit bug rather than a mail bug because I strongly
suspect it's a toolkit problem.

Opening the main mail window (run "mozilla -mail", and exit) leaks a
GlobalWindowImpl that is not garbage collected.  There is a single leaked JS
root: an nsXPCWrappedJS (that is an nsIController).  It is leaked from an
nsXULControllers object (it is inserted into that objects supports array and
never removed).  The leaked nsXULControllers object is leaked from a
GlobalWindowImpl.  This seems like a simple circular reference problem that
should be fixed by adding |mControllers = nsnull; // Forces release| to
|GlobalWindowImpl::CleanUp|.  However, that doesn't fix it.  I have no idea why?
Where did I go wrong?
Comment 1 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2000-10-14 22:18:33 PDT
Created attachment 17168 [details]
leaked objects
Comment 2 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2000-10-14 22:34:44 PDT
A few more thoughts.  I previously thought that CleanUp was called because the
window's script object was not rooted.  However, it could be that the window's
script object was never requested.  (How much could such a window be used? 
Maybe it was some sort of prototype?)

The mControllers=nsnull might be a good idea anyway, whether it's the problem
here or not.  I think once in the past I saw it reduce the fallout from another
leak.
Comment 3 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2000-10-15 15:36:16 PDT
The changes for bug 55426 may make this harder to detect.
Comment 4 Peter Trudelle 2000-10-15 15:52:47 PDT
->danm for investigation. Please nominate for rtm or future as appropriate.
Comment 5 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2001-01-24 14:24:52 PST
I'm seeing a very similar leak in the message compose window.
Comment 6 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2001-01-24 15:31:55 PST
Hmmm, nsHTMLInputElement and nsHTMLTextAreaElement also have an
nsXULControllers.  I bet that's the problem.  If so, is releasing it on
SetDocument(null) the right solution?
Comment 7 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2001-01-24 16:01:49 PST
Well, it didn't work.
Comment 8 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2001-01-24 16:12:53 PST
It didn't work because nsGfxTextControlFrame2::Destroy calls GetControllers.  At
least, that's why fixing the nsXULControllers leaked from the input element
didn't work.  I still don't know if that's the cause of the bigger leak.
Comment 9 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2001-01-28 12:42:21 PST
Well, I fixed the leak of the nsXULControllers from the input element (by adding
a SetControllers method and calling it in nsGfxTextControlFrame2::Destroy), and
the rest of the leak didn't go away, so the cycle is through the
nsXULControllers object that is the mControllers of the GlobalWindowImpl.  So
what's wrong here?  Is GlobalWindowImpl::Close() not being called?
Comment 10 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2001-01-28 14:35:44 PST
Created attachment 23695 [details] [diff] [review]
patch that fixes the message-compose leak, but not the main mail window leak
Comment 11 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2001-01-28 17:05:35 PST
Created attachment 23702 [details] [diff] [review]
patch that may help main mail window leak, since it makes the crash in bug 65798 reliable
Comment 12 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2001-02-04 13:40:37 PST
Taking this bug since I've been working on it.
Comment 13 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2001-02-07 17:00:14 PST
Created attachment 24721 [details] [diff] [review]
proposed patch
Comment 14 Chris Waterson 2001-02-15 19:49:53 PST
Nice. r=waterson
Comment 15 Chris Waterson 2001-02-15 19:50:44 PST
I should add, I'm assuming you only want r= on the last patch?
Comment 16 Brendan Eich [:brendan] 2001-02-15 23:01:02 PST
sr=brendan@mozilla.org.  Only thought is to subroutine so that CleanUp and
SetDocShell both call a BreakCycles method (or a better-named one).  Your call.

/be
Comment 17 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2001-02-17 19:43:09 PST
Created attachment 25535 [details] [diff] [review]
fix for leak in main mail window
Comment 18 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2001-02-20 07:06:59 PST
Hmm.  Perhaps a more general fix for this type of leak would be to set
mDocument to null in GlobalWindowImpl::SetDocShell.  This might break
a lot of potential cycles, but I'm not sure whether it would break all
the ones we need to break...
Comment 19 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2001-02-20 07:10:14 PST
Created attachment 25701 [details] [diff] [review]
same patch with a comment explaining why
Comment 20 Chris Waterson 2001-03-09 15:33:54 PST
r=waterson
Comment 21 David Hyatt 2001-03-09 17:28:53 PST
dbaron, you need to patch text area and input element as well, since they have
controllers also.

sr=hyatt on the XULElement, but please make the same patch to the other elts
before checking in.
Comment 22 David Hyatt 2001-03-09 17:34:53 PST
Never mind.  Odds are slim it would become a problem.  Fire away.
Comment 23 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2001-03-10 12:53:25 PST
Fix for nsXULElement checked in 2001-03-09 19:18 PST.  I need to investigate
whether we need a:
 * Input/TextArea fix
 * general fix in GlobalWindowImpl
so leaving the bug open for now.
Comment 24 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2001-04-17 18:38:47 PDT
Moving to 0.9.1 for dealing with the remaining issues.
Comment 25 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2001-12-19 07:34:41 PST
Created attachment 62196 [details] [diff] [review]
old patch that I removed from my tree that doesn't help anything (?)
Comment 26 Ryan VanderMeulen [:RyanVM] 2008-08-01 14:44:01 PDT
dbaron, is this still reproducible?
Comment 27 Andrew McCreight [:mccr8] 2011-08-30 13:59:54 PDT
This is ancient and probably not relevant any more.
Comment 28 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-08-30 17:55:33 PDT
based on comment 23, fixed

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