Closed Bug 1406923 Opened 7 years ago Closed 5 years ago

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

Categories

(WebExtensions :: Frontend, defect, P3)

Product:
WebExtensions
Component:
Frontend
Type:
defect

Tracking

(firefox57 affected)

Status:
RESOLVED WORKSFORME
Tracking Flags:
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.
status-firefox57: --- → affected
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: 5 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.