User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111203 Firefox/11.0a1 Build ID: 20111203031109 Steps to reproduce: 1. In Nightly, use a new profile and open https://addons.mozilla.org/en-US/firefox/ and about:memory?verbose in separate tabs; 2. Close AMO and click Minimize memory usage, GC, CC a dozen times in about:memory; Actual results: A zombie compartment is created. Expected results: The compartment should disappear. However it is not happening in Firefox 7.0.1 and 8.0.1.
I can reproduce this with all extensions disabled. I'll leave my browser up for a while to see how long the zombie lasts.
There's a zombie compartment with the addon installer, but it sounds like this happens even without that.
Yes. And the compartment is still there after about an hour or so.
Nightly build- Last GOOD: 2011-11-30 built from: http://hg.mozilla.org/mozilla-central/rev/cc94a16983b0 First BAD: 2011-12-01 built from: http://hg.mozilla.org/mozilla-central/rev/e9686560b98d
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cc94a16983b0&tochange=e9686560b98d Nothing in there sticks out at me, so we may need to bisect further.
cc94a16 is a bad changeset for me.
I wonder if there's a difference between debug and release builds here, since my manual bisecting doesn't seem to match up with what I'm seeing with mozregression.
Sorry, I don't have cycles atm to bisect this. (Literally...my CPU is tied up doing other builds.)
Regression window(cached m-c) Works http://hg.mozilla.org/mozilla-central/rev/cc94a16983b0 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111129 Firefox/11.0a1 ID:20111129214819 Fails http://hg.mozilla.org/mozilla-central/rev/639fd053363e Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111130 Firefox/11.0a1 ID:20111130034720 Pushlog http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cc94a16983b0&tochange=639fd053363e Regression window(cached m-i) Works http://hg.mozilla.org/integration/mozilla-inbound/rev/b10b930500f1 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111129 Firefox/11.0a1 ID:20111129084520 Fails http://hg.mozilla.org/integration/mozilla-inbound/rev/67451553bcbb Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111129 Firefox/11.0a1 ID:20111129110720 Pushlog http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b10b930500f1&tochange=67451553bcbb
Thanks, Alice. Looks like this must be bug 697383.
Confirmed on local build. This problem happens after landing of Bug 697383.
Fabrice, lets check your code what global object is used when the property is resolved. We might be holding on to the global of the page somehow.
Hmm. At what point does the init() code that sets this._window (and adds unload listeners, for that matter) run?
(In reply to Boris Zbarsky (:bz) from comment #13) > Hmm. At what point does the init() code that sets this._window (and adds > unload listeners, for that matter) run? When navigator.mozApps is first accessed. See https://mxr.mozilla.org/mozilla-central/source/dom/base/nsDOMClassInfo.cpp#7088 I wonder if setting this._window = null on "unload" would help (here: https://mxr.mozilla.org/mozilla-central/source/dom/base/Webapps.js#240)
We should file a followup there to not use an unload listener too....
(In reply to Boris Zbarsky (:bz) from comment #15) > We should file a followup there to not use an unload listener too.... Oh, interesting. What should we use instead? (This is relevant to my interests.)
I think the inner-window-destroyed notification would work better (e.g. wouldn't inhibit bfcache). Note, by the way, that what you probably have there is a ref to the _outer_ window... Not sure whether this causes a problem in practice or not.
Setting this._window to null fixes the issue. Boris, what's the way to listen for the inner-window-destroyed notification?
It's an observer service notification. There's some data about it at https://developer.mozilla.org/en/Observer_Notifications#Windows and there are existing consumers similar to yours in ConsoleAPIStorage.jsm and ConsoleAPI.js that use it if you want to take a look. I wonder why we need the explicit this._window set. Is the |this| object there not being CCed for some reason?
In case I'm not at MemShrink tomorrow morning: This apparently happens because AMO frobs navigator.mozApps. It should be fixed, but it's not a big MemShrink priority.
> This patch uses the outer-window-destroyed observer That seems a bit odd. I'd think this object's lifetime should be tied to the inner window, not the outer... I guess this._window is an outer window, though....
> That seems a bit odd. I'd think this object's lifetime should be tied to > the inner window, not the outer... I guess this._window is an outer window, > though.... Because you need to close the window in order to observe the zombie compartment here, the leak is actually tied to the outer window's lifetime. The reason for that is that if the window navigates (and we fire inner-window-destroyed) then the outer window that we're holding on to gets transplanted to hold on to the *new* inner (which isn't a zombie). Then, when the window is closed, we fire outer-window-destroyed and null out our reference. I think it's conceptually cleaner to listen for inner-window-destroyed (using nsDOMWindowUtils.currentInnerWindowID to wait for the correct notification) since that way we won't have a reference to a window that we don't really want to keep alive nor care about.
Created attachment 579251 [details] [diff] [review] updated patch Now using the innerwindowID.
Created attachment 579869 [details] [diff] [review] new updated patch Per discussion with blake, we now save the windowInnerId. Tested both when closing the tab and navigating away to another site. This prevents a leak of about 5MB when going to addons.mozilla.org
Comment on attachment 579869 [details] [diff] [review] new updated patch Thanks.
Verifying FIXED using latest hourly build based on cset: https://hg.mozilla.org/mozilla-central/rev/98db2311a44c Addons.mozilla.org now closes OK without the need to press any force clean buttons. Just wait a minute of less, and refresh the about:memory page and its now gone.