380969 - XPCOMUtils leaks

Status: RESOLVED WORKSFORME

(Core :: XPConnect, defect)

Description

[Robert Sayre]

It's hard to tell what is a real problem, since we're leaking all JSContexts on shutdown (bug 375063). I was able to make many of the leaks go away by changing generateFactoryMethod to accept an array of strings rather than interfaces, so we don't close over C.i.nsIFoo. That leaves 4-6 objects leaking.

Comment 1

[Robert Sayre]

Attachment: patch

I tried this with FUEL. With this patch and the corresponding changes to the FUEL boilerplate, leaks go below pre-FUEL levels--if no category manager entries are added in the postRegistration functions. It turns out that there is no way for JS components to finalize, because we never call unregisterSelf. If we did call that, it would give FUEL a chance to delete those category manager entries.

I have not verified the cause of the remaining leak--it could persist if we do call unregisterSelf on shutdown, but I suspect removing the catman entries will be sufficient.

Comment 2

[Benjamin Smedberg]

I don't understand. Category manager keeps string/string pairs, it doesn't keep objects alive. And XPCOM registration should be persistent, so we definitely shouldn't be calling unregisterSelf.

I don't understand what you mean by "JS components to finalize"... we should be releasing the script scope for all JS components here: http://mxr.mozilla.org/mozilla/source/js/src/xpconnect/loader/mozJSComponentLoader.cpp#1274

Comment 3

[Robert Sayre]

(In reply to comment #2)
> I don't understand. Category manager keeps string/string pairs, it doesn't keep
> objects alive. 

Oh, I was mistaken. We're leaking a BackstagePass, and fuel is registered as a global JS property.

> I don't understand what you mean by "JS components to finalize"

Nevermind that, I was misguided.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

After I checked in the patch for bug 385087 to make the content prefs service use XPCOMUtils, Rlk on fxdbug-linux-tbox went from 3.07KB to 3.34KB with the following new leaks:

TOTAL                                          3416       8.51%   
--NEW-LEAKS-----------------------------------leaks------leaks%---
nsJSID                                          108     200.00%
XPCWrappedNative                               1736      10.71%
XPCWrappedNativeProto                           728       4.00%
TOTAL                                          2572

       |<----------------Class--------------->|<-----Bytes------>|<----------------Objects---------------->|<--------------References-------------->|
                                                Per-Inst   Leaked    Total      Rem      Mean       StdDev     Total      Rem      Mean       StdDev

Before checkin <http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1182804780.26250.gz&fulltext=1>:

    83 XPCWrappedNative                               56     1568     4006       28 ( 1167.25 +/-   539.90)    78528       28 ( 1623.36 +/-   534.49)
    84 XPCWrappedNativeProto                          28      700      833       25 (  257.93 +/-    97.64)        0        0 (    0.00 +/-     0.00)
   449 nsJSID                                         36       36      510        1 (  113.73 +/-    63.23)     2586        1 (  184.72 +/-   117.88)

After checkin <http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1182805800.28454.gz&fulltext=1>:

    83 XPCWrappedNative                               56     1736     4051       31 ( 1164.88 +/-   533.66)    78681       31 ( 1610.35 +/-   528.02)
    84 XPCWrappedNativeProto                          28      728      852       26 (  258.51 +/-    95.43)        0        0 (    0.00 +/-     0.00)
   450 nsJSID                                         36      108      517        3 (  114.18 +/-    63.63)     2638        3 (  185.41 +/-   119.78)

Comment 5

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #4)
> After I checked in the patch for bug 385087 to make the content prefs service
> use XPCOMUtils, Rlk on fxdbug-linux-tbox went from 3.07KB to 3.34KB with the
> following new leaks:

Urm, it appears the leak was actually caused by dtownsend's checkin for bug 257155.

Comment 6

[Robert Sayre]

"XPCOMUtils leaks" -- oop, no it doesn't. Global objects used to leak and now they don't.

Comment 7

[Robert Sayre]

(now that bug 180380 is fixed)