385237 - the content pref service leaks

Status: RESOLVED FIXED

(Toolkit :: Preferences, defect)

Description

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

After the content pref service got checked in, Rlk on fxdbug-linux-tbox went
from 2.99KB to 3.07KB and then to 3.16KB.  It now fluctuates between the latter
two numbers, whereas before it was fluctuating between the former two.

Specifically:

Before the content prefs checkins (http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1182279060.23027.gz&fulltext=1):
    83 XPCWrappedNative                               56     1512     3906       27 ( 1111.30 +/-   506.37)    75930       27 ( 1533.28 +/-   490.80)
    84 XPCWrappedNativeProto                          28      672      818       24 (  246.17 +/-    90.19)        0        0 (    0.00 +/-     0.00)

After the content prefs checkins (http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1182279900.26876.gz&fulltext=1):
    83 XPCWrappedNative                               56     1568     3887       28 ( 1134.88 +/-   521.61)    73593       28 ( 1581.79 +/-   527.41)
    84 XPCWrappedNativeProto                          28      700      809       25 (  252.20 +/-    94.93)        0        0 (    0.00 +/-     0.00)

--NEW-LEAKS-----------------------------------leaks------leaks%-----------------------
XPCWrappedNativeProto                           700       4.17%
XPCWrappedNative                               1568       3.70%
TOTAL                                          2268

I thought the fix for bug 385086 would help, but it turns out that it hasn't.

After the fix for bug 385086 (http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1182292800.8673.gz&fulltext=1):
    83 XPCWrappedNative                               56     1568     3819       28 ( 1147.60 +/-   545.77)    73858       28 ( 1605.39 +/-   550.97)
    84 XPCWrappedNativeProto                          28      700      783       25 (  253.16 +/-    99.11)        0        0 (    0.00 +/-     0.00)

The fix for bug 385087 might also help, or the problem might lie not in the toolkit/components/contentprefs code but rather in the browser/base/content code.

Comment 1

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

I've narrowed down the source of the leak.  It's triggered by the content pref sink adding itself as a web progress listener and a DOMContentLoaded listener:

http://lxr.mozilla.org/mozilla/source/browser/base/content/browser-contentPrefSink.js#77

Note: either addition triggers the leak, but the leak isn't compounded by the combination of additions.  If either or both additions take place, then 56 more XPCWrappedNative bytes and 28 more XPCWrappedNativeProto bytes are leaked than if I comment out both additions.

That happens to be the exact amount by which Rlk increased after I checked in content prefs, so this is the source of the entire leak.

What I can't quite figure out is why this leaks, given that the sink removes itself as a listener on shutdown via its destroy method, and I've confirmed that the method is being properly called.  I will note, however, that adding and then immediately removing the listeners in init does prevent the leak:

  init: function ContentPrefSink_init() {
    gBrowser.addProgressListener(this, Ci.nsIWebProgress.NOTIFY_STATE_DOCUMENT);
    gBrowser.addEventListener("DOMContentLoaded", this, false);
    gBrowser.removeEventListener("DOMContentLoaded", this, false);
    gBrowser.removeProgressListener(this);

And so does calling destroy immediately after init in delayedStartup:

    ContentPrefSink.init();
    ContentPrefSink.destroy();

So the problem only manifests when listener removal is triggered by BrowserShutdown.

Breaking the dependencies since fixing this leak does not depend on the fixes for those other two bugs after all.

Comment 2

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

After further digging, I've determined that the content pref service's factory is what's leaking.  When I remove the entire implementation of the content pref service except for the nsIModule implementation that registers it with the component registry, it continues to leak, but if I remove the factory that the module returns from its getClassObject method, the leak goes away.

The leak doesn't seem to be specific to my implementation of the factory, since I can reproduce the leak with a factory as simple as an empty object literal ({}).  I also tried replacing my entire nsIModule and nsIFactory implementations with ones created by Ted Mielczarek's JavaScript Component Wizard <http://ted.mielczarek.org/code/mozilla/jscomponentwiz/> in case the issue was related to something else in my implementation, and I observed the same leak.

The leak does go away with the patch to bug 180380, however, so making this bug depend on that one.  Given my tests, I suspect that *every* JS component with a factory is leaking its factory, and this bug may be responsible for leaks reported in bug 337050, bug 378618, bug 381239, and bug 380873/bug 380931.

I'll add a note to those bugs pointing to this comment and bug 180380.

Comment 3

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

Now that bug 180380 has been resolved, this leak is gone.