Closed Bug 778318 Opened 12 years ago Closed 12 years ago

Greasemonkey user scripts "YousableTubeFix" and "Textarea backup with expiry" cause zombie compartments

Categories

(WebExtensions :: General, defect)

x86
Windows 7
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: zeidspex, Assigned: khuey)

References

Details

(Keywords: regression, Whiteboard: [MemShrink:P1])

Attachments

(3 files)

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.
Attachment #646739 - Attachment description: leak.txt → Data from about:memory page
Severity: normal → major
Whiteboard: [Memshrink]
Try using safe-mode and see if you can replicate.
Severity: major → normal
No, it works fine under safe-mode. It is an addons issue I believe. Most likely related to the Huey Fix.
I believe I have Identified the offending addon. It is Greasemonkey.
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. ;)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Youtube yes, OkCupid no. I have "Super Linkifier" and "Textarea backup with expiry".
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.
(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!
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.
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.
cc greasemonkey devs.
Summary: Firefox 15 Beta Leaks All Compartments → Greasemonkey user scripts "YousableTubeFix" and "Textarea backup with expiry" cause zombie compartments
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.
Keywords: regression
Does Greasemonkey use sandboxes?  This sounds a lot like the jetpack issues we ran into.
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.
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.
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?
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.
-> major because of Greasemonkey's popularity.

Worth noting that Scriptish does not seem to have this issue.
Severity: normal → major
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.
> 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.
(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.
Ah, sorry, I misread comment 17.
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.
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.
(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.
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.
(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.
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.
I think I've figured out the underlying issue here.  Now to just figure out how to fix it ...
Assignee: nobody → khuey
Component: Untriaged → Add-ons
Product: Firefox → Tech Evangelism
Version: 15 Branch → unspecified
No longer blocks: 773980
Depends on: 773980
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.
So how to fix the lesser breakage in 14 is still an open question?
Or, rather, do we know what's causing the breakage in 14?
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).
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.
> 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.
Ok, when the fixed version is public I'll clone this bug.
Greasemonkey 0.9.21 pushed public, modulo AMO review delays.
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 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...
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
(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 :-(
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;
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?
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.
This page has more detailed information: https://developer.mozilla.org/en/Zombie_compartments
(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.
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.
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.
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.
Whiteboard: [Memshrink] → [MemShrink:P1]
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.
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.
> 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.
(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.
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.
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.
(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.
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/
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.
> 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.
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.
(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.
It's a mistake in step 2 -- I tested with 1.0beta7.  Thanks for catching that.
Agreed. See bug 781482 comment 2.
Blocks: 781482
No longer blocks: 781482
Depends on: 781482
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
What version of Greasemonkey are you using?
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!
> 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.
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.
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
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 )
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.
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...
(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.
Just sent Greasemonkey 1.0 to AMO.
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.
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?
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.
> 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".
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.
1.0 has already been approved.
1.0 is shipping.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
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/
Component: Add-ons → General
Product: Tech Evangelism → WebExtensions
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: