nsMsgWindow and nsMsgProgress leak due to cycle

VERIFIED FIXED in Thunderbird1.1

Status

Thunderbird
Message Compose Window
VERIFIED FIXED
13 years ago
13 years ago

People

(Reporter: dbaron, Assigned: Scott MacGregor)

Tracking

({mlk})

Trunk
Thunderbird1.1
x86
Linux

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

13 years ago
nsMsgWindow and nsMsgProgress leak since each one owns the other and nothing
breaks the cycle.

Steps to reproduce (using trace-refcnt or your favorite leak instrumentation):
 * start thunderbird
 * press Ctrl-M
 * type an email address, TAB, a subject ("test"), TAB, a message ("test")
 * click the X in the corner of the window
 * choose "Save" (to Drafts)
 * quit thunderbird

This is responsible for an increase of 1 in the ++DOMWINDOW / --DOMWINDOW count
since nsMsgProgress owns a JS object (which keeps the window object for the
compose window alive), due to the code here:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mail/components/compose/content/MsgComposeCommands.js&rev=1.61&mark=1848#1835
(Reporter)

Comment 1

13 years ago
Created attachment 175367 [details]
stack showing nsMsgWindow held by nsMsgProgress
(Reporter)

Comment 2

13 years ago
Created attachment 175368 [details]
stack showing nsMsgProgress held by nsMsgWindow
re-assign to mscott
Assignee: sspitzer → mscott
oops, this might be seamonkey bug, and not tbird.  (but tbird may have this
problem, too.)
(Assignee)

Updated

13 years ago
Status: NEW → ASSIGNED
Component: MailNews: Composition → Message Compose Window
Product: Core → Thunderbird
Target Milestone: --- → Thunderbird1.1
(Assignee)

Comment 5

13 years ago
Created attachment 178506 [details] [diff] [review]
the fix

good find dbaron.

I could have easily broken the cycle by calling
m_msgWindow->SetStatusFeedback(nsnull) after we issue the on stop notification 


However, I don't believe the progress class should have a strong reference to
the msg window anyway, so instead I re-wrote it to use a weak reference.

Now the progress object gets destroyed after the msg window is released
(usually on shut down). 

In addition to the weak reference change, we could also clear out the status
feedback reference by hand after the progress class issues an on stop
notification. That would cause the progress object to get deleted right away
instead of waiting until the msg window goes away or someone else sets a status
feedback object on the msg window.
Attachment #178506 - Flags: superreview?(bienvenu)

Updated

13 years ago
Attachment #178506 - Flags: superreview?(bienvenu) → superreview+
(Assignee)

Updated

13 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
(Reporter)

Updated

13 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.