Closed Bug 1406923 Opened 4 years ago Closed 2 years ago

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

Categories

(WebExtensions :: Frontend, defect, P3)

defect

Tracking

(firefox57 affected)

RESOLVED WORKSFORME
Tracking Status
firefox57 --- affected

People

(Reporter: bkelly, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [MemShrink:P2], webext?)

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.
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?
Flags: needinfo?(kmaglione+bmo)
: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.
Flags: needinfo?(bkelly)
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.
Flags: needinfo?(bkelly)
It looks like a platform issue. Apparently we're not getting a tab removed event for the tab.
Assignee: nobody → kmaglione+bmo
Component: WebExtensions: Untriaged → WebExtensions: Frontend
Flags: needinfo?(kmaglione+bmo)
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
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.
Priority: -- → P2
Whiteboard: [MemShrink] → [MemShrink:P2]
Kris, are you still looking into this?
Flags: needinfo?(kmaglione+bmo)
Yes, but I've had higher priorities for the past few weeks.
Flags: needinfo?(kmaglione+bmo)
Product: Toolkit → WebExtensions
Assignee: kmaglione+bmo → nobody
Priority: P2 → P3
Whiteboard: [MemShrink:P2] → [MemShrink:P2], webext?

re-verify whether this is happening

Flags: needinfo?(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
]]

Flags: needinfo?(mixedpuppy)

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

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.