leak global object, etc., in main mail window

RESOLVED FIXED

Status

()

Core
XUL
P3
normal
RESOLVED FIXED
17 years ago
6 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

({mlk})

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink])

Attachments

(7 attachments)

(Assignee)

Description

17 years ago
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?
(Assignee)

Comment 1

17 years ago
Created attachment 17168 [details]
leaked objects
(Assignee)

Updated

17 years ago
Keywords: mlk
(Assignee)

Comment 2

17 years ago
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.
(Assignee)

Comment 3

17 years ago
The changes for bug 55426 may make this harder to detect.

Comment 4

17 years ago
->danm for investigation. Please nominate for rtm or future as appropriate.
Assignee: trudelle → danm

Updated

17 years ago
Target Milestone: --- → mozilla1.0
(Assignee)

Comment 5

17 years ago
I'm seeing a very similar leak in the message compose window.
(Assignee)

Comment 6

17 years ago
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?
(Assignee)

Comment 7

17 years ago
Well, it didn't work.
(Assignee)

Comment 8

17 years ago
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.
(Assignee)

Comment 9

17 years ago
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?
(Assignee)

Comment 10

17 years ago
Created attachment 23695 [details] [diff] [review]
patch that fixes the message-compose leak, but not the main mail window leak
(Assignee)

Comment 11

17 years ago
Created attachment 23702 [details] [diff] [review]
patch that may help main mail window leak, since it makes the crash in bug 65798 reliable
(Assignee)

Comment 12

17 years ago
Taking this bug since I've been working on it.
Assignee: danm → dbaron
Priority: P3 → P2
Target Milestone: mozilla1.0 → mozilla0.9
(Assignee)

Comment 13

17 years ago
Created attachment 24721 [details] [diff] [review]
proposed patch

Comment 14

17 years ago
Nice. r=waterson

Comment 15

17 years ago
I should add, I'm assuming you only want r= on the last patch?
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
(Assignee)

Comment 17

17 years ago
Created attachment 25535 [details] [diff] [review]
fix for leak in main mail window
(Assignee)

Comment 18

17 years ago
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...
(Assignee)

Comment 19

17 years ago
Created attachment 25701 [details] [diff] [review]
same patch with a comment explaining why

Comment 20

17 years ago
r=waterson

Comment 21

17 years ago
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

17 years ago
Never mind.  Odds are slim it would become a problem.  Fire away.
(Assignee)

Comment 23

17 years ago
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.
(Assignee)

Comment 24

17 years ago
Moving to 0.9.1 for dealing with the remaining issues.
Target Milestone: mozilla0.9 → mozilla0.9.1
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

16 years ago
Target Milestone: mozilla0.9.1 → mozilla0.9.2
(Assignee)

Updated

16 years ago
Target Milestone: mozilla0.9.2 → Future
(Assignee)

Updated

16 years ago
Priority: P2 → P3

Updated

16 years ago
Blocks: 92580

Updated

16 years ago
No longer blocks: 92580
(Assignee)

Updated

16 years ago
Summary: leak global object, etc., in main mail window → used to leak global object, etc., in main mail window
(Assignee)

Comment 25

16 years ago
Created attachment 62196 [details] [diff] [review]
old patch that I removed from my tree that doesn't help anything (?)

Updated

9 years ago
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: jrgmorrison → xptoolkit.widgets
dbaron, is this still reproducible?
Whiteboard: [MemShrink]
This is ancient and probably not relevant any more.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WONTFIX
(Assignee)

Comment 28

6 years ago
based on comment 23, fixed
Resolution: WONTFIX → FIXED
Summary: used to leak global object, etc., in main mail window → leak global object, etc., in main mail window
Target Milestone: Future → ---
You need to log in before you can comment on or make changes to this bug.