Last Comment Bug 707403 - When opening a tab in background, zombie compartment is created by Scriptish/Greasemonkey with some user scripts (possibly due to GM_registerMenuCommand)
: When opening a tab in background, zombie compartment is created by Scriptish/...
Status: RESOLVED FIXED
[MemShrink:P2]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86_64 Windows 7
: -- normal (vote)
: ---
Assigned To: general
:
:
Mentors:
Depends on:
Blocks: LeakyAddons ZombieCompartments
  Show dependency treegraph
 
Reported: 2011-12-02 20:09 PST by Fanolian
Modified: 2012-01-12 15:32 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description Fanolian 2011-12-02 20:09:18 PST
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
Comment 1 Andrew McCreight [:mccr8] 2011-12-02 20:16:48 PST
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.
Comment 2 Fanolian 2011-12-02 20:22:07 PST
The compartment stays after closing the tab and constantly clicking "Minimize memory usage", "GC" and "CC" for about a minute.
Comment 3 Andrew McCreight [:mccr8] 2011-12-02 20:24:15 PST
Okay, great.  I figured that was the case but I thought I should double check.
Comment 4 Paul-Kenji Cahier 2011-12-03 04:29:47 PST
Confirming the issue here on latest nightly(20111203). Disabling Scriptish fixed zombie compartement issues that were present with it enabled.
Comment 5 Fanolian 2011-12-03 10:12:55 PST
(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.
Comment 6 LeppeRMessiaH 2011-12-03 21:57:32 PST
Happens to me as well since Firefox 6 I guess. Had to disable some scripts to get rid of the problem.
Comment 7 Fanolian 2011-12-04 13:54:08 PST
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.
Comment 8 Paul-Kenji Cahier 2011-12-04 14:08:49 PST
That's not a real fix though. The fix would need to be in greasemonkey/Scriptish.
Comment 9 Fanolian 2011-12-06 10:57:32 PST
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.
Comment 10 Nicholas Nethercote [:njn] 2011-12-06 14:13:30 PST
MemShrink:P2 because GreaseMonkey is a very popular add-on (#3 on AMO).  Anyone know how to contact the GreaseMonkey authors?
Comment 11 Fanolian 2011-12-07 03:26:10 PST
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.
Comment 12 Anthony Lieuallen 2011-12-07 11:41:27 PST
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?
Comment 13 Andrew McCreight [:mccr8] 2011-12-07 11:44:20 PST
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.
Comment 14 Anthony Lieuallen 2011-12-07 13:01:58 PST
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?
Comment 15 Nicholas Nethercote [:njn] 2011-12-07 21:46:02 PST
> 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?
Comment 16 Erik Vold 2011-12-16 17:55:04 PST
Closed in Scriptish: 
https://github.com/scriptish/scriptish/commit/297721ae2f178008ea22449a936ea8e728ef80c4

Thank you for the detailed reports!
Comment 17 Nicholas Nethercote [:njn] 2011-12-16 19:02:12 PST
Fanolian, Paul-Kenji: are either of you able to confirm the fix?  I'd like confirmation before we mark it fixed.  Thanks.
Comment 18 Erik Vold 2011-12-18 13:28:53 PST
(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
Comment 19 Fanolian 2011-12-18 20:12:40 PST
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.
Comment 20 Anthony Lieuallen 2011-12-18 20:37:40 PST
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).
Comment 21 Anthony Lieuallen 2012-01-08 19:38:49 PST
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.
Comment 22 Nicholas Nethercote [:njn] 2012-01-08 20:29:35 PST
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!
Comment 23 Fanolian 2012-01-09 12:29:38 PST
(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.
Comment 24 Anthony Lieuallen 2012-01-09 15:26:16 PST
(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.
Comment 25 Nicholas Nethercote [:njn] 2012-01-09 15:40:02 PST
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.
Comment 26 Fanolian 2012-01-12 12:07:45 PST
Can this be marked as fixed now?
Comment 27 Nicholas Nethercote [:njn] 2012-01-12 15:32:10 PST
Yes!

Note You need to log in before you can comment on or make changes to this bug.