87040 - Leaks in creating new mail message

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect)

Description

[dprice (gone)]

set XPCOM_MEM_LEAK_LOG=\filename
start mozilla -mail                 Size = 39,312K
ctrl-M to start a new mail message  Size = 45,556K
close mail message                  Size = 44,684K
ctrl-M to start a new mail message  Size = 45,860K
close mail message                  Size = 45,304K
ctrl-M to start a new mail message  Size = 46,404K
close mail message                  Size = 45,708K
exit

Our size keeps going up as we create and cancel new mail messages.
Simply starting with -mail and exiting, the leaklog tells me it's leaking 48K
After starting with -mail and canceling one new message it's leaking 1186K
After starting with -mail and canceling three new messages it's leaking 1489K

According to the leak log we leak 25 instances of nsDocument and the largest
standouts by size are... 
nsCSSSelector       77K
nsStr               77K
CSSDeclarationImpl  93K
nsXULAttribute     192K
nsXULAttributes    122K
nsXULElement       172K

Comment 1

[dprice (gone)]

keywords += mlk

Comment 2

[dprice (gone)]

Attachment: leaklog of start with -mail and exit

Comment 3

[dprice (gone)]

Attachment: leaklog after canceling 1 new message

Comment 4

[dprice (gone)]

Attachment: leaklog after calceling 3 messages

Comment 5

[dprice (gone)]

-> putterman

I'm available to help/answer questions.

Comment 6

[scottputterman]

reassigning to ducarroz.

Comment 7

[chris hofmann]

boy it would be good if we could nail some of this mail leaks in the next week.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

waterson and I poked at this for a bit last night, and didn't get very far.

We did manage to figure out what was needed to reproduce this.  It requires
focusing something in the mail compose window.  Starting with
"./mozilla -P <PROFILE> -compose"
one must focus something in the mail compose window.  (This seems to happen
automatically for some people, but not for me.)  If I don't focus anything, I
don't see the leak.  If I focus the from dropdown, the attachments widget, any
of the address fields, the subject line, or the message body editor, I see the leak.

It seems like the leak might be related to some ownership cycle that goes
through XPConnect.  There was a nsXULPrototypeAttribute::mEventHandler root that
wasn't unrooted, but it didn't seem to be rooting much in the GC, although
GC_MARK_DEBUG didn't seem too helpful.  The other strange thing was that there
were 2 leaked references but the imbalance in references that had cycles through
XPConnect looked like it was an imbalance of 3.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Bug 93146 explains the leaked roots I was seeing, which may or may not be the
cause of this leak.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The leaks are really from an nsEventStateManager - it's mDocument (I wish that
were an nsCOMPtr) and its gLastFocusedDocument.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The nsEventStateManager is leaked by the nsPresContext.  The nsPresContext is
leaked by 3 different imgRequestProxy (why do they own it?).

The nsEventStateManager, nsPresContext, and imgRequestProxy are among the 14K of
leaks that I see when I don't focus anything.  So it's a small leak that gets
much bigger when the user focuses something.

Comment 12

[Chris Waterson]

cc'ing pavlov. could you vend us a clue here, sir?

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

And the imgRequestProxy are somehow leaked from an nsImageBoxFrame (there are 6
frames leaked).  So some frame isn't destroying its children properly.  (Or
anonymous children, or something.)

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I suspect this is related to the assertions:

###!!! ASSERTION: not a container: 'nsnull == aChildList', file
/builds/seamonkey/mozilla/layout/html/base/src/nsFrame.cpp, line 365
###!!! Break: at file
/builds/seamonkey/mozilla/layout/html/base/src/nsFrame.cpp, line 365

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This fixes it:

Index: nsCSSFrameConstructor.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp,v
retrieving revision 1.609
diff -u -d -r1.609 nsCSSFrameConstructor.cpp
--- nsCSSFrameConstructor.cpp   2001/07/25 07:52:19     1.609
+++ nsCSSFrameConstructor.cpp   2001/08/02 04:22:08
@@ -5479,7 +5479,6 @@
       } // End of RESIZER CONSTRUCTION logic
 
       else if (aTag == nsXULAtoms::image) {
-        processChildren = PR_TRUE;
         isReplaced = PR_TRUE;
         rv = NS_NewImageBoxFrame(aPresShell, &newFrame);
       }

Comment 16

[Chris Waterson]

Righteous. r/sr=waterson

Comment 17

[scottputterman]

This is great.  Thanks David.  Did this fix all of the leak originally reported?
 Adding Varada.  We should update the numbers we're keeping track of after this
goes in.

Comment 18

[David Hyatt]

Who is trying to add a child to an <image> tag.  While I think this patch is
correct, I'd also like to see the bad XUL corrected.

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It was something related to bringing in editorOverlay.xul, but I didn't get
beyond that.

Comment 20

[David Hyatt]

hewitt's document inspector could presumably be used to find out the id and type
of the element being placed under the <image> in the content model.  lxr could
find that element, and then we could fix this in the content model as well.

Comment 21

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The offending XUL seems to be
http://lxr.mozilla.org/seamonkey/source/themes/modern/communicator/widgetBindings.xml#53
which has CSS binding it to the menubutton-icon binding which creates anonymous
children (including a menubutton-icon-icon image, from which the leak came):
http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/content/xul.css#576
http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/content/bindings/menubutton.xml#53

Comment 22

[David :Bienvenu]

which probably explains why it only happens in the modern skin, varada?

Comment 23

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: a possible XUL fix

Comment 24

[David Hyatt]

sr=hyatt

Comment 25

[Stuart Parmenter]

r=pavlov to both patches

Comment 26

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Taking

Comment 27

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in (nsCSSFrameConstructor.cpp), 2001-08-02 19:41 PDT.

Filed bug 93359 on getting the XBL fixed.

Comment 28

[Christopher Aillon (sabbatical, not receiving bugmail)]

*** Bug 93393 has been marked as a duplicate of this bug. ***

Comment 29

[[e-mail] (gone - use [e-mail] instead)]

Attachment: Current leak log

Comment 30

[[e-mail] (gone - use [e-mail] instead)]

Attachment: Leak log, after one cancel

Comment 31

[[e-mail] (gone - use [e-mail] instead)]

Attachment: Leak log, after 3 cancels

Comment 32

[[e-mail] (gone - use [e-mail] instead)]

Since I'm not leaking any |nsXULElement|s, |nsXULAttributes/nsXULAttribute|, 
|CSSDeclarationImpl| or |nsCSSSelector| classes, can I mark this verified?  
This was with a current debug trunk CVS build.  I'm assuming the leakage of 
|nsStr| is either known, or can be filed seperately, since the patches were 
meant to address the XUL/CSS leaks.  Thanks.

Comment 33

[[e-mail] (gone - use [e-mail] instead)]

verified fixed, see my criteria, above.