29355 - leaking nsThread

Status: RESOLVED WORKSFORME

(Core :: XPCOM, defect, P3)

Description

[(not reading, please use [e-mail] instead)]

I'm working on a patch to fix the leaking of nsThread.

I've got a little more work to do, before I can hand it off to warren.

most of the nsThread.cpp code is written by warren, so he gets to be the lucky 
owner of this bug.

Comment 1

[(not reading, please use [e-mail] instead)]

whoops, wrong product.

Comment 2

[Warren Harris]

Which nsThread is leaking?

I know they don't leak in general for necko.

Comment 3

[(not reading, please use [e-mail] instead)]

here's a patch.

I just talked to warren, and there are problems with this patch, but I'm
attaching it for completeness.

other problems with this patch are it uses a static nsCOMPtr.

Comment 4

[(not reading, please use [e-mail] instead)]

Attachment: a bad patch, but one that shows the problem I was trying to fix.

Comment 5

[(not reading, please use [e-mail] instead)]

attaching the refcount balanced subtree.

setenv XPCOM_MEM_REFCNT_LOG log-file.dat
setenv XPCOM_MEM_LOG_CLASSES nsThread

./mozilla http://www.yahoo.com
perl -w ~/find-leakers.pl log-file.dat 
perl -w ~/make-tree.pl --object 0x0805A260 --ignore-balanced < ./log-file.dat >
~/tree.txt

Comment 6

[(not reading, please use [e-mail] instead)]

Attachment: the refcount tree

Comment 7

[(not reading, please use [e-mail] instead)]

from a phone conversation with warren:

1)  stomp one in the xpcom shutdown (and not by using an nsCOMPtr, like I did in
my patch)

2)  is nspr calling our Exit() function?

Comment 8

[Warren Harris]

I think there are 2 things going on here:

1. nspr isn't calling our destructor (nsThread::Exit) when it quits the main 
thread of the app. Cc'ing wtc.

2. I suspect that imap is still going to have thread problems because nowhere in 
your code do you join with the imap thread to synchronize shutdown with it.

I'm attaching my patches to work around the first problem. Let me know if they 
look ok. I also am cleaning up the global allocator too. Note that I also filed 
bug 29621 about the nsDirectoryService leak

Comment 9

[Warren Harris]

Attachment: fix for leaking the main thread

Comment 10

[(not reading, please use [e-mail] instead)]

testing / reviewing patch now.

Comment 11

[Warren Harris]

Moving non-essential, non-beta2 and performance-related bugs to M17.

Comment 12

[Warren Harris]

post-beta2

Comment 13

[Warren Harris]

What happened with this? I forget whether the code went in already or not.

Comment 14

[(not reading, please use [e-mail] instead)]

I thought you checked something in a one point.

Comment 15

[Warren Harris]

Marking this fixed then.

Comment 16

[leger]

Anything I can do to verify?

Comment 17

[Warren Harris]

Maybe look at the bloat log on tinderbox?

Comment 18

[Tracy Walker [:tracy]]

- Per last comments, and no reopen - Marking Verified/Fixed.  Please reopen if 
still a problem. 

Comment 19

[Warren Harris]

You obviously didn't look at tinderbox, because the leak is still there. 
Reopening.

Comment 20

[leger]

Updating QA Contact

Comment 21

[Dan M]

Moving Warren's thread bugs from his circular file to mine, because I was told it 
was the thing to do. Anyone who actually wants these bugs, now, feel free to 
correct my hastiness.

Comment 22

[Håkan Waara]

So is this leak still current or fixed?

Comment 23

[Jerome Lacoste]

This bug seems to have a patch. Could:
- the whiteboad be updated?
- the patch be retested and fixed.

Comment 24

[Asa Dotzler [:asa]]

Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)

Comment 25

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

Certainly not a problem anymore after the thread manager landing.