New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GM_registerMenuCommand memory leak #2067
Comments
GM+alertbox+个别脚本,导致firefox内存暴涨 |
I'm not denying anything, but the above steps are quite involved. Can you produce a reduced test case to help support your position? Also, is this a "leak" i.e. the memory is permanently allocated even when no longer used? Or does it go away at an appropriate time (i.e. closing the tab)? Also: |
The described testcase seems to be as minimal and simple as possible while maintaining the connection to reality... |
Step 4 is optional and you can replace BuzzFeed with any other site. The test case boils down to start watching a page where a userscript calls GM_registerMenuCommand and wait. You just have to wait a little longer for the increase to be noticeable with default settings. I didn't try I'm currently working around this by delaying GM_registerMenuCommand until the first document.onmouseover. |
(moved from other issue per @arantius's comment) My own user script leaks memory significantly with To reproduce:
If you comment out the single call to |
I believe I am experiencing the same issue. Since upgrading Greasemonkey past v2.3 after an hour+ of browsing Firefox becomes much slower and less responsive. With higher memory usage and randomly noticeable delay in scrolling, opening links in new tabs, closing tabs etc. I narrowed it down to being caused by the following userscript http://userscripts-mirror.org/scripts/show/174606 (you will have to modify it to add grants for GM_getValue, GM_registerMenuCommand, GM_setValue) Removing the GM_registerMenuCommand grant and commenting out the line calling the GM_registerMenuCommand function and so far Firefox seems to be running without decreased responsiveness. Setup is Greasemonkey 3.1, Firefox 38 (beta) 64-bit. |
shrug I did @simonzack 's steps. I visit about:memory, click measure, and it says " 0 ── ghost-windows". How many is "some" searches? |
@arantius I remember I did less than 5, let me check it again. |
I cant speak for @simonzack or his script but running the script I linked above (with the added grants) and I get a lot of ghost windows created. I cant figure out how to attach a file to the issue tracker so I uploaded a about:memory memory report to dropbox https://www.dropbox.com/s/apxw6sspgp0e28h/memory-report.json.gz?dl=0 |
Just use GitHub Gists... |
Firefox:
// ==UserScript==
// @name GM_registerMenuCommand Tester
// @namespace https://github.com/arantius
// @include *
// @version 1
// @grant GM_registerMenuCommand
// ==/UserScript==
for (var i = 0; i < 99; i++) {
GM_registerMenuCommand(
'Test command #' + i,
function() { alert('Clicked menu command!') });
} If I disable the script, the numbers are similar. If I repeat the same steps several times, the specific numbers vary, but all stay in the same window regardless of whether the script is running or not. Ok, how about this script: // ==UserScript==
// @name GM_registerMenuCommand Tester
// @namespace https://github.com/arantius
// @include *
// @version 1
// @grant GM_registerMenuCommand
// ==/UserScript==
var memHog = [];
for (var i = 0; i < 9999; i++) {
// Just over 1k bytes, 10k times = ~10MB
var val = ''
+ 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
+ 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
+ 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
+ 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
+ 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
+ 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
+ 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
+ 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
+ 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
+ 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
+ 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
+ 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
+ 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
+ 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
+ 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
+ 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
+ Math.random();
memHog.push(val);
// Plus who knows how much for the DOM?
var p = document.createElement('p');
p.textContent = val;
document.body.appendChild(p);
}
GM_registerMenuCommand(
'Test command #' + i,
function() { alert('Clicked menu command!') }); This should consume tons of memory! If I comment out the Ok, restore the menu command in the script, fresh re-launch Firefox. Fresh, then after each load/close memory: 160/60, 260/220, 280/120, 300/120, 340/120, 400/140, 320/120, 340/140, 300/120. More jitter, but around the same point. I don't see any memory leak. If you want to convince me, please provide reams of data and explanations a five year old could understand. |
…nd_memoryLeak_maybe' Refs greasemonkey#2067
The situation has gotten significantly worse under Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:40.0) Gecko/20100101 Firefox/40.0. Distill Web Monitor is not involved this time. After just 5 minutes of normal browsing with multiple tabs, Firefox starts to lock up every 10 seconds for one second. Typing, scrolling, tab animations - everything stops. Currently with only about:memory open, I'm seeing 203 ghost windows and an explicit memory allocation of 734.96 MB. Pressing "Minimize memory usage" does virtually nothing except freeze Firefox for 10 seconds. GM 3.2 is the only extension installed. |
Can you repeat a test like mine above ? And if so what sort of results do you see? Saying just ".. minutes of normal web browsing" is hard to reproduce. I've tried and failed to reproduce this. Or can you say specifically which page(s) trigger this? |
I opened around 15 tabs with popular sites like reddit, amazon, imdb and wikipedia, clicked a link in each tab, scrolled around for 5 seconds and closed all tabs. I'm gonna try your tester script. |
Here are my steps with which I'm consistently able to reproduce a ghost window in a fresh profile with GM HEAD (6f5ba1c) installed in FF 39:
If step 4 is left out, no ghost window appears. I think the issue might be related to |
Removed my minimalist test script, installed arantius' memHog test script and restarted Firefox. Memory consumption (private working set in Windows task manager) directly after start with empty page: 244 MB Opened http://arstechnica.com. Memory after loading finished: 1355 MB Opened new empty tab. Closed first tab. Memory: 754 MB. Opened about:memory and clicked "Minimize memory usage" Memory 635 MB. Clicked a second time. 637 MB. Clicked "Measure". 575.79 MB explicit allocations |
Choosing arstechnica was a bad idea. Takes forever. |
I believe you, but we're seeing very different results. We need to know why. There's something you're doing differently. Perhaps something in your Firefox profile (can you try with a new blank profile)? Other actions you're taking? Clicking menus? Extensions installed? |
I followed your steps exactly and got 376.34 MB and 0 ghost windows. However, ghost windows do appear out of nowhere when I press "Minimize memory usage" and measure again. They do not appear when the GM_registerMenuCommand call is commented out. |
Can confirm one ghost per open/navigate/close tab after clicking minimize. |
Also, with the smaller (99 loop GM_rmc) script:
That's pretty definitive leaking. The menu commands keep a reference to the window for too long. |
Side note: I never press the minimize button while I'm in my everyday profile so I guess "heap-minimize notifications" get sent automatically from time to time. |
Definitely, that button is just a nice way to deterministically reproduce when they happen. |
In an attempt to not leak them? Refs greasemonkey#2067
If:
At let me know if things get better, worse, or no change. Somehow my steps to reproduce ghost windows from this morning don't work anymore, but I've done two things that may help the situation. Confirmation either way would be real nice. |
Greasemonkey 3.3beta1 is a huge improvement but I still see a small number of ghost windows popping up after pressing "Minimize memory usage" - currently 18 after 3 hours in my main profile (13 addons, 41 userscripts of which 4 add menu commands, process size 496 MB after closing all tabs). Unfortunately I haven't found a way to replicate this reliably with a clean profile. At first I thought it had to do with restored tabs from a previous session. Worked only twice though... |
I have noticed bursts of high CPU usage every 30 to 60 seconds with adsbypasser, during which Firefox's UI freezes. I concluded that the problem lies in GM_registerMenuCommand, since commenting the line that calls that function solves the problem. I think it might be related to this issue, since on every burst, Firefox's RAM usage seems to drop a few MB. Perhaps to some periodic garbage collecting? I seem to recall that the issue started occuring at the end of may, which coincides with the release of version 3.2, but it might be a coincidence. I'm going to try with 3.1 I have tried installing Greasemonkey 3.3beta1, but the issue persists. |
1) Store menu commmands' data in a private closure, *in the sandbox*. 2) To list registered commands: a) Parent/chrome passes a message to child/frame. b) Frame passes an event (visible to content) into the sandbox. c) Sandbox passes private-closure-scoped commands' data to a frame-scoped callback. d) Frame passes data up to parent as a message. e) Chrome uses this data to populate the menu, at popupshowing time. 3) To run a command: a) User clicks on the menu item. b) Chrome sends a message to the frame. c) Frame sends an event to the sandbox. d) Sandbox finds the related registered command, calls its callback. Phew! But no references to documents/windows/browsers are ever stored anywhere, so they can't possibly leak anymore. Along the way, simplify frame script by moving object methods to standalone functions; less state, less binding to fix "this" references. The ContentObserver object is now really just there for `.observe()`. TODO: Restore "delayed execution" feature, the only other usage of the (removed) ScriptRunner structure. Refs: greasemonkey#2200 Refs: greasemonkey#2067
I've prepared a much bigger, hopefully much better, change to target this issue. Please review (@Ventero perhaps?): |
Will do, probably won't be able to finish it until tomorrow though. |
I'm not really sure I'm 100% comfortable with dispatching the I can't think of an easy way to avoid this though. Sure, only running the menu command that was actually clicked is easy enough, but I can't think of a way to dispatch this event privately somehow which does not involve storing a sandbox reference somewhere (at least implicitly in some kind of closure). I'll follow this up with some additional review comments tomorrow. |
Thanks for the early comments. This was the best idea I had to avoid storing references that might leak. But your comments have prompted me to think back. What I've done above is to implicitly rely on the implicit reference that the sandbox has to the content window, and traverse that implicit link by dispatching an event. Trusting that the sandbox will be GCed when the window object is. Can we perhaps add an expando to the content window/browser to explicitly make this link, but outside of the content scope? If so does it retain the property of being GCed at the right time by default? |
arantius/greasemonkey@async-menu-command...ventero:async-menu-command is a small experiment with using a function reference chain to go from frame script to sandbox and back. It doesn't work very well though, as it seems that the message listener keeping a strong reference to the menu commander function returned from the sandbox means the sandbox is kept alive indefinitely. Maybe your idea will work, since by storing the function returned by Maybe we could reach out to some Mozilla folks - after all, if anyone knows how to cross from chrome to a content sandbox through a frame script without leaking objects everywhere, it should be them ... |
1) Store menu commmands' data in a private closure, *in the sandbox*. 2) To list registered commands: a) Parent/chrome passes a message to child/frame. b) Frame passes an event (visible to content) into the sandbox. c) Sandbox passes private-closure-scoped commands' data to a frame-scoped callback. d) Frame passes data up to parent as a message. e) Chrome uses this data to populate the menu, at popupshowing time. 3) To run a command: a) User clicks on the menu item. b) Chrome sends a message to the frame. c) Frame sends an event to the sandbox. d) Sandbox finds the related registered command, calls its callback. Phew! But no references to documents/windows/browsers are ever stored anywhere, so they can't possibly leak anymore. Along the way, simplify frame script by moving object methods to standalone functions; less state, less binding to fix "this" references. The ContentObserver object is now really just there for `.observe()`. TODO: Restore "delayed execution" feature, the only other usage of the (removed) ScriptRunner structure. Refs: greasemonkey#2200 Refs: greasemonkey#2067
Especially now that the Now, on content detecting/intercepting menu commands. I put: addEventListener('greasemonkey-menu-command-list', function(e) {
document.body.innerHTML +=
'Content scope detected greasemonkey-menu-command-list event! Detail: '
+JSON.stringify(e.detail)+'<br>';
}, true);
addEventListener('greasemonkey-menu-command-run', function(e) {
document.body.innerHTML +=
'Content scope detected greasemonkey-menu-command-run event! Detail: '
+JSON.stringify(e.detail)+'<br>';
}, true); Into a content page, and it logged:
So, yeah. As-is, content can see A) when you open the monkey menu, and B) when you select a command. But it doesn't know which script (the UUIDs are random per user) or anything about which command besides its index in the list. And it's pretty trivial for content to register the listener before the sandbox runs (especially for Now I'm thinking of inserting an installation-random value into the event name. At that point it's infeasible for content to attach the right event listener, how do you feel about that? |
1) Store menu commmands' data in a private closure, *in the sandbox*. 2) To list registered commands: a) Parent/chrome passes a message to child/frame. b) Frame passes an event (visible to content) into the sandbox. c) Sandbox passes private-closure-scoped commands' data to a frame-scoped callback. d) Frame passes data up to parent as a message. e) Chrome uses this data to populate the menu, at popupshowing time. 3) To run a command: a) User clicks on the menu item. b) Chrome sends a message to the frame. c) Frame sends an event to the sandbox. d) Sandbox finds the related registered command, calls its callback. Phew! But no references to documents/windows/browsers are ever stored anywhere, so they can't possibly leak anymore. Along the way, simplify frame script by moving object methods to standalone functions; less state, less binding to fix "this" references. The ContentObserver object is now really just there for `.observe()`. TODO: Restore "delayed execution" feature, the only other usage of the (removed) ScriptRunner structure. Refs: greasemonkey#2200 Refs: greasemonkey#2067
Version 3.3beta2 ( https://addons.mozilla.org/firefox/downloads/file/329720/greasemonkey-3.3beta2-fx.xpi?src=devhub ) contains the above-discussed enhancements. All testing is highly appreciated! |
On Firefox 40 (64 bit) beta under Windows 7, trying GM 3.3beta2. Testing my usual scripts that caused ghost windows and lag (in minutes with prior GM versions) and after ~ 4 hours I have 0 ghost windows. Will comment again if that changes. |
try on firefox 39. ff40 had some changes that apparently fixed or reduced the problem, that ff39 lacked, according to #2200 (comment) |
No ghost windows whatsoever after using current git HEAD in FF39 over the weekend, both with general usage and also when following the steps with which I could 100% reproduce a ghost window before. (Ab)Using the content window to communicate with the sandbox still feels a bit iffy to me, but I can't argue with the results. With random event names, I also don't see a way how untrusted content could interfere with the events anymore, so +1 from me. |
Positive reports here
And
|
i'll join in on the positive reports as well. the beta seems to work much better than 3.2, at similar level as 2.3.1 did. i no longer have extreme browser lag and/or 3+ gb ram usage with it. i've been running firefox nonstop for about a day, ram is still hovering around 700 mb-1 gb, cpu use is at less than 5%, and that's with nearly 50 tabs open, 2 of them being youtube with yt center. |
Awesome, thanks for the data! I'll get right on a 3.3 release. |
A user of one of my scripts discovered a memory leak which occurs when Distill Web Monitor is running and I can confirm this with Fx 35 + GM 2.3 + DWM 1.1.2.
Distill Web Monitor is an extension to periodically check web pages for changes. It seems to be doing this in a way that trips up Greasemonkey. On each check, Greasemonkey injects its userscripts into an invisible window and if GM_registerMenuCommand gets called, memory consumption will increase over time.
Steps to reproduce (clean profile):
@include *, @grant GM_registerMenuCommand
, code:GM_registerMenuCommand('Test', function() { alert("Test!"); });
If you uncomment GM_registerMenuCommand, memory will stay roughly the same (140 to 180 MB on my system).
Is it possible to make Greasemonkey only run in "real" windows?
The text was updated successfully, but these errors were encountered: