1296094 - devtools leaks browser.xul through webbrowser.js _actorByBrowser Map()

Status: RESOLVED INACTIVE

(DevTools :: General, defect, P3)

Description

[Ben Kelly [:bkelly, not reviewing]]

In firefox 48 I recently got a leaked browser.xul when I accidentally closed a window.  (So I don't know exactly what I had open...)

The retaining path is:

$ /c/devel/heapgraph/find_roots.py cc-edges.2200.1471467236.log 0000022799F35000
Parsing cc-edges.2200.1471467236.log. Done loading graph.

000002279B807060 [JS Object (Window)]
    --[UnwrapDOMObject(obj)]--> 0000022799F35000 [nsGlobalWindow # 98 inner chrome://browser/content
/browser.xul]

    Root 000002279B807060 is a marked GC object.


bkelly@kosh /c/devel/tmp/cclogs/browser
$ /c/devel/heapgraph/find_roots.py gc-edges.2200.1471467236.log -bro 000002279B807060
Parsing gc-edges.2200.1471467236.log. Done loading graph.

via mCallback :
00000227BFE43780 [Proxy <no private>]
    --[private]--> 000002279CADE100 [Object <no private>]
    --[_actorByBrowser]--> 00000227CAA84580 [Map 2278e3ff7c0]
    --[key]--> 00000227D0AC0240 [Proxy <no private>]
    --[private]--> 000002278AFC4B50 [XULElement <no private>]
    --[shape]--> 00000227BFC87858 [shape]
    --[base]--> 000002279C504510 [base_shape]
    --[global]--> 000002279B807060 [Window <no private>]

This is a Map() which holds browser objects alive strongly in its key:

https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/webbrowser.js#257

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

There are a few spots that use the map's size, but I think it could be become a WeakMap relatively easily.

If anyone can construct a test case that reproduces this issue, that would be wonderful to have!

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

I was unable to reproduce with simple steps like closing the window with devtools open.  Not sure what I did!

Maybe I fat-fingered a ctrl-t and ctrl-w at the same time?

Comment 3

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Presumably [0] and [1] are supposed to clean this up in the face of tabs/windows closing. Strange that there's an executeSoon() in there... The comment about deadlocks (in C++ and triggered from JS!?) is not exactly inspiring.

[0] http://searchfox.org/mozilla-central/source/devtools/server/actors/webbrowser.js#61
[1] http://searchfox.org/mozilla-central/source/devtools/server/actors/webbrowser.js#735

Comment 4

[Patrick Brosset <:pbro>]

Moving this inactive P2 to the backlog (P3)