1406923 - browser.xul leak in extensions/ext-browser.js

Status: RESOLVED WORKSFORME

(WebExtensions :: Frontend, defect, P3)

Description

[Ben Kelly [:bkelly, not reviewing]]

I found I had a browser.xul window leak in FF57 beta today.  I analyzed the GC/CC logs and found that its being held alive via TabTracker's _tabIds property in ext-browser.js.

bkelly@valen:/mnt/c/devel/tmp/cclogs$ /mnt/c/devel/heapgraph/find_roots.py cc-edges.13384.1507553472.log 0000019D681B5800
Parsing cc-edges.13384.1507553472.log. Done loading graph.

0000019D6989D060 [JS Object (Window)]
    --[UnwrapDOMObject(obj)]--> 0000019D681B5800 [nsGlobalWindow # 29 inner chrome://browser/content/browser.xul]

    Root 0000019D6989D060 is a marked GC object.

bkelly@valen:/mnt/c/devel/tmp/cclogs$ /mnt/c/devel/heapgraph/find_roots.py gc-edges.13384.1507553472.log -bro 0000019D6989D060
Parsing gc-edges.13384.1507553472.log. Done loading graph.

via mCallback :
0000019D71780080 [Proxy <no private>]
    --[proxy target]--> 0000019D63906F80 [Object <no private>]
    --[_tabIds]--> 0000019D6392E180 [Proxy <no private>]
    --[proxy target]--> 0000019D6392E200 [Map 0000019D5FF12E20]
    --[value]--> 0000019D6EDEA600 [Proxy <no private>]
    --[proxy target]--> 0000019D70DF8730 [XULElement <no private>]
    --[group]--> 0000019D69882E20 [object_group]
    --[group_global]--> 0000019D6989D060 [Window <no private>]

The only non-standard addon I have installed is the multi-account containers addon.  I don't think that is a web-extension, though.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Actually, it does seem to be related to multi-account container addon.  STR:

1. Open a new browser window.
2. Open a container tab in that window.
3. Load a content page in the container tab.  I used example.com.
4. Close the window with the container tab still active.
5. Open about:memory in another window/tab in same session.
6. Minimize and measure.  You will observer an increasing amount of browser.xul memory under detached windows in the parent process.

Kris, does this look like a platform issue or a problem with the addon itself?

Comment 2

[Jonathan Kingston [:jkt] he/him]

:bkelly could you try opening the addon as a web extension using the web-ext command?

https://github.com/mozilla/multi-account-containers/blob/master/README.md#web-extension-development

We inject content scripts but I think we rely on Firefox to do memory cleanup. This to me seems like a Firefox issue but perhaps we can mitigate somehow.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

This is 100% reproduceable for me.  Does it not happen for you?  I'm not sure when I will have time to do any investigation here.  I'm a bit slammed at the moment.

Comment 4

[Kris Maglione [:kmag]]

It looks like a platform issue. Apparently we're not getting a tab removed event for the tab.

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Thanks for taking this.  It would be great to include a test like this as part of the fix:

https://dxr.mozilla.org/mozilla-release/source/addon-sdk/source/test/leak/leak-utils.js#38

Comment 6

[Kris Maglione [:kmag]]

This would show up in regular window leak checks if it were happening during tests. That map is global, and lasts until the end of the browser session.

Comment 7

[Eric Rahm [:erahm]]

Kris, are you still looking into this?

Comment 8

[Kris Maglione [:kmag]]

Yes, but I've had higher priorities for the past few weeks.

Comment 9

[Shane Caraveo (:mixedpuppy)]

re-verify whether this is happening

Comment 10

[Shane Caraveo (:mixedpuppy)]

At least on osx, I am not seeing a leak based on str in comment 1.

On startup, I have 4 lines with browser.xhtml. After running STR, I have 6. Mimize memory and re-report, I'm back to 4.

I also verified that we are getting tab-removed events in ext-browser.js.

I do see an exception (only when running the multi account container extension), but it doesn't appear to leave a detached window anywhere:

Extension error: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMWindowUtils.removeSheetUsingURIString]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: resource://gre/modules/ExtensionCommon.jsm :: runSafeSyncWithoutClone :: line 75" data: no] undefined 75
[[Exception stack
runSafeSyncWithoutClone@resource://gre/modules/ExtensionCommon.jsm:75:12
cleanup@resource://gre/modules/ExtensionContent.jsm:404:11
close@resource://gre/modules/ExtensionContent.jsm:925:14
inner-window-destroyed@resource://gre/modules/ExtensionContent.jsm:1010:19
observe@resource://gre/modules/ExtensionContent.jsm:1028:27
Current stack
runSafeSyncWithoutClone@resource://gre/modules/ExtensionCommon.jsm:81:9
cleanup@resource://gre/modules/ExtensionContent.jsm:404:11
close@resource://gre/modules/ExtensionContent.jsm:925:14
inner-window-destroyed@resource://gre/modules/ExtensionContent.jsm:1010:19
observe@resource://gre/modules/ExtensionContent.jsm:1028:27
]]

Comment 11

[Shane Caraveo (:mixedpuppy)]

I'm closing given I am getting the necessary tab-removed events.