1181122 - Ghost window after opening developer tool with userscript Youtube Link Title

Status: RESOLVED DUPLICATE of bug 1174950

(DevTools :: General, defect)

Description

[Fanolian]

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:42.0) Gecko/20100101 Firefox/42.0
Build ID: 20150706030206

Steps to reproduce:

Preparation:
1. In a new profile, install Greasemonkey 3.2: https://addons.mozilla.org/en-US/firefox/addon/greasemonkey/versions/?page=1#version-3.2 . Restart Nightly.
2. Install the script Youtube Link Title v2015.6.24: https://greasyfork.org/en/scripts/413-youtube-link-title/versions . Restart Nightly.

Steps to reproduce:
1. On the first tab, open about:memory.
2. On the second tab, open any dummy URL, e.g. google.com . It is only to prevent Web Content (Plugin Container) getting terminated after closing all other tabs except about:memory.
3. On the third tab, open http://example.com
4. On the third tab, open developer tool by right click -> Inspect Element, or from the hamburger menu.
5. Close the third tab.
6. On the first tab (about:memory), minimize memory usage more than once. (The first time does not show the ghost windows.)

Actual result:
Ghost windows are created at the Web Content:
130 (100.0%) -- event-counts
└──130 (100.0%) -- window-objects/top(https://www.google.com.hk/, id=4294967297)/active/window(https://www.google.com.hk/)/dom
   ├──129 (99.23%) ── event-listeners
   └────1 (00.77%) ── event-targets

2 (100.0%) -- ghost-windows
└──2 (100.0%) ── http://example.com/ [2]

Meanwhile in Main Process:
1,874 (100.0%) -- event-counts
└──1,874 (100.0%) -- window-objects
   ├──1,352 (72.15%) -- top(chrome://browser/content/browser.xul, id=3)/active
   │  ├──1,351 (72.09%) -- window(chrome://browser/content/browser.xul)/dom
   │  │  ├──1,349 (71.99%) ── event-listeners
   │  │  └──────2 (00.11%) ── event-targets
   │  └──────1 (00.05%) ── window(about:blank)/dom/event-targets
   ├────268 (14.30%) -- top(none)/detached
   │    ├──181 (09.66%) ── window(chrome://browser/content/devtools/framework/toolbox.xul)/dom/event-listeners
   │    └───87 (04.64%) ── window(chrome://browser/content/devtools/inspector/inspector.xul)/dom/event-listeners
   ├────219 (11.69%) -- top(about:newtab, id=82)/active/window(about:newtab)/dom
   │    ├──218 (11.63%) ── event-listeners
   │    └────1 (00.05%) ── event-targets
   └─────35 (01.87%) ++ (4 tiny)

   
Or in a real-life example, cnn.com (showing Web Content only):
679 (100.0%) -- event-counts
└──679 (100.0%) -- window-objects
   ├──548 (80.71%) ── top(none)/ghost/window(http://edition.cnn.com/)/dom/event-listeners [9]
   └──131 (19.29%) -- top(https://www.google.com.hk/, id=2147483652)/active/window(https://www.google.com.hk/)/dom
      ├──130 (19.15%) ── event-listeners
      └────1 (00.15%) ── event-targets

20 (100.0%) -- ghost-windows
└──20 (100.0%) ── http://edition.cnn.com/ [20]

Although the issue is triggered by a particular userscript, the issue is a regression from a Firefox change.

From mozregression
LAST GOOD Nightly: 2015-03-26
FIRST BAD Nightly: 2015-03-27
Last good revision: 37d3dcbf23a9
First bad revision: e046475a75cb
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=37d3dcbf23a9&tochange=e046475a75cb

Inbound:
Last good revision: 9d23ce7d6739
First bad revision: 2446d768506c
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9d23ce7d6739&tochange=2446d768506c

The regression range is in version 39, but I cannot reproduce the issue on 39 Release. 40 Beta is affected.

Note:
The issue cannot be reproduced with a clean profile, nor Greasemonkey without any scripts enabled.
It looks like the leak occurs only once per session. No more ghost windows or detached/ghost window-objects are created if I retry on another domain.
I can reproduce it either with e10s or not.
Script author is notified via https://greasyfork.org/en/forum/discussion/4993/firefox-40-memory-leak-after-opening-firefox-developer-tool-with-ylt-enabled

Comment 1

[Fanolian]

Attachment: memory-report.json.gz

Memory report after creating ghost windows.

Comment 2

[Andrew McCreight [:mccr8]]

Is it possible to use mozregression to work on another branch? From the inbound link, it looks like it wasn't introduce on inbound, but on another branch that got merged over there.

Comment 3

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

(In reply to Andrew McCreight [:mccr8] from comment #2)
> Is it possible to use mozregression to work on another branch? From the
> inbound link, it looks like it wasn't introduce on inbound, but on another
> branch that got merged over there.

There is an option for "--inbound-branch", so "mozregression --inbound-branch fx-team" might work here.

Comment 4

[Eric Rahm [:erahm]]

We think this is related to bug 1174950.

Comment 5

[Andrew McCreight [:mccr8]]

Thanks for the detailed steps to reproduce. This looks like the same issue as bug 1174950 based on the way the windows are leaking, and my patch there seems to fix it.  However, I'm still intrigued by the regression range in comment 0.  Maybe there was some kind of Addon SDK or other front end change that caused this to leak. We should fix that, too, so I'll leave this bug open to figure that out.

Comment 6

[Andrew McCreight [:mccr8]]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #3)
> There is an option for "--inbound-branch", so "mozregression
> --inbound-branch fx-team" might work here.

That worked, thanks. I got it down to a checkin-needed landing on fx-team.

12:44.64 LOG: MainThread Bisector INFO Last good revision: ad9f9f565c39
12:44.64 LOG: MainThread Bisector INFO First bad revision: f94c011d8270
12:44.64 LOG: MainThread Bisector INFO Pushlog:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=ad9f9f565c39&tochange=f94c011d8270

Comment 7

[Andrew McCreight [:mccr8]]

A local bisection points to bug 1134180 as introducing this leak.

Comment 8

[Andrew McCreight [:mccr8]]

Alexandre, could please you look at figuring out why your patch caused Greasemonkey to start leaking? A newer version of Greasemonkey might fix this (as seen in bug 1174950 comment 11, though I haven't confirmed that myself), but it is alarming that a change could cause addons to start leaking.

Comment 9

[Alexandre Poirot [:ochameau]]

That sounds very unlikely, as this patch only modify code related to devtools.
That code shouldn't be even loaded unless we start opening the tools!
But I'll take a look.

Comment 10

[Alexandre Poirot [:ochameau]]

Ah. I read the bug too quickly... it involves devtools! So yes, it can easily be related to that patch. I'm trying to reproduce without much success so far, but I'm using a somewhat clean profile.

Comment 11

[Alexandre Poirot [:ochameau]]

I'm finally able to reproduce it, but only with a nightly build.
If I build my own firefox, from today's master with an empty mozconfig,
I'm not able to reproduce it (even when using the same profile I used with my nightly build).

Any tip to help reproducing it is welcomed!

Comment 12

[Andrew McCreight [:mccr8]]

(In reply to Alexandre Poirot [:ochameau] from comment #11)
> Any tip to help reproducing it is welcomed!

You have to back out bug 1174950, which fixes the ghost window part of it (but I think not the leaking sandbox).

Comment 13

[Fanolian]

I can no longer recreate the ghost windows after bug 1174950.
I cannot reproduce detached windows devtools/framework/toolbox.xul nor inspector.xul in Main Process neither. Perhaps it is also fixed by bug 1174950 or was a false positive.

I do reproduce a detached window inspector.xul with Element Hiding Helper for Adblock Plus, but it may relate to bug 1117816 instead of bug 1134180.

Comment 14

[Alexandre Poirot [:ochameau]]

I'm now able to reproduce it when I backout bug 1174950.
But I'm wondering, what is a ghost window?
I thought that thanks to NukeCrossCompartment we were not able to leak the content document/window anymore. We would just have references to the DeadWrapper singleton instead.
I'm trying to use CCAnalyser to help me figure this out, but need some more info about what I should look for.
Is ghost window about trying to also get rid of DeadWrapper references?

Comment 15

[Andrew McCreight [:mccr8]]

(In reply to Alexandre Poirot [:ochameau] from comment #14)
> I'm now able to reproduce it when I backout bug 1174950.
Great!

> But I'm wondering, what is a ghost window?
A ghost window is a content window that is detached and also does not share the same domain as a window that isn't detached. (The latter criteria is just to handle the case where a web page is holding alive an iframe or something.)

> I thought that thanks to NukeCrossCompartment we were not able to leak the
> content document/window anymore. We would just have references to the
> DeadWrapper singleton instead.
> I'm trying to use CCAnalyser to help me figure this out, but need some more
> info about what I should look for.

Before bug 1174950, NukeCrossCompartmentWrappers would only kill references from compartments with system principal to windows with compartments with non-system principals. What is happening here is that there is a sandbox, created by Greasemonkey, which has an expanded principal. An expanded principal is not a system principal, so we wouldn't kill the wrapper to the window. We also never would kill the reference to the sandbox from system principal code because it is a sandbox and not a window. My patch makes this more general, and kills references from expanded principal compartments to non-system principal windows.

> Is ghost window about trying to also get rid of DeadWrapper references?

No, the entire window is leaking.

My theory is that bug 1134180 is somehow causing us to leak the sandboxes where we weren't before, which is not fixed by bug 1174950. Leaking sandboxes is less of a problem than leaking entire windows.

Comment 16

[Alexandre Poirot [:ochameau]]

I'm looking at this but it is quite challenging to process the GC graph to figure this out.
So far I've found various leaks from devtools to content (given that they are from chrome sandboxes, using system principal, they should be killed by NukeCrossCompartmentwrappers).
But I haven't been able to see any leak from devtools to one of the greasemonkey/userscript sandboxes.

The userscript sandbox holds a ref to the document at line 108
YouTube_Link_Title/YouTube_Link_Title.user.js
108:  var d = document;

Then I'm seeing greasemonkey itself leaking the userscript sandbox:
utils.js
  commandFunc attribute -> Youtube.js:Setup.show

xmlhttprequester.js
  sandbox attribute -> Youtube.js:sandbox global
  itself holded by:
    hitch.js
    hitch() closure, via obj reference

storageFront.js
  GM_ScriptStorageFront
  _sandbox attribute -> Youtube.js:sandbox global

I have hard time understanding why we need devtools to enable this leak.
Was NukeCrossCompartmentWrapper, before bug 1174950, also killing references from Greasemonkey sandbox to the Userscript sandbox (chrome sandbox <--> expanded principal)?
That would explain why all these references to the Userscript sandbox are killed and do not introduce a leak by themselves.

I do not see any edge between devtools compartments and the Userscript sandbox.
I've seen edges from devtools to the content document, fixed them, but it doesn't prevent the leak.

Comment 17

[Andrew McCreight [:mccr8]]

(In reply to Alexandre Poirot [:ochameau] from comment #16)
> I have hard time understanding why we need devtools to enable this leak.
> Was NukeCrossCompartmentWrapper, before bug 1174950, also killing references
> from Greasemonkey sandbox to the Userscript sandbox (chrome sandbox <-->
> expanded principal)?
> That would explain why all these references to the Userscript sandbox are
> killed and do not introduce a leak by themselves.

No, Gecko only calls NukeCrossCompartmentWrapper on nsGlobalWindows, not sandboxes. Browser JS can call Cu.nukeSandbox() on a sandbox to kill references to the sandbox in a similar way, but that has to be done manually. Ideally Greasemonkey would call that on sandboxes it creates if it knows they should go away at a certain point.

> I do not see any edge between devtools compartments and the Userscript
> sandbox.
> I've seen edges from devtools to the content document, fixed them, but it
> doesn't prevent the leak.

That's odd. Maybe it isn't really worth spending any more time on this unless people start seeing additional problem.  Thanks for investigating!

Comment 18

[Alexandre Poirot [:ochameau]]

Actually, I see it leaking with my current patch queue, without even opening the devtools.
If I just revert bug 1174950, and do not open devtools, I see ghost windows, we need to wait quite a bit before they appear.

But that was a good time spent that highlighted two quite big leaks in devtools.

Should we close this bug then?

Comment 19

[Andrew McCreight [:mccr8]]

Yeah, I'll just dupe it to the other one, as it mostly fixed this.