Last Comment Bug 778318 - Greasemonkey user scripts "YousableTubeFix" and "Textarea backup with expiry" cause zombie compartments
: Greasemonkey user scripts "YousableTubeFix" and "Textarea backup with expiry"...
Status: RESOLVED FIXED
[MemShrink:P1]
: regression
Product: Tech Evangelism
Classification: Other
Component: Add-ons (show other bugs)
: unspecified
: x86 Windows 7
: -- major with 2 votes (vote)
: ---
Assigned To: Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
:
Mentors:
Depends on: 773980 781482
Blocks: LeakyAddons ZombieCompartments hueyfix 780331
  Show dependency treegraph
 
Reported: 2012-07-27 15:23 PDT by Zaid
Modified: 2012-08-31 05:58 PDT (History)
22 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Data from about:memory page (542.10 KB, text/plain)
2012-07-27 15:23 PDT, Zaid
no flags Details
Data from about:support page (9.24 KB, text/plain)
2012-07-27 15:34 PDT, Zaid
no flags Details
output of about:memory with verbose on, at point mentioned in the comment (106.10 KB, text/plain)
2012-08-21 10:33 PDT, Anthony Lieuallen
no flags Details

Description Zaid 2012-07-27 15:23:59 PDT
Created attachment 646739 [details]
Data from about:memory page

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:14.0) Gecko/20100101 Firefox/14.0.1
Build ID: 20120713134347

Steps to reproduce:

After a long period of web-browsing, I noticed that the memory usage was around 1.4 GB!! I closed all the other tab, ALL of them. Then I opened an about:memory tab. Clicked “minimize memory usage” numerous times, but the memory usage did not go down.


Actual results:

Memory usage remained almost unaffected.


Expected results:

Memory usage should go down. When using Firefox 14 under similar circumstances, the memory usage goes down to 400-500MB after this procedure.
Comment 1 Zaid 2012-07-27 15:34:29 PDT
Created attachment 646742 [details]
Data from about:support page
Comment 2 Jesper Hansen 2012-07-27 15:42:44 PDT
Try using safe-mode and see if you can replicate.
Comment 3 Zaid 2012-07-27 16:07:05 PDT
No, it works fine under safe-mode. It is an addons issue I believe. Most likely related to the Huey Fix.
Comment 4 Zaid 2012-07-27 16:21:08 PDT
I believe I have Identified the offending addon. It is Greasemonkey.
Comment 5 Andrew McCreight [:mccr8] 2012-07-27 16:23:33 PDT
That's pretty interesting!  This is what about:memory looked like after you clicked on Minimize Memory, right?  I guess you have a bazillion ghost windows, so it must be!

Do you have any GreaseMonkey scripts that are targeted YouTube or OkCupid in particular?  Those seem to make up the bulk of your ghost windows, but maybe those are just pages you end up closing a lot of.

I was going to guess ProxTube due to all of the YouTube. ;)
Comment 6 Zaid 2012-07-27 16:25:20 PDT
Youtube yes, OkCupid no. I have "Super Linkifier" and "Textarea backup with expiry".
Comment 7 Andrew McCreight [:mccr8] 2012-07-27 16:27:10 PDT
bug 711675 and bug 707403 have some previous reports of Greasemonkey causing zombie compartments, but it is a bit worrying that it can happen in 15.
Comment 8 Justin Lebar (not reading bugmail) 2012-07-27 16:32:57 PDT
(In reply to Zaid from comment #4)
> I believe I have Identified the offending addon. It is Greasemonkey.

The next step would be identifying which of your GM userscripts is causing this problem, so we can figure out if the problem is in GM, the userscript, or both.

Thanks for investigating this!
Comment 9 Zaid 2012-07-27 16:48:28 PDT
I think that "YousableTubeFix" (http://userscripts.org/scripts/show/13333) and "Textarea backup with expiry" (http://userscripts.org/scripts/show/42879) are the offending scripts. The textarea backup runs on all websites (by design).

Please someone use those scripts to try and replicate my result.
Comment 10 Justin Lebar (not reading bugmail) 2012-07-27 16:57:21 PDT
I can confirm that the youtube user script causes ghost compartments at youtube.com, and the textarea backup script causes ghost windows on pastebin.mozilla.org.
Comment 11 Justin Lebar (not reading bugmail) 2012-07-27 17:02:57 PDT
cc greasemonkey devs.
Comment 12 Zaid 2012-07-27 19:05:20 PDT
I am glad the issue has been identified. Hope this gets fixed. Thanks guys for your work on Firefox.

(In reply to Andrew McCreight [:mccr8] from comment #7)
> bug 711675 and bug 707403 have some previous reports of Greasemonkey causing
> zombie compartments, but it is a bit worrying that it can happen in 15.

I just want to stress that this is a regression from Firefox 14. These problems do not occur with Firefox 14, only Firefox 15.
Comment 13 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-07-27 19:28:39 PDT
Does Greasemonkey use sandboxes?  This sounds a lot like the jetpack issues we ran into.
Comment 14 Anthony Lieuallen 2012-07-27 19:32:43 PDT
If "use sandboxes" means "call Components.utils.evalInSandbox" then yes, Greasemonkey uses sandboxes extensively.  There's also https://github.com/greasemonkey/greasemonkey/issues/1578 which claims the problem is worse in 15.
Comment 15 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-07-27 19:47:44 PDT
Does Greasemonkey set up the sandbox's prototype to be the content page's window?  If so, does Greasemonkey store those sandboxes somewhere and clean them up when the page goes away?

If so, this sounds a lot like https://bugzilla.mozilla.org/show_bug.cgi?id=751420#c12.
Comment 16 Anthony Lieuallen 2012-07-27 19:53:36 PDT
In released versions, Greasemonkey does:

  var sandbox = new Components.utils.Sandbox(aContentWin);
  sandbox.__proto__ = aContentWin;

Where aContentWin is the XPCNativeWrapper (or whatever you guys call it these days) content window reference, yes.

We don't directly save that reference past eval time, but the script being eval'ed could leak it to the content scope, or into chrome via GM_registerMenuCommand, via the callback function reference.  Both should be cleaned up when/shortly after the content window is closed.  But I haven't had time to look at the registerMenuCommand bug (1578) yet.

Are they supposed to be explicitly cleaned up?  Or is standard garbage collection enough?
Comment 17 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-07-27 19:59:14 PDT
Well if something holds the sandbox alive that will hold the window alive (and it won't be freed by our new leak magic in Firefox 15).  What JetPack was doing was putting these sandboxes into an array, and then later removing them from the array when the page was destroyed.  Before removing the Sandbox from that array, it tried to touch the page (which is now gone) and got an exception, which stopped the code from ever removing the Sandbox from the array.

I'll dig in here on Monday.
Comment 18 Kris Maglione [:kmag] 2012-07-30 11:55:39 PDT
-> major because of Greasemonkey's popularity.

Worth noting that Scriptish does not seem to have this issue.
Comment 19 Kris Maglione [:kmag] 2012-07-31 13:13:03 PDT
Can you guys fix the exception that Kyle mentioned and let me know so I can review your update? I don't want to have to downgrade Greasemonkey... that would make a lot of people very unhappy.

Also, since I'm not seeing the error in the Error Console, it would probably be best to wrap the related event listener and observer handling code in try-catch blocks and Cu.reportError any exceptions you catch.
Comment 20 Anthony Lieuallen 2012-07-31 13:15:54 PDT
> Can you guys fix the exception that Kyle mentioned

Which exception are you referring to?  We don't use Jetpack and that's the closest reference I can find.
Comment 21 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-07-31 13:51:05 PDT
(In reply to Kris Maglione [:kmag] from comment #19)
> Can you guys fix the exception that Kyle mentioned and let me know so I can
> review your update? I don't want to have to downgrade Greasemonkey... that
> would make a lot of people very unhappy.
> 
> Also, since I'm not seeing the error in the Error Console, it would probably
> be best to wrap the related event listener and observer handling code in
> try-catch blocks and Cu.reportError any exceptions you catch.

That was just a theory, I haven't actually found anything wrong yet ...

I'm actively looking at this, but it's tough because afaict these scripts leak considerably before and after bug 695480.
Comment 22 Kris Maglione [:kmag] 2012-07-31 13:52:42 PDT
Ah, sorry, I misread comment 17.
Comment 23 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-07-31 14:00:04 PDT
So, it looks to me like in Firefox 14 YousableTubeFix leaks the last tab closed if and only if it had a youtube video somewhere on the tab.  In Firefox 15 all tabs with youtube videos are leaked indefinitely.
Comment 24 Kris Maglione [:kmag] 2012-07-31 14:10:24 PDT
Short circuiting the registerMenuCommand function fixes the problems on Firefox 14. That function stores the following in a global array:

  var command = {
      name: commandName,
      accessKey: accessKey,
      commandFunc: commandFunc,
      contentWindow: wrappedContentWin,
      contentWindowId: GM_util.windowId(wrappedContentWin),
      frozen: false};

I suspect the commandFunc object, which is a function from the sandbox scope and will hold alive a reference to the sandbox, is what's keeping the compartments alive. I don't know where the cleanup is failing.
Comment 25 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-07-31 14:15:25 PDT
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #23)
> So, it looks to me like in Firefox 14 YousableTubeFix leaks the last tab
> closed if and only if it had a youtube video somewhere on the tab.  In
> Firefox 15 all tabs with youtube videos are leaked indefinitely.

And that behavior changes from the 14 behavior to the 15 behavior in the first Nightly to include bug 659480.
Comment 26 Anthony Lieuallen 2012-07-31 14:23:13 PDT
Re comment 24:

The cleanup is:
https://github.com/greasemonkey/greasemonkey/blob/master/components/greasemonkey.js#L428

I'll try wrapping with try/catch and reportError.  And checking that it's called at the times we expect.

P.S. Why do so many errors (seemingly) from components/modules not get reported on their own?  I've guessed where the error is and try/catch/dump'ed more times than I'd like to count.
Comment 27 Josh Matthews [:jdm] 2012-07-31 14:26:29 PDT
(In reply to Anthony Lieuallen from comment #26)
> Re comment 24:
> 
> The cleanup is:
> https://github.com/greasemonkey/greasemonkey/blob/master/components/
> greasemonkey.js#L428
> 
> I'll try wrapping with try/catch and reportError.  And checking that it's
> called at the times we expect.
> 
> P.S. Why do so many errors (seemingly) from components/modules not get
> reported on their own?  I've guessed where the error is and
> try/catch/dump'ed more times than I'd like to count.

That's often for reasons like bug 503244, in my experience.
Comment 28 Kris Maglione [:kmag] 2012-07-31 14:27:24 PDT
It's deep XPCOM magic. I think there are open bugs to make those errors visible. I have helper code to wrap all of my event listeners in try-catch blocks.

I'm not sure that it's an error in this case. I tried wrapping the relevant listeners in try-catch blocks while I was poking around and didn't see anything, so it may be another issue entirely.
Comment 29 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-07-31 15:01:59 PDT
I think I've figured out the underlying issue here.  Now to just figure out how to fix it ...
Comment 30 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-07-31 18:28:41 PDT
The problem is the following code:

service.prototype.contentDestroyed = function(contentWindowId) {
  this.withAllMenuCommandsForWindowId(null, function(index, command) {
    var closed = false;
    try { closed = command.contentWindow.closed; } catch (e) { }

    if (closed ||
        (contentWindowId && (command.contentWindowId == contentWindowId))
    ) {
      gMenuCommands.splice(index, 1);
    }
  }, true);
};

If the wrapper to the contentWindow is already cut at this point, attempting to touch contentWindow.closed will throw.  The variable 'closed' will thus be false and the command will not be removed from the list if it does not specify a contentWindowId.  We can explicitly test for this situation, and if the contentWindow is already gone, default closed to true.  We'll need to backport Bug 773980 to beta to fix this everywhere.

I opened a pull request with this change: https://github.com/greasemonkey/greasemonkey/pull/1594

I confirmed that with this change trunk behaves like Firefox 14.
Comment 31 Kris Maglione [:kmag] 2012-07-31 18:32:50 PDT
So how to fix the lesser breakage in 14 is still an open question?
Comment 32 Kris Maglione [:kmag] 2012-07-31 18:33:42 PDT
Or, rather, do we know what's causing the breakage in 14?
Comment 33 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-07-31 18:39:48 PDT
I do not know what's causing the memory leak in 14, but it's minor compared to what happens in 15+.  IMO that's beyond the scope of this bug.  We can certainly look at it in another bug (triaged against the other MemShrink stuff of course).
Comment 34 Kris Maglione [:kmag] 2012-07-31 19:07:38 PDT
I don't think we know enough to say it's minor. I can file a separate bug if you think it's best, but for all we know, most Greasemonkey users could be leaking compartments for a sizable percentage of the pages they visit. I don't like to think what even sporadic leaks would do to a heavy Facebook or Gmail user.
Comment 35 Justin Lebar (not reading bugmail) 2012-07-31 19:11:03 PDT
> I can file a separate bug if you think it's best

I'd prefer a separate bug because it's likely a separate cause.
Comment 36 Kris Maglione [:kmag] 2012-07-31 19:12:50 PDT
Ok, when the fixed version is public I'll clone this bug.
Comment 37 Anthony Lieuallen 2012-07-31 19:55:02 PDT
Greasemonkey 0.9.21 pushed public, modulo AMO review delays.
Comment 38 Andrew McCreight [:mccr8] 2012-07-31 20:42:37 PDT
Kris, is there any way we can search for other AMO addons that might be doing the same thing?  Things calling .closed?  Or maybe that is too generic.
Comment 39 Andrew McCreight [:mccr8] 2012-07-31 20:43:41 PDT
I proposed semi-jokingly in IRC that we could create a "closed window proxy" that would be like a dead object proxy, except it wouldn't throw when you accessed its .closed property, it would just return "true".  Hopefully this isn't very common...
Comment 40 Anthony Lieuallen 2012-08-01 06:35:46 PDT
So, Kyle:

I merged your patch and (whoops) built a new version with it without testing -- it looked small and reasonable.  But (AFAICT) you can't assign to Components.utils so it failed.  I fixed, deleted that version from AMO and uploaded a new working XPI, but AMO is still hosting (a cached copy of?) the original broken XPI rather than the new fixed one.  Short term, if you'd like to test, I just uploaded a fixed copy to github:

https://github.com/downloads/arantius/greasemonkey/greasemonkey-0.9.21.xpi
Comment 41 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-08-01 06:37:18 PDT
(In reply to Anthony Lieuallen from comment #40)
> So, Kyle:
> 
> I merged your patch and (whoops) built a new version with it without testing
> -- it looked small and reasonable.  But (AFAICT) you can't assign to
> Components.utils so it failed.  I fixed, deleted that version from AMO and
> uploaded a new working XPI, but AMO is still hosting (a cached copy of?) the
> original broken XPI rather than the new fixed one.  Short term, if you'd
> like to test, I just uploaded a fixed copy to github:
> 
> https://github.com/downloads/arantius/greasemonkey/greasemonkey-0.9.21.xpi

Bah, I should have tested it on 14 :-(
Comment 42 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-08-01 06:43:11 PDT
Anyways, it should be simple enough to change it so that you only call isDeadWrapper if it exists, without setting a property on Components.utils.  Something like:

var closed = Component.utils.isDeadWrapper ? Component.utils.isDeadWrapper(command.contentWindow) : false;
Comment 43 Anthony Lieuallen 2012-08-01 07:54:45 PDT
So that I'm testing the same thing as everyone else and not misunderstanding something:

Could somebody on this bug please explain in simple terms exactly what you see (at about:memory) that convinces you that there is a memory leak?

Should I be looking for a line in about:memory that mentions a URL for a tab that I've closed?  Do I have to expand some ++ lines to see them?
Comment 44 Andrew McCreight [:mccr8] 2012-08-01 07:56:44 PDT
about:compartments is usually the easiest way to look for these kinds of problems.  In this particular case, you will see compartments for pages like YouTube that have been closed already.
Comment 45 Andrew McCreight [:mccr8] 2012-08-01 07:57:42 PDT
This page has more detailed information: https://developer.mozilla.org/en/Zombie_compartments
Comment 46 Kris Maglione [:kmag] 2012-08-01 08:23:50 PDT
(In reply to Andrew McCreight [:mccr8] from comment #38)
> Kris, is there any way we can search for other AMO addons that might be
> doing the same thing?  Things calling .closed?  Or maybe that is too generic.

I don't think so. There's an MXR repo for all add-ons on AMO, but searching for '.closed' comes up with a lot of irrelevant results, but a bunch for actual windows:

https://mxr.mozilla.org/addons/search?string=.closed&find=\.js$,\.jsm$&filter=^[^\0]*$

We could normally add a compatibility test to the validator, but we'd pretty much be stuck looking for any accesses to the property 'closed' for this, which would have far too many false positives.
Comment 47 Kris Maglione [:kmag] 2012-08-02 13:55:05 PDT
I think I need some clarification here. I went back and tried the previous public version of Greasemonkey, and with only YousableTubeFix installed, the behavior is pretty much the same. The latest release doesn't seem to change anything on Firefox 15 either, so I need some clarification on what leaks Kyle was experiencing on 15 and not 14.
Comment 48 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-08-02 14:14:01 PDT
I used the following steps:

- Load a Youtube URL (I used http://www.youtube.com/watch?v=tgnrUz8lAiQ&feature=g-u-u from reporter's about:memory)
- Wait a bit, then close the youtube tab.
- Open about:memory and Minimize memory.  The youtube compartment should still be hanging around in both 14 and 15.
- Open twitter.com
- Minimize memory again.  In 14, the youtube compartment will be gone.  In 15, it will still be there.
Comment 49 Kris Maglione [:kmag] 2012-08-03 08:44:56 PDT
So here's the breakdown. For Greasemonkey 0.9.20, with YousableTubeFix installed:

Firefox 14: Visiting a YouTube video leaks a compartment. Visiting another page seems to clear up that leak. We only ever get one YouTube video compartment leaked (though in Firefox 14, they all load into the same compartment anyway). Short circuiting the menu item insertion solves the problem.

Firefox 15: In Kyle's tests, we get a leaked compartment for every YouTube video page visited. Visiting another page does not clean them up. I can't reproduce this with the latest beta with either Greasemonkey 0.9.20 or the newer 0.9.22.
Comment 50 Nicholas Nethercote [:njn] 2012-08-08 16:30:45 PDT
khuey, jlebar:  confirmation of a fix in 0.9.22 would be great -- a commenter on my blog said he was still having problems with 0.9.22.
Comment 51 Zaid 2012-08-09 03:14:19 PDT
I am currently using the latest Firefox 15 beta, and GM 0.9.22, and the leaks are still happening.

I don't remember where I read an advice to comment out "GM_registerMenuCommand()" lines in the userscripts, but I tried that and the leaks are now gone. So, I think that any userscript that registers a menu is going to leak all the pages it runs on, because that command seems to be the cause of the problem.
Comment 52 Anthony Lieuallen 2012-08-09 05:45:10 PDT
> I am currently using the latest Firefox 15 beta, and GM 0.9.22, and the leaks are still happening.

Can you give me some idiot-simple steps to follow and signs to look for to confirm this?  There's been enough back and forth that I'd prefer verbosity to eliminate doubt.
Comment 53 Zaid 2012-08-09 07:26:44 PDT
(In reply to Anthony Lieuallen from comment #52)
> > I am currently using the latest Firefox 15 beta, and GM 0.9.22, and the leaks are still happening.
> 
> Can you give me some idiot-simple steps to follow and signs to look for to
> confirm this?  There's been enough back and forth that I'd prefer verbosity
> to eliminate doubt.

1- Install Firefox 15 beta
2- Install latest Greasemonkey (0.9.22) from the Addons website
3- Install "YousableTubeFix" from http://userscripts.org/scripts/show/13333
4- Install "Textarea backup with expiry" from http://userscripts.org/scripts/show/42879
5- Open about:memory in new tab
6- Open about:compartments in new tab
7- Open the following links in separate tabs:
http://www.youtube.com
http://www.youtube.com/watch?v=R4em3LKQCAQ
http://pastebin.mozilla.org/
8- Close all tabs opened in step 7
9- Goto about:memory and hit "Minimize memory usage" few times. Notice that the Ghost windows count will be a non-zero value (it should be zero!).
10- Goto about:compartments and refresh the page and notice the ghost windows that exist for all links opened in step 7.

Step 9 and 10 show the signs of a memory leak. In proper working, there should be no ghost windows that correspond to the closed tabs.

Hope this is idiot-proof and clear.
Comment 54 Anthony Lieuallen 2012-08-09 07:40:23 PDT
Yes, very, thank you.  I _thought_ I had done something exactly like that when developing the fix I claimed above.  https://github.com/greasemonkey/greasemonkey/issues/1600 created to track the second try at a fix.
Comment 55 Kris Maglione [:kmag] 2012-08-09 08:24:27 PDT
Zaid: See comment 49. There are two leaks, one that only happens on Firefox 15 and leaks like mad, and one that happens on other Firefox versions as well, and only leaks once. I'll open a new bug for the latter when this is confirmed fixed.
Comment 56 Zaid 2012-08-09 08:36:53 PDT
(In reply to Kris Maglione [:kmag] from comment #55)
> Zaid: See comment 49. There are two leaks, one that only happens on Firefox
> 15 and leaks like mad, and one that happens on other Firefox versions as
> well, and only leaks once. I'll open a new bug for the latter when this is
> confirmed fixed.

Yes, I have read all that. The small leak that is relevant to Firefox 14 does not affect the "Textarea backup with expiry" at all. That script didn't cause any leaks whatsoever in Firefox 14. So, maybe we should focus on the textarea backup script because the YousableTubeFix might be confusing as a test case.
Comment 57 Anthony Lieuallen 2012-08-09 10:01:16 PDT
I believe Greasemonkey 0.9.22 fixes this for Firefox 14, and 1.0beta7 currently in development has this fixed for 15.  See:
https://addons.mozilla.org/en-US/firefox/addon/greasemonkey/versions/
Comment 58 Zaid 2012-08-09 21:58:11 PDT
I can confirm that Greasemonkey 1.0beta7 does fix the serious leak in Firefox 15. "Textarea backup with expiry" is not leaking anymore, and YousableTubeFix leaks returned to the level of Firefox 14.

Is GM 0.9.xx branch going to get the fix soon?! Or maybe just let Firefox 15 automatically upgrade to the GM 1.0beta branch.
Comment 59 Anthony Lieuallen 2012-08-10 05:45:20 PDT
> Is GM 0.9.xx branch going to get the fix soon?! Or maybe just let Firefox 15 automatically upgrade to the GM 1.0beta branch.

Time will tell.  Beta7 is the first that I announced for public testing.  I think it's probably stable and ready.  If it is, I'll just release it.  If 15 lands before 1.0 is ready, then I'll probably backport that fix for a .23 release.
Comment 60 Nicholas Nethercote [:njn] 2012-08-13 22:22:14 PDT
1.0Beta7 still leaks badly for me on youtube.com with YousableTubeFix.  Here are the steps I used.

1- Run Firefox 17 trunk build with a new profile
2- Install latest Greasemonkey (0.9.22) from the Addons website
3- Install "YousableTubeFix" from http://userscripts.org/scripts/show/13333
4- Open about:compartments in new tab
5- Open the following link in a separate tab:
http://www.youtube.com/watch?v=R4em3LKQCAQ
6- Close the tab opened in step 5
7- Goto about:compartments and refresh the page and notice the zombie compartments and ghost windows.
8- Repeat steps 5, 6, 7 for multiple other YouTube videos.

I get persistent zombie compartments and ghost windows for every video I open.
Comment 61 Zaid 2012-08-14 01:04:22 PDT
(In reply to Nicholas Nethercote [:njn] from comment #60)
> 1.0Beta7 still leaks badly for me on youtube.com with YousableTubeFix.  Here
> are the steps I used.
> .....
> 2- Install latest Greasemonkey (0.9.22) from the Addons website
> .....
> I get persistent zombie compartments and ghost windows for every video I
> open.

Are you talking about 1.0beta7 or 0.9.22?! It seems you got the two confused in you comment above.
Comment 62 Nicholas Nethercote [:njn] 2012-08-14 02:07:44 PDT
It's a mistake in step 2 -- I tested with 1.0beta7.  Thanks for catching that.
Comment 63 Kris Maglione [:kmag] 2012-08-14 06:06:10 PDT
Agreed. See bug 781482 comment 2.
Comment 64 Zaid 2012-08-20 13:34:28 PDT
Comment on attachment 646739 [details]
Data from about:memory page

Explicit Allocations
1,271.16 MB (100.0%) — explicit
├────732.51 MB (57.63%) — js
│ ├──621.62 MB (48.90%) ++ (669 tiny)
│ ├───24.73 MB (01.95%) — compartment([System Principal], jar:file:///C:/Users/zzz/AppData/Roaming/Mozilla/Firefox/Profiles/gvrqe12c.default/extensions/%7Bd10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d%7D.xpi!/bootstrap.js)
│ │ ├──18.39 MB (01.45%) ++ gc-heap
│ │ └───6.33 MB (00.50%) ++ (7 tiny)
│ ├───20.73 MB (01.63%) ── gc-heap-decommitted
│ ├───20.03 MB (01.58%) ++ compartment([System Principal], about:blank)
│ ├───18.09 MB (01.42%) — compartment([System Principal], chrome://browser/content/browser.xul)
│ │ ├──13.89 MB (01.09%) ++ gc-heap
│ │ └───4.21 MB (00.33%) ++ (5 tiny)
│ ├───13.77 MB (01.08%) ++ compartment(http://www.youtube.com/watch?v=tgnrUz8lAiQ&feature=g-u-u)
│ └───13.54 MB (01.06%) ++ compartment(atoms)
├────463.39 MB (36.45%) ── heap-unclassified
├─────35.97 MB (02.83%) — images
│ ├──35.66 MB (02.81%) — content
│ │ ├──35.66 MB (02.81%) — used
│ │ │ ├──23.48 MB (01.85%) ── uncompressed-heap
│ │ │ └──12.18 MB (00.96%) ++ (2 tiny)
│ │ └───0.00 MB (00.00%) ++ unused
│ └───0.31 MB (00.02%) ++ chrome
├─────25.21 MB (01.98%) — storage
│ ├──23.74 MB (01.87%) ++ sqlite
│ └───1.46 MB (00.12%) ── prefixset/all
└─────14.08 MB (01.11%) ++ (10 tiny)

Other Measurements
0.21 MB ── canvas-2d-pixel-bytes
1,271.10 MB ── explicit
0.49 MB ── gfx-d2d-surfacecache
4.21 MB ── gfx-d2d-surfacevram
25.10 MB ── gfx-surface-image
504 ── ghost-windows
854.64 MB ── heap-allocated
902.11 MB ── heap-committed
47.45 MB ── heap-committed-unused
5.55% ── heap-committed-unused-ratio
3.20 MB ── heap-dirty
126.35 MB ── heap-unused
23.48 MB ── images-content-used-uncompressed
389 ── js-compartments-system
568 ── js-compartments-user
414.00 MB ── js-gc-heap
25.12 MB ── js-main-runtime-analysis-temporary
260.28 MB ── js-main-runtime-gc-heap-allocated
132.99 MB ── js-main-runtime-gc-heap-arena-unused
0.00 MB ── js-main-runtime-gc-heap-chunk-clean-unused
0.00 MB ── js-main-runtime-gc-heap-chunk-dirty-unused
393.27 MB ── js-main-runtime-gc-heap-committed
132.99 MB ── js-main-runtime-gc-heap-committed-unused
51.09% ── js-main-runtime-gc-heap-committed-unused-ratio
20.73 MB ── js-main-runtime-gc-heap-decommitted
0.59 MB ── js-main-runtime-mjit
126.54 MB ── js-main-runtime-objects
214.30 MB ── js-main-runtime-scripts
121.80 MB ── js-main-runtime-shapes
31.18 MB ── js-main-runtime-strings
30.07 MB ── js-main-runtime-type-inference
0 ── low-commit-space-events
0 ── low-memory-events-physical
0 ── low-memory-events-virtual
1,365.43 MB ── private
1,275.38 MB ── resident
0.00 MB ── shmem-allocated
0.00 MB ── shmem-mapped
23.74 MB ── storage-sqlite
1,728.31 MB ── vsize
0.55 MB ── window-objects-dom
0.73 MB ── window-objects-layout-arenas
0.04 MB ── window-objects-layout-pres-contexts
0.46 MB ── window-objects-layout-style-sets
0.00 MB ── window-objects-layout-text-runs
0.60 MB ── window-objects-style-sheets
Comment 65 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-08-20 13:35:42 PDT
What version of Greasemonkey are you using?
Comment 66 Zaid 2012-08-20 15:10:46 PDT
Ohh sorry, false alarm, the last comment was a mistake. I tried to edit the about:memory attachment, because I wanted to remove the personal details in the attachment, but instead it posted the change as a comment (and didn't change the attachment). I couldn't find a way to remove the comment!
Comment 67 Justin Lebar (not reading bugmail) 2012-08-20 15:14:51 PDT
> I couldn't find a way to remove the comment!

You can't.  But there doesn't appear to be any personal data in this particular about:memory dump.

In the future, the output from about:memory?verbose is often more useful than the output from vanilla about:memory.  In particular, I'd want to see the explicit/js/tiny node expanded.
Comment 68 Nicholas Nethercote [:njn] 2012-08-20 16:45:26 PDT
This bug has stalled.  My testing indicates that 1.0beta7 doesn't fix the leak with YousableTubeFix.  Does anyone have evidence to the contrary?  Anthony, do you have anything to add?

I really want this to be fixed by the time FF15 is released on August 28.
Comment 69 Anthony Lieuallen 2012-08-21 10:33:08 PDT
Created attachment 653833 [details]
output of about:memory with verbose on, at point mentioned in the comment

Ok, time to be even more careful and explicit:

1) "cd" to my test profile directory; rm -rf *
2) ~/opt/firefox-dev/firefox -no-remote -P test
   (I keep the beta channel firefox around for development, this is FF 15.)
3) about:support says
   "Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20100101 Firefox/15.0"
   (Is there a better thing to say for this?  I keep getting updates on the
   beta channel but it's always "15.0" -- which version of 15.0 is it?"
4) Navigate to https://addons.mozilla.org/en-US/firefox/addon/greasemonkey/
5) Install 1.0beta7 from the development channel at the bottom.
6) Restart.
7) Navigate to http://userscripts.org/scripts/show/13333 .
8) Install said user script.
9) Restart.
10) Navigate the only open (about:home) tab to about:compartments
11) There's tons of system compartments; user compartments says only about:home
12) More verbose shows lots more data in system; empties user.
13) New (second) tab, navigate to http://www.youtube.com/watch?v=ik2CZqsAw28
14) Open the Greasemonkey menu to see it has the
    "YousableTubeFix Configuration" item.
15) New (third) tab, navigate to http://www.youtube.com/watch?v=whYqhpc6S6g
16) New (fourth) tab, navigate to http://www.youtube.com/watch?v=p2BxAu6WZI8
17) Switch to the first (about:compartments) tab, press F5
18) User compartments now says:

http://p2-cz5bjbpvdvflk-6mxxmqtgzklugihz-if-v6exp3-v4.metric.gstatic.com/v6exp3/iframe.html
http://www.youtube.com/watch?v=ik2CZqsAw28
http://www.youtube.com/watch?v=ik2CZqsAw28, about:blank [2]
http://www.youtube.com/watch?v=ik2CZqsAw28, http://userscripts.org/scripts/show/13333/YousableTubeFix
http://www.youtube.com/watch?v=p2BxAu6WZI8
http://www.youtube.com/watch?v=p2BxAu6WZI8, about:blank [2]
http://www.youtube.com/watch?v=p2BxAu6WZI8, http://userscripts.org/scripts/show/13333/YousableTubeFix
http://www.youtube.com/watch?v=whYqhpc6S6g
http://www.youtube.com/watch?v=whYqhpc6S6g, about:blank [2]
http://www.youtube.com/watch?v=whYqhpc6S6g, http://userscripts.org/scripts/show/13333/YousableTubeFix

19) Close the right-most tab, press F5, user compartments now says:

http://p2-cz5bjbpvdvflk-6mxxmqtgzklugihz-if-v6exp3-v4.metric.gstatic.com/v6exp3/iframe.html
http://www.youtube.com/watch?v=ik2CZqsAw28
http://www.youtube.com/watch?v=ik2CZqsAw28, about:blank [2]
http://www.youtube.com/watch?v=ik2CZqsAw28, http://userscripts.org/scripts/show/13333/YousableTubeFix
http://www.youtube.com/watch?v=whYqhpc6S6g
http://www.youtube.com/watch?v=whYqhpc6S6g, about:blank [2]
http://www.youtube.com/watch?v=whYqhpc6S6g, http://userscripts.org/scripts/show/13333/YousableTubeFix

20) Close the right-most tab, press F5, user compartments now says:

http://p2-cz5bjbpvdvflk-6mxxmqtgzklugihz-if-v6exp3-v4.metric.gstatic.com/v6exp3/iframe.html
http://www.youtube.com/watch?v=ik2CZqsAw28
http://www.youtube.com/watch?v=ik2CZqsAw28, about:blank [2]
http://www.youtube.com/watch?v=ik2CZqsAw28, http://userscripts.org/scripts/show/13333/YousableTubeFix

21) Close the right-most tab (only one left, about:compartments, now),
    press F5, user compartments now says:

http://www.youtube.com/watch?v=ik2CZqsAw28
http://www.youtube.com/watch?v=ik2CZqsAw28, http://userscripts.org/scripts/show/13333/YousableTubeFix


Whoops.  Ok, one compartment (two?) leaked.  If I start step 9 (restart) through
18, but skip step 14 (open the menu), all the compartments go away.  If I do
step 9 through 18, but repeat 14 on every tab, I still get the same one(/two)
compartment left over, not one(/two) per tab. **

This needs to be fixed, but it doesn't seem like a release blocker to me.


** about:memory verbose from this point attached
Comment 70 Anthony Lieuallen 2012-08-21 11:35:04 PDT
I thought I realized that this one last "leak" was just a slightly-extra-long-held compartment:  If you open the menu (the condition to cause the leak listed above) then a reference into the window via the callback function gets saved in the chrome DOM.  This (<menuitem> node) is removed only the next time that menu gets rebuilt at onPopupShowing time.  But I couldn't get this to ever remove that one reference, so I might be a little off, or we might be doing event handler/dom cleanup wrong.  (In: https://github.com/greasemonkey/greasemonkey/blob/master/content/menucommander.js )
Comment 71 Nicholas Nethercote [:njn] 2012-08-22 22:59:34 PDT
I just tried the steps in comment 60 (again) and comment 69 with a trunk build of FF17 and didn't get any zombie compartments.  So it looks like it is fixed in 1.0beta7.  Sorry for any confusion;  I'm not sure what I did wrong last time.
Comment 72 Nicholas Nethercote [:njn] 2012-08-23 19:54:23 PDT
What's the release schedule for GM 1.0?  It'd be great if it could be released before Firefox 15, which is due on August 28...
Comment 73 Kris Maglione [:kmag] 2012-08-23 19:58:15 PDT
(In reply to Nicholas Nethercote [:njn] from comment #72)
> What's the release schedule for GM 1.0?  It'd be great if it could be
> released before Firefox 15, which is due on August 28...

Agreed. This needs to be fixed before Firefox 15. If 1.0 won't be out before then, please push out a minor update that fixes this and let me know when you have an update ready either way and I'll review it as soon as I can.

Thanks.
Comment 74 Anthony Lieuallen 2012-08-24 07:34:32 PDT
Just sent Greasemonkey 1.0 to AMO.
Comment 75 Kris Maglione [:kmag] 2012-08-24 09:13:30 PDT
I'm still seeing short term zombie compartments in the latest 15 beta. I installed YousableTubeFix, opened 4 or so YouTube videos in tabs, closed them all, minimized memory, and compartments for all of them remained until I opened a new tab with google.com.
Comment 76 Anthony Lieuallen 2012-08-24 09:29:11 PDT
This sounds like quibbling over semantics.  Zombie means undead.  These compartments are not zombies, they're just held onto a little longer than you intuitively think they should be.  They do get removed.

In this specific case, we're doing gBrowser.addEventListener("pagehide", ...), which gets called when the next tab is opened.  And that calls the clean-up routine.

We're also observing inner-window-destroyedbut it is never called in these reproduction steps.  That might be a bug?  Maybe we should be observing dom- and outer- as well?  A quick patch in doesn't seem to fire those on tab-close either.  What should we be listening/observing on to know that a window is gone?
Comment 77 Kris Maglione [:kmag] 2012-08-24 09:32:40 PDT
I'm not sure what part of the semantics trouble you, but zombie compartments are zombie compartments no matter how long they live. If your cleanup listeners aren't having the desired effect, it would probably be best to just use weak references for anything that that might wedge open a page compartment.
Comment 78 Justin Lebar (not reading bugmail) 2012-08-24 09:53:57 PDT
> This sounds like quibbling over semantics.  Zombie means undead.  These compartments are 
> not zombies, they're just held onto a little longer than you intuitively think they 
> should be.  They do get removed.

I don't claim that our semantics are obvious, but we distinguish between "compartments"; "zombie compartments", which live longer than they should; and "/immortal/ zombie compartments".
Comment 79 Nicholas Nethercote [:njn] 2012-08-24 14:23:20 PDT
Can we approve this anyway?  A few compartments that live longer than expected is still much better than what we had before.  The remaining problems can be fixed post-1.0.
Comment 80 Kris Maglione [:kmag] 2012-08-27 11:24:24 PDT
1.0 has already been approved.
Comment 81 Nicholas Nethercote [:njn] 2012-08-29 20:11:53 PDT
1.0 is shipping.
Comment 82 Anthony Lieuallen 2012-08-31 05:58:38 PDT
FYI: This Greasemonkey leak should be 100% patched including the single compartment mentioned in comment 69, test nightlies (or build yourself from head) if you'd like to confirm.  https://arantius.com/misc/gm-nightly/

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