344623 - Increased Rlk (leak) on balsa after landing of bug 342311

Status: VERIFIED FIXED

(Core :: XPCOM, defect)

Description

[Adam Guthrie]

2212	MOZ_CO_DATE=2006:07:13:08:42:00
5572	MOZ_CO_DATE=2006:07:13:10:25:00

Checkins: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-07-13+08%3A42&maxdate=2006-07-13+10%3A25&cvsroot=%2Fcvsroot

--NEW-LEAKS-----------------------------------leaks------leaks%
nsProxyObject                                  1456          -
nsUrlClassifierCallbackWrapper                  400          -
nsUrlClassifierDBServiceWorker                   40          -
nsObserverService                                48          -
nsXPCWrappedJS                                 1344     833.33%
nsThread                                        216     200.00%
xptiInterfaceInfo                                60      50.00%
nsXPCWrappedJSClass                             132      50.00%
nsStringBuffer                                   24      50.00%

Comment 1

[Benjamin Smedberg]

http://pastebin.ca/92373 is one of many odd stacks I get from the refcnt balancer while tracing this leak. This seems extremely scary to me. Darin, how are we calling nsThread::Shutdown from a proxy, and why is it recursive?

Comment 2

[Darin Fisher]

nsThreadPool uses a proxy of nsIThread to call nsIThread::Shutdown on the "right" thread.  Since nsThread::Shutdown spins the event loop, the stack you saw can happen and there shouldn't be any problems caused by it.

Comment 3

[Darin Fisher]

FWIW, I considered inventing and passing a flag to nsIThread::Shutdown to prevent it from pumping events, which would avoid these scary looking stacks in some cases, but I couldn't come up with any other justification for doing so other than that those stacks look scary.

Comment 4

[Benjamin Smedberg]

Spins which event loop? This stack is on the main thread, right? Why does nsThread::Shutdown appear three times?

Comment 5

[Benjamin Smedberg]

Oh, are those nsThread::Shutdown calls for different threads?

Comment 6

[Benjamin Smedberg]

Attachment: Duh

I'm going to commit this as self-evident. It doesn't fix the bug, but all the nsProxyObject leaks were bogus: only the urlclassifier leaks were new.

Comment 7

[Darin Fisher]

> Oh, are those nsThread::Shutdown calls for different threads?

Yes... "thread pool"

Comment 8

[Benjamin Smedberg]

Attachment: Sample refcount tree for a leaked wrapper

Comment 9

[Benjamin Smedberg]

Attachment: Really delete stuff, and count references properly, rev. 1

Two different refcounting issues makes it really painful to balance.

Comment 10

[Benjamin Smedberg]

Fixed on trunk.

Comment 11

[Adam Guthrie]

Verifified FIXED. Before and after bsmedberg's patch:

8165	MOZ_CO_DATE=2006:07:19:08:58:00 - before
2224	MOZ_CO_DATE=2006:07:19:11:51:00 - after
2212	MOZ_CO_DATE=2006:07:19:13:04:00 - in a build where we don't randomly leak nsRunnable(s)