Closed Bug 707403 Opened 13 years ago Closed 13 years ago

When opening a tab in background, zombie compartment is created by Scriptish/Greasemonkey with some user scripts (possibly due to GM_registerMenuCommand)

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Platform:
x86_64
Windows 7
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: Fanolian+BMO, Unassigned)

References

Details

(Whiteboard: [MemShrink:P2]) 

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111202 Firefox/11.0a1
Build ID: 20111202031055

Steps to reproduce:

I can create a zombie compartment for certain with the following steps:

1. In a new profile, install the latest Scriptish/Greasemonkey and YouTube Link Title (http://userscripts.org/scripts/show/83584).
2. In about:config, set browser.tabs.loadBookmarksInBackground to true.
3. Restart Firefox and open about:memory.
4. Open a new tab in the background, by middle clicking, from any page in the default bookmark, e.g. Booksmark Toolbar > Getting Started.
5. Close the background tab and wait for few seconds, then reload about:memory.

The Getting Started page now becomes a zombie compartment. In fact, any pages that opened in the background and trigger the userscript will become zombie compartments. If a tab is opened in foreground, or if the page does not trigger any offending userscripts, it does not become a zombie compartment after closing it.

In summary:
Background new tab + offending scripts in effect = Zombie
Background new tab + non-offending scripts in effect = OK
Foreground new tab + offending scripts = OK
Foreground new tab + non-offending scripts = OK

Another script that will create zombie compartments in a similar manner (but on YouTube pages only) is YousableTubeFix (http://userscripts.org/scripts/show/13333), which has a much larger user base.

Zombie compartment of closing the tab for a minute:
│  ├───1,746,384 B (02.33%) -- compartment(http://www.mozilla.org/en-US/firefox/central/)
│  │   ├────897,024 B (01.20%) -- gc-heap
│  │   │    ├──417,336 B (00.56%) -- arena
│  │   │    │  ├──410,112 B (00.55%) -- unused [2]
│  │   │    │  ├────3,720 B (00.00%) -- padding [2]
│  │   │    │  └────3,504 B (00.00%) -- headers [2]
│  │   │    ├──257,584 B (00.34%) -- objects
│  │   │    │  ├──131,592 B (00.18%) -- function [2]
│  │   │    │  └──125,992 B (00.17%) -- non-function [2]
│  │   │    ├──125,640 B (00.17%) -- shapes
│  │   │    │  ├──113,560 B (00.15%) -- tree [2]
│  │   │    │  └───12,080 B (00.02%) -- dict
│  │   │    ├───72,896 B (00.10%) -- scripts [2]
│  │   │    ├───21,184 B (00.03%) -- type-objects [2]
│  │   │    └────2,384 B (00.00%) -- strings
│  │   ├────327,680 B (00.44%) -- mjit-code
│  │   │    ├──315,140 B (00.42%) -- method
│  │   │    └───12,540 B (00.02%) -- unused
│  │   ├────250,208 B (00.33%) -- analysis-temporary [2]
│  │   ├────140,080 B (00.19%) -- script-data [2]
│  │   ├─────64,816 B (00.09%) -- shapes-extra
│  │   │     ├──52,480 B (00.07%) -- tree-tables [2]
│  │   │     ├───7,648 B (00.01%) -- tree-shape-kids [2]
│  │   │     ├───3,008 B (00.00%) -- dict-tables
│  │   │     └───1,680 B (00.00%) -- empty-shape-arrays [2]
│  │   ├─────44,240 B (00.06%) -- object-slots [2]
│  │   ├─────17,232 B (00.02%) -- type-inference
│  │   │     ├──12,048 B (00.02%) -- object-main [2]
│  │   │     └───5,184 B (00.01%) -- tables [2]
│  │   └──────5,104 B (00.01%) -- string-chars
Blocks: ZombieCompartments, LeakyAddons
Summary: Zombie compartments with Scriptish/Greasemonkey and some user scripts when opening a tab in background → When opening a tab in background, zombie compartment is created by Scriptish/Greasemonkey and some user scripts
Whiteboard: [MemShrink]
Nice details steps to reproduce!  Does clicking on minimize memory usage make the compartment go away?  Just reloading about:memory will only recompute memory usage, it won't necessarily free anything up.
The compartment stays after closing the tab and constantly clicking "Minimize memory usage", "GC" and "CC" for about a minute.
Okay, great.  I figured that was the case but I thought I should double check.
Confirming the issue here on latest nightly(20111203). Disabling Scriptish fixed zombie compartement issues that were present with it enabled.
(In reply to Paul-Kenji Cahier from comment #4)
> Confirming the issue here on latest nightly(20111203). Disabling Scriptish
> fixed zombie compartement issues that were present with it enabled.

Or disabling that particular user script only. Before loading the tab in background of course.

A blank, or with non-affecting user scripts enabled, Scriptish/Greasemonkey does not create zombie compartments.
Happens to me as well since Firefox 6 I guess. Had to disable some scripts to get rid of the problem.
The script YouTube Link Title has an updated version which should have eliminated the zombie compartment problem.

The changelog says:
"2011-12-04: Removed entry from Greasemonkey menu because GM_registerMenuCommand() seems to cause zombie compartments. To access the settings, you now have to click on one of the YouTube/Vimeo/LiveLeak icons"

If someone else can further confirm it, please mark this bug as invalid. Thank you.
That's not a real fix though. The fix would need to be in greasemonkey/Scriptish.
https://scriptish.lighthouseapp.com/projects/83146/tickets/563-gm_registermenucommand-causes-zombie-compartments
Another Scriptish user sees a similar symptom and confirm that it is caused by GM_registerMenuCommand.
Summary: When opening a tab in background, zombie compartment is created by Scriptish/Greasemonkey and some user scripts → When opening a tab in background, zombie compartment is created by Scriptish/Greasemonkey with some user scripts (possibly due to GM_registerMenuCommand)
MemShrink:P2 because GreaseMonkey is a very popular add-on (#3 on AMO).  Anyone know how to contact the GreaseMonkey authors?
Whiteboard: [MemShrink] → [MemShrink:P2]
Greasemonkey github page: https://github.com/greasemonkey/greasemonkey
The author states in his AMO page that "Please do not contact me directly about Greasemonkey". I guess github is the best way.
Greasemonkey dev here.  (The text on my AMO page does continue to say please contact the list.)  This is the first time I've heard the term zombie compartment and could certainly use help addressing this.  What should I be looking for?
You can see an overview of zombie compartments here: https://developer.mozilla.org/en/Zombie_compartments

Perhaps somebody else is more familiar with the specifics mentioned in Comment 9.
So, when a command is registered, a reference to the related content window is stored in (an object stored in) a (component) global array ( https://github.com/greasemonkey/greasemonkey/blob/master/components/greasemonkey.js#L216 ).  It's supposed to be removed when the window/tab is closed ( https://github.com/greasemonkey/greasemonkey/blob/master/components/greasemonkey.js#L386 ) but perhaps it's not being.  Or is this a big enough error that even removing the reference won't kill the zombie?
> Or is this a big enough
> error that even removing the reference won't kill the zombie?

It might be worth trying this:  null out the reference before deleting it.  Just to see if it makes a difference?
Closed in Scriptish: 
https://github.com/scriptish/scriptish/commit/297721ae2f178008ea22449a936ea8e728ef80c4

Thank you for the detailed reports!
Fanolian, Paul-Kenji: are either of you able to confirm the fix?  I'd like confirmation before we mark it fixed.  Thanks.
(In reply to Nicholas Nethercote [:njn] from comment #17)
> Fanolian, Paul-Kenji: are either of you able to confirm the fix?  I'd like
> confirmation before we mark it fixed.  Thanks.

If someone would like to test Scriptish, then use a nightly build: https://github.com/scriptish/scriptish-nightlies/downloads
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111218 Firefox/11.0a1

The issue is fixed tested with Scriptish nightly-0.1.6a2pre.20111219.0400 build.
However, Greasemonkey has not yet fixed the issue in its latest nightly(2011.12.09) build which can be found at https://arantius.com/misc/gm-nightly/.

I think this bug cannot be marked as fixed at this moment.
We haven't submitted a fix to GM yet (downstream: https://github.com/greasemonkey/greasemonkey/issues/1482 ).  So far I haven't been able to discern the cause.  I can't find anywhere that anything (explicitly) acts different whether it's a foreground or background tab in relation to this code.  And the code path we've always had for cleaning this sort of thing up is essentially the same as in Scriptish's fix (observe inner-window-destroyed, clean up references at that point).
Copied from GM's issue:

This should fix things. Please check the next nightly (after 2012.01.08, should be up in a few hours) available at:

https://arantius.com/misc/gm-nightly/

To confirm.
Anthony, can you describe at a high-level what the problem was?  I'm asking because I'm trying to improve the documentation to help add-on authors avoid memory leaks, and so I'm wondering if this is a case that might be common.  I looked at https://github.com/arantius/greasemonkey/commit/40993dd69ad110df4578170da73c16aab08cc8b0 but didn't understand it.  Thanks!
(In reply to Anthony Lieuallen from comment #21)
> Copied from GM's issue:
> 
> This should fix things. Please check the next nightly (after 2012.01.08,
> should be up in a few hours) available at:
> 
> https://arantius.com/misc/gm-nightly/
> 
> To confirm.

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20120109 Firefox/12.0a1

I can confirm that this issue is fixed in greasemonkey-2012.01.09.nightly in Nightly and Firefox 9.0.1. Thank you Anthony Lieuallen and Erik Vold.
(In reply to Nicholas Nethercote [:njn] from comment #22)
> Anthony, can you describe at a high-level what the problem was?

The best explanation I have is:

- When a user script calls GM_registerMenuCommand() we take note of (especially, among some other data): The callback function, the related content window, and the related inner content window's ID (if known).
- When we notice that a window is closed (Firefox 3: observe dom-window-destroyed, Firfeox 4+: observe inner-window-destroyed, either: listen for pagehide on gBrowser with persisted==false) we scan these entries to see which, if any, are stale.
- This patch guarantees that every time, we scan the entire set for either of two reasons: the content window's ".closed" is truthy, or the closing inner window's ID matches.

I believe that in Greasemonkey this was never a real _leak_.  We just held on to some references longer than we needed to, sometimes.  At some point, we would scan the whole list, and clean up by ".closed".  Previously, the scan did not run if an inner window ID was not passed.  This patch skips the ID check ahead of time, and instead scans the whole set every time.  So clean up happens more often.

The reason the compartment hung around is because we were holding an explicit content window reference, but also a closure reference via the callback.
Thanks for the detailed explanation, Anthony.  I guess the lesson is that when an add-on holds references to windows, it needs to be really careful to release them at appropriate times.
Can this be marked as fixed now?
Yes!
Status: UNCONFIRMED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.