365723 - [TB3a1] memory leak after closing message window

Status: VERIFIED FIXED

(Thunderbird :: Mail Window Front End, defect, P2)

Description

[-fullmetaljacket-]

possible memory leak after closing message window.

step to reproduce:

1. install leak monitor https://addons.mozilla.org/firefox/2490/
2. open email or rss to message new window
3. close window

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
from leak monitor log:

Leaks in window 0x5e2cec0:
[+] [leaked object] (6a13ca0) = [Object]
 [+] maxWantedNesting (6a13c80, chrome://messenger-smime/content/msgHdrViewSMIMEOverlay.js, 21-22) = [Function]
  [ ] prototype (3a6be40) = [Object]
 [+] signedStatus (6a13c60, chrome://messenger-smime/content/msgHdrViewSMIMEOverlay.js, 26-58) = [Function]
  [ ] prototype (3a6be80) = [Object]
 [+] encryptionStatus (6a13c40, chrome://messenger-smime/content/msgHdrViewSMIMEOverlay.js, 63-108) = [Function]
  [ ] prototype (3a68a40) = [Object]
 [+] QueryInterface (6a13c20, chrome://messenger-smime/content/msgHdrViewSMIMEOverlay.js, 114-117) = [Function]
  [ ] prototype (3a66f00) = [Object]
[+] [leaked object] (5e33e20) = [Object]
 [+] onStartHeaders (5e33e00, chrome://messenger/content/msgHdrViewOverlay.js, 263-293) = [Function]
  [ ] prototype (3a9c500) = [Object]
 [+] onEndHeaders (5e33de0, chrome://messenger/content/msgHdrViewOverlay.js, 297-312) = [Function]
  [ ] prototype (3a9c540) = [Object]
 [+] processHeaders (5e33dc0, chrome://messenger/content/msgHdrViewOverlay.js, 316-383) = [Function]
  [ ] prototype (3a9c580) = [Object]
 [+] handleAttachment (5e33da0, chrome://messenger/content/msgHdrViewOverlay.js, 387-428) = [Function]
  [ ] prototype (3a9c620) = [Object]
 [+] onEndAllAttachments (5e33d80, chrome://messenger/content/msgHdrViewOverlay.js, 434-435) = [Function]
  [ ] prototype (3a9c6a0) = [Object]
 [+] onEndMsgDownload (5e33d60, chrome://messenger/content/msgHdrViewOverlay.js, 439-453) = [Function]
  [ ] prototype (3a9c7c0) = [Object]
 [+] onEndMsgHeaders (5e33d40, chrome://messenger/content/msgHdrViewOverlay.js, 457-458) = [Function]
  [ ] prototype (3a9c880) = [Object]
 [+] onMsgHasRemoteContent (5e33d20, chrome://messenger/content/msgHdrViewOverlay.js, 462-463) = [Function]
  [ ] prototype (3a9cb20) = [Object]
 [ ] mSecurityInfo = null
 [+] mSaveHdr (76ec280) = [XPCWrappedNative_NoHelper]
  [ ] messageKey = true
  [+] getStringProperty (76dc920) = [Function]
   [ ] prototype (5df8c60) = [Object]
  [ ] isRead = true
  [ ] messageId = true
  [ ] flags = true
  [ ] markHasAttachments (76dc6a0) = [Function]
  [ ] getUint32Property (76db6a0) = [Function]
  [+] QueryInterface (3a71960) = [Function]
   [ ] prototype (3aa2f60) = [Object]
  [+] getProperty (3a71920) = [Function]
   [ ] prototype (3aa2fc0) = [Object]
  [+] setProperty (3a7a880) = [Function]
   [ ] prototype (3a9f3e0) = [Object]
  [+] setStringProperty (3a7a860) = [Function]
   [ ] prototype (3aa0780) = [Object]
  [+] setUint32Property (3a7a840) = [Function]
   [ ] prototype (3aa07a0) = [Object]
  [ ] isFlagged = true
  [+] markRead (3a7a5a0) = [Function]
   [ ] prototype (3aa07e0) = [Object]
  [+] markFlagged (3a7a460) = [Function]
   [ ] prototype (3aa08c0) = [Object]
  [ ] priority = true
  [+] setPriorityString (3a7a3e0) = [Function]
   [ ] prototype (3aa0960) = [Object]
  [+] OrFlags (3a7a3c0) = [Function]
   [ ] prototype (3aa09c0) = [Object]
  [+] AndFlags (3a7a3a0) = [Function]
   [ ] prototype (3aa09e0) = [Object]
  [ ] threadId = true
  [ ] threadParent = true
  [ ] messageSize = true
  [ ] lineCount = true
  [ ] statusOffset = true
  [ ] messageOffset = true
  [ ] offlineMessageSize = true
  [ ] date = true
  [ ] dateInSeconds = true
  [ ] ccList = true
  [ ] author = true
  [ ] subject = true
  [ ] recipients = true
  [+] setReferences (3a98220) = [Function]
   [ ] prototype (3aa0a60) = [Object]
  [ ] numReferences = true
  [+] getStringReference (3a980e0) = [Function]
   [ ] prototype (3aa0a80) = [Object]
  [+] setRecipientsArray (3a980c0) = [Function]
   [ ] prototype (3aa0ac0) = [Object]
  [+] setCCListArray (3a9dba0) = [Function]
   [ ] prototype (3aa0b60) = [Object]
  [ ] mime2DecodedAuthor = true
  [ ] mime2DecodedSubject = true
  [ ] mime2DecodedRecipients = true
  [ ] Charset = true
  [ ] label = true
  [ ] accountKey = true
  [ ] folder = true
 [+] getSecurityInfo (5e33d00, chrome://messenger/content/msgHdrViewOverlay.js, 469-470) = [Function]
  [ ] prototype (3a6ff60) = [Object]
 [+] setSecurityInfo (5e33ce0, chrome://messenger/content/msgHdrViewOverlay.js, 473-474) = [Function]
  [ ] prototype (3a6ff80) = [Object]
 [ ] mDummyMsgHeader = null
 [+] getDummyMsgHeader (5e33cc0, chrome://messenger/content/msgHdrViewOverlay.js, 480-483) = [Function]
  [ ] prototype (3a70d80) = [Object]
 [ ] mProperties = null
 [+] getProperties (5e33ca0, chrome://messenger/content/msgHdrViewOverlay.js, 487-491) = [Function]
  [ ] prototype (3a70e00) = [Object]
 [+] securityInfo (76dda20) = [XPCWrappedNative_NoHelper]
  [+] QueryInterface (3a6b120) = [Function]
   [ ] prototype (5df8be0) = [Object]

Comment 1

[Peter Weilbacher]

This looks very similar to bug 339072 (which is about SeaMonkey).

Comment 2

[David Ascher (:davida)]

It'd be good to know if this leak increase on subsequent openings of the window, or if it's only one leak for all time.  In my experience, the former is most likely.  Moving off of blocking.

Comment 3

[Mark Banner (:standard8)]

I'll take a look at this as well.

Comment 4

[Mark Banner (:standard8)]

Attachment: The fix [Checkin: Comment 7]

I've had a dig around, and it turns out that nsMessenger was adding itself as a folder listener (in nsMsgMailSession), but never removing itself. This doesn't show up as a leak on shutdown, because nsMsgMailSession releases all its listeners on shutdown. However, we're still effectively leaking a bunch of memory until the user shuts down - and each time a message window is created and closed we leak the memory.

Leak Monitor still says we're leaking a cached chrome window after you close the window, however it then says we reclaim it later on, so I'm not really worried about that, the listener is causing the major leak.

Comment 5

[Mark Banner (:standard8)]

I think this is definitely wanted now.

Comment 6

[Serge Gautherie (:sgautherie)]

Wanted on 1.8(.1) branch too:
<http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/mailnews/base/src/nsMessenger.cpp&rev=1.320.2.20&mark=387-390,395#386>

While there, could you fix the (branch) double |nsresult rv| declaration too ?

Comment 7

[Serge Gautherie (:sgautherie)]

1. Open, then Close (SeaMonkey) MailNews 3-pane window.


[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.0.1pre) Gecko/2008062602 SeaMonkey/2.0a1pre] (nightly) (W2Ksp4)

These 2 do leak (= not reclaimed), and every time:
{{
Leaks in window 0x1d07660:
[+] [leaked object] (3264ea0) = [Object]
 [ ] onStartHeaders (3264ec0, chrome://messenger/content/msgHdrViewOverlay.js, 291-329) = [Function]
 [ ] onEndHeaders (3264ee0, chrome://messenger/content/msgHdrViewOverlay.js, 333-349) = [Function]
 [ ] processHeaders (3264f00, chrome://messenger/content/msgHdrViewOverlay.js, 353-446) = [Function]
 [ ] handleAttachment (3264f20, chrome://messenger/content/msgHdrViewOverlay.js, 450-485) = [Function]
 [ ] onEndAllAttachments (3264f40, chrome://messenger/content/msgHdrViewOverlay.js, 491-496) = [Function]
 [ ] onEndMsgDownload (3e34000, chrome://messenger/content/msgHdrViewOverlay.js, 500-514) = [Function]
 [ ] onEndMsgHeaders (3e34020, chrome://messenger/content/msgHdrViewOverlay.js, 518-519) = [Function]
 [ ] onMsgHasRemoteContent (3e34040, chrome://messenger/content/msgHdrViewOverlay.js, 523-524) = [Function]
 [ ] mSecurityInfo (3ef4ac0) = [XPCWrappedNative_NoHelper]
 [ ] mSaveHdr = null
 [ ] securityInfo = true
 [ ] mDummyMsgHeader = null
 [ ] dummyMsgHeader = true
 [ ] mProperties = null
 [ ] properties = true
[+] [leaked object] (3e0b300) = [Object]
 [ ] maxWantedNesting (3e0b320, chrome://messenger-smime/content/msgHdrViewSMIMEOverlay.js, 56-57) = [Function]
 [ ] signedStatus (3e0b340, chrome://messenger-smime/content/msgHdrViewSMIMEOverlay.js, 61-94) = [Function]
 [ ] encryptionStatus (3e0b360, chrome://messenger-smime/content/msgHdrViewSMIMEOverlay.js, 99-126) = [Function]
 [ ] QueryInterface (3e0b380, chrome://messenger-smime/content/msgHdrViewSMIMEOverlay.js, 131-134) = [Function]
}}

This is (like) comment 0.
R.Fixed, by comment 4 patch:
{{
2008-06-26 07:55	bugzilla%standard8.plus.com 	mozilla/mailnews/base/src/nsMessenger.cpp 	1.390
}}


[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.0.1pre) Gecko/2008062615 SeaMonkey/2.0a1pre] (SEA-WIN32-TBOX-trunk) (W2Ksp4)

Fwiw, remaining reclaimed/(false) _first-close-only_ leaks:

{{
Leaks in window 0x2569c20:
[ ] [leaked object] (2569c20) = [ChromeWindow]
[ ] [leaked object] (363ea60, chrome://communicator/content/viewZoomOverlay.js, 269-274) = [Function]
[ ] [leaked object] (2b0a3c0, chrome://messenger/content/threadPane.js, 43-88) = [Function]
[ ] [leaked object] (36ecca0, chrome://messenger/content/msgMail3PaneWindow.js, 919-924) = [Function]
[ ] [leaked object] (2bf3ac0, chrome://communicator/content/permissions/imageContextOverlay.xul, 102-113) = [Function]
[ ] [leaked object] (2ba8d60, chrome://global/content/charsetOverlay.js, 240-245) = [Function]
[ ] [leaked object] (36eccc0, chrome://messenger/content/msgMail3PaneWindow.js, 929-931) = [Function]
[ ] [leaked object] (36ecd80, chrome://messenger/content/msgMail3PaneWindow.js, 1001-1010) = [Function]
[ ] [leaked object] (36f20c0, chrome://messenger/content/msgMail3PaneWindow.js, 1210-1220) = [Function]
[ ] [leaked object] (36f20e0, chrome://messenger/content/msgMail3PaneWindow.js, 1224-1247) = [Function]
}}

(I guess) This is the end of comment 4.
Do we want a separate bug to look into improving this, or do we just don't (need to) care ?

Comment 8

[Serge Gautherie (:sgautherie)]

(In reply to comment #7)
> R.Fixed, by comment 4 patch:

V.Fixed, actually.

Comment 11

[Serge Gautherie (:sgautherie)]

(In reply to comment #7)
> [Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.0.1pre)
> Gecko/2008062615 SeaMonkey/2.0a1pre] (SEA-WIN32-TBOX-trunk) (W2Ksp4)

Fwiw, from bug 336973, closing a message (in its own) window leaks+reclaims a subset of what the 3-pane does:
{{
Leaks in window 0x3fac4c0:
[ ] [leaked object] (3fac4c0) = [ChromeWindow]
[ ] [leaked object] (472d820, chrome://communicator/content/viewZoomOverlay.js, 269-274) = [Function]
[ ] [leaked object] (44fab20, chrome://global/content/charsetOverlay.js, 240-245) = [Function]
[ ] [leaked object] (4739560, chrome://communicator/content/permissions/imageContextOverlay.xul, 102-113) = [Function]
}}

Comment 13

[Mark Banner (:standard8)]

(In reply to comment #7)
> {{
> Leaks in window 0x2569c20:
> [ ] [leaked object] (2569c20) = [ChromeWindow]
> [ ] [leaked object] (363ea60, chrome://communicator/content/viewZoomOverlay.js,
> 269-274) = [Function]
> [ ] [leaked object] (2b0a3c0, chrome://messenger/content/threadPane.js, 43-88)
> = [Function]
> [ ] [leaked object] (36ecca0, chrome://messenger/content/msgMail3PaneWindow.js,
> 919-924) = [Function]
> [ ] [leaked object] (2bf3ac0,
> chrome://communicator/content/permissions/imageContextOverlay.xul, 102-113) =
> [Function]
> [ ] [leaked object] (2ba8d60, chrome://global/content/charsetOverlay.js,
> 240-245) = [Function]
> [ ] [leaked object] (36eccc0, chrome://messenger/content/msgMail3PaneWindow.js,
> 929-931) = [Function]
> [ ] [leaked object] (36ecd80, chrome://messenger/content/msgMail3PaneWindow.js,
> 1001-1010) = [Function]
> [ ] [leaked object] (36f20c0, chrome://messenger/content/msgMail3PaneWindow.js,
> 1210-1220) = [Function]
> [ ] [leaked object] (36f20e0, chrome://messenger/content/msgMail3PaneWindow.js,
> 1224-1247) = [Function]
> }}
> 
> (I guess) This is the end of comment 4.
> Do we want a separate bug to look into improving this, or do we just don't
> (need to) care ?

On Thunderbird only the first item - the ChromeWindow - is leaked and then reclaimed (at least for the Message window, I didn't check the 3-pane), so I guess SeaMonkey has it slightly worse. I would probably talk to dbaron about if it is worth trying to clean up the reclaimed memory earlier or not, but I expect it may just be things we can't do anything about due to the cycle collection routines.

Comment 14

[Serge Gautherie (:sgautherie)]

(In reply to comment #13)
> (In reply to comment #7)
> > [ ] [leaked object] (2569c20) = [ChromeWindow]
> 
> On Thunderbird only the first item - the ChromeWindow - is leaked and then
> reclaimed (at least for the Message window, I didn't check the 3-pane), so I

True enough, I had not yet bothered to install plugins (like LME, Venkman, ...) in TB:
I'll have to...

> guess SeaMonkey has it slightly worse. I would probably talk to dbaron about if

Probably: like bug 442189, which I found in the process ;->

> it is worth trying to clean up the reclaimed memory earlier or not, but I
> expect it may just be things we can't do anything about due to the cycle
> collection routines.

I concur:
better if we could get rid of the leaks+reclaims;
but will live with them if that's the way it works.

***

(Then, right now, I'm only looking forward (here) to the port to the branch ;->)

Obviously, THANKS for finding the root cause of this one out !

Comment 15

[Serge Gautherie (:sgautherie)]

(In reply to comment #14)
> better if we could get rid of the leaks+reclaims;
> but will live with them if that's the way it works.

I filed bug SM+TB bug 442569, TB bug 442478, SM bug 442482.