We leak one webshell per address in the compose window.

VERIFIED FIXED in M14

Status

MailNews Core
Composition
P2
normal
VERIFIED FIXED
19 years ago
10 years ago

People

(Reporter: Scott MacGregor, Assigned: Chris Waterson)

Tracking

({memory-leak})

Trunk
x86
Other
memory-leak

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [PDT+] 3/3)

Attachments

(3 attachments)

(Reporter)

Description

19 years ago
I've noticed that whenever I reply to messages with a couple recipients, I'm
seeing a lot of webshell leaks.

It looks like for every recipient row in the envelope, we leak the gfx text
webshell associated with that row!!

*Yikes*

Jean-Francois, who owns the widget that we enter our email addresses into? Would
buster be the right person to get the ball rolling with this?

(I fixed a leak in gfx text widgets once before where we were leaking
webshells...maybe that leak is back? I'll go look)
(Reporter)

Comment 1

19 years ago
Nominating for beta1 status.

Keywords: beta1
(Reporter)

Comment 2

19 years ago
This may require some leak detection work on our own first. Just bringing up the
compose window and then closing the compose window leaked nsMsgCompose and
everything underneath it!!! Uhoh...


nsMsgCompose leak (and everything under it) because of bug 21056.
(Reporter)

Comment 4

19 years ago
Okay, then we should nominate that bug for dogfood. Although according to that
bug, nsMsgCompose should only be leaked for the first compose window.

The webshell leaks I'm reporting here for each email address happen for every
compose window I bring up. if I reply to a message with 30 people, we leak 30
web shells.

So I'll still leave this bug as a stand alone bug from #21056 and we can
nominate that one for beta1 separately.

Summary: We leak one webshell per address in the compose window. → [MLK] We leak one webshell per address in the compose window.

Comment 5

19 years ago
Moving [MLK] to mlk keyword
Keywords: mlk
Summary: [MLK] We leak one webshell per address in the compose window. → We leak one webshell per address in the compose window.

Comment 6

19 years ago
Putting on PDT+ radar for beta1.
Whiteboard: [PDT+]

Updated

19 years ago
Status: NEW → ASSIGNED
Target Milestone: M14

Comment 7

19 years ago
Scott - can you nominate that other bug for Beta1 then? It's currently not set 
as beta1.  I'm sure you can include a better explanation than me :-)
QA Contact: lchiang → suresh

Updated

19 years ago
Whiteboard: [PDT+] → [PDT+] ETA:2/20
putting buster in cc list in case he has any idea why input element created dynamically from JS are leaking.

Comment 9

19 years ago
no, there's nothing special about being created dynamically from the point of 
view of the text control.  the webshell must be being leaked because the frame 
itself is being leaked.  either that, or something really odd is going on due to 
some webshell/docshell change. one experiment would be to undefine 
NEW_WEBSHELL_INTERFACES in nsGfxTextControlFrame.cpp.  If you don't see the 
leak, the leak is probably caused by something holding onto the webshell as a 
result of: mWebShell->SetDocument(...)
no are probably right. Sofar here is what I have found:

case 1, without creating a second recipient:
 create a WS for the top level window
   create a WS for the html inner frame
    create a WS for the gfx text control frame
    ...
    delete gfx text control frame WS
   delete html inner frame WS
  delete top level window WS

case 2, creating a second recipient:
 create a WS for the top level window
   create a WS for the html inner frame
    create a WS for the gfx text control frame (1st recipient)
    create a WS for the gfx text control frame (2nd recipient)
    ...
  delete top level window WS

in this second case, we are leaking the inner frame as well the two gfx text widgets!

Comment 11

19 years ago
maybe the tree widget itself is being leaked.
I just did the quick test with NEW_WEBSHELL_INTERFACES undified. The result are the same, we are still leaking 
the same way.
I make some progress. We are leaking because somebody hold a reference on the DocumentViewer member variable 
mPresShell. If a manally decreasse the refcount during the destroy process, all the WebShell are then correctly 
released.
cc'ing sfraser
I am kind of lost with this bug! I would really appreciate some help.
Whiteboard: [PDT+] ETA:2/20 → [PDT+] ETA:2/20 Help welcome!
Peter, this is apparently a circular reference problem with webshell. I don't know if it's a tree or gfxTextWidget 
problem.  As I am totally lost on this one, I reassign this bug to your team. Let me know if I can help on something 
else (that was my last PDT+).
Assignee: ducarroz → trudelle
Status: ASSIGNED → NEW

Comment 17

19 years ago
If there are any tree issues here, they are almost certainly unique to this 
uber-widget, and its singular usage of trees.  Please sit with Hyatt to get his 
assistance on this.  As his hands are once again unusable, you'll have to do the 
typing. I think you are still the right person to own this bug, so I'm 
reassigning it and cc'ing phil.
Assignee: trudelle → ducarroz

Updated

19 years ago
Status: NEW → ASSIGNED
Whiteboard: [PDT+] ETA:2/20 Help welcome! → [PDT+] ETA:UNKNOW Help welcome!
(Assignee)

Comment 18

19 years ago
i'll take a look at this one.
Assignee: ducarroz → waterson
Status: ASSIGNED → NEW
Whiteboard: [PDT+] ETA:UNKNOW Help welcome! → [PDT+]
(Assignee)

Updated

19 years ago
Status: NEW → ASSIGNED
Priority: P3 → P2
Whiteboard: [PDT+] → [PDT+] 2/25
(Assignee)

Comment 19

19 years ago
travis and I looked at this for a while tonight. as soon as you hit "enter" in 
the first address field, it leaks. (other random typing doesn't seem to induce 
the leak.)

the webshells that are leaked are the two ender widgets (for the first address 
that you just entered, and the second address line that got created), and the 
main message body area.

the outermost webshell is destroyed, as is its presshell. the inner presshells 
are not destroyed.

must sleep now. will look more tomorrow.
(Assignee)

Comment 20

19 years ago
This is a complete rats nest. We are leaking a PresShell, which is touched by
just about everything under the sun. This is going to be very hard to track
down, I think.
Whiteboard: [PDT+] 2/25 → [PDT+] 3/3
(Assignee)

Comment 21

19 years ago
Created attachment 5931 [details] [diff] [review]
fix.
(Assignee)

Comment 22

19 years ago
Created attachment 5932 [details] [diff] [review]
better fix
(Assignee)

Comment 23

19 years ago
Come to daddy, PresShell.

hyatt: can you code review me?

Comment 24

19 years ago
In your better fix case, shouldn't the first nsIPresShell* be 
nsCOMPtr<nsIPresShell> ?
(Assignee)

Comment 25

19 years ago
Created attachment 5934 [details] [diff] [review]
heh. third time's the charm.
(Assignee)

Comment 26

19 years ago
Heh. Oh yeah. Guess I should compile first.
(Reporter)

Comment 27

19 years ago
oh man...I can't believe you found it!! Nice one chris. I'm very impressed. You
can use me as a reviewer if you still need one.
(Assignee)

Comment 28

19 years ago
fix checked in; r=hyatt, travis. a=jar.
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED
Great job guys. Thanks

Comment 30

19 years ago
Number of webshell leaks that are dumped to the console looks right while bring 
up the compose window, typing addresses, and then closing the compose window. 
Marking this as Verified. Tested on Debug build that is pulled around yesterday 
evening. 

(Thanks to mscott in helping me to verify this bug).
Status: RESOLVED → VERIFIED
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.