Closed Bug 727413 Opened 13 years ago Closed 13 years ago

Geenstijl extension 2.11.0.1 causes zombie compartments

Categories

(Core :: General, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: eviljeff, Unassigned)

References

()

Details

(Whiteboard: [MemShrink:P3])

The above addon leaks a reference to the geenstijl website after the page is closed. Just viewing a blog page such as http://www.geenstijl.nl/mt/archieven/2012/02/jonge_verzorgde_vrouw_beukt_91.html#comments is sufficient. After the page is closed about:memory?verbose still shows a reference to the webpage. Developer will be informed and invited to join bug.
Looked at geenstijlplugin.js, the problem code must be in here. When I started on this plug-in I used a greecemonkey template and modified it slightly. This greecemonkey template has a function called Hitch: // This method comes from a greecemonkey script and is used to add a method to the DOM window atm0sNamespace("atm0s.geenstijl.mozilla.extension").hitch = function(obj, meth) { // Check if the given method name exists on the given object if (!obj[meth]) { throw "method '" + meth + "' does not exist on object '" + obj + "'"; } // Reference caller var hitchCaller = Components.stack.caller.filename; // Get arguments var staticArgs = Array.prototype.splice.call(arguments, 2, arguments.length); // Return the method return function() { // make a copy of staticArgs (don't modify it because it gets reused for // every invocation). var args = staticArgs.concat(); // add all the new arguments for (var i = 0; i < arguments.length; i++) { args.push(arguments[i]); } // invoke the original function with the correct this obj and the combined // list of static and dynamic arguments. return obj[meth].apply(obj, args); }; } I use this method to expose a few functions to the page that act as an interface to the plug-in code, basicly so I can store and read preferences, open new tabs without the pop-up blocker interfring and also to launch webworkers. But, these function (assigned to DOMWindow) are never removed, I think because the original greecemonkey script I followed, didn't either. You would think that a garbagecollector should understand.. right? If I am right, it would mean that there would be any leaks, if you first navigate to e.g. www.google.com before closing the tab, is this assumption correct?
Correction: If I am right, it would mean that there would _NOT_ be any leaks, if you first navigate to e.g. www.google.com before closing the tab, is this assumption correct?
Hi, from further testing, navigating to another site after a page geenstijl, before closing the tab, still leads to a leak to the blog page on geenstijl. The subsequent webpage is leaked as well(!) I did the following: 1) navigated to http://www.geenstijl.nl/mt/archieven/2012/02/jonge_verzorgde_vrouw_beukt_91.html#comments 2) loaded google.co.uk 3) closed tab 4) checked about:memory?verbose and saw a reference to geenstijl AND google.co.uk It doesn't seem to happen if geenstijl isn't loaded in the tab first. i.e. Just navigating to google.co.uk, then bing.com, then closing the tab, doesn't leak.
Ok thanks Andrew, Learned myself to use the build-in memory tool, and checked how to test for ghost compartments. I noticed that the plug-in indeed caused the problem. I uploaded a new version 2.11.0.2 This is what I changed in geenstijlplugin.js: 1) Added an unload handler that removed the referenced methods attached to the DOM window (this did not help) 2) Renamed the namespace in geenstijlplugin.js from atm0s.* to atm0sxul.* since these are also used in the injected scripts (this did help, but not completely, read below) Now, when opening geenstijl.nl and then after page loaded completely, then closing the tab, there are no ghost compartments any more. But, I noticed, that when a new webworker ( chrome://igeenstijlplugin/content/scripts/atm0s.geenstijl.mozilla.extension.commentFormatterWorker2.js ) is created more then once in a single TAB, ghost compartments arise. This can be accomplished by either, reloading the page for a second time, or changing the KUDO filter (below the main post and the comments) this also invokes a new webworker. I checked to see if the call to webworker.Terminate() is actually called, and I can confirm this. I am running out of ideas now, and I think you were just a little to fast by stating that this is_not_a_firefox_bug.. I am not so sure.
Totally of-topic: I noticed that the Developers hub (since today) localized for me to Dutch. O-My-God!! Even MSDN doesn't translate that-bad!! Laughing stock! I changed back to English ;)
I know, my English is evenly bad :)
(In reply to Atmos from comment #4) > But, I noticed, that when a new webworker ( > chrome://igeenstijlplugin/content/scripts/atm0s.geenstijl.mozilla.extension. > commentFormatterWorker2.js ) is created more then once in a single TAB, > ghost compartments arise. > > This can be accomplished by either, reloading the page for a second time, or > changing the KUDO filter (below the main post and the comments) this also > invokes a new webworker. > > I checked to see if the call to webworker.Terminate() is actually called, > and I can confirm this. So, the webworker holds a reference to the page and Terminate() is called but isn't terminated? Or is terminated but there is still a reference? Cc'ing Justin on this to see if he has any insight into the issue or if its come up with other addons.
(In reply to Andrew Williamson [:eviljeff] from comment #7) > So, the webworker holds a reference to the page and Terminate() is called > but isn't terminated? Or is terminated but there is still a reference? > > Cc'ing Justin on this to see if he has any insight into the issue or if its > come up with other addons. I don't know if it is terminated or not, the webworker is eh working, everything does what it should, only it seems to be related to the zombie compartment issue, for it is the only thing different from closing the tab right away. I can not debug firefox, only my own code, and that does what I expect it to do, maybe someone should inspect the workings of geenstijlplugin.js and see if that COULD cause a problem with memory leaks according to what is expected. Greetz, atm0s
And please inspect that latest uploaded version, not the one mentioned above.
Ah *facepalm* I just realized that my webworker also uses the same namespace as the injected scripts. I always thought that these injected scripts would be running in a different "scope", so that could not give any problems, let me first try to change this, and and see if this solves the problem. I'll report my findings later.
I tried everyhing.. I think the problem lays in the fact that my injected scripts are not running in the scope of the page, but instead in something much higher up like Chrome .. What do I need to change to the scheme below, to inject my scripts in the security scope of the DOM content, so the garbage collector will remove them along with the rest when the page is unloaded? --> OnDomContentLoad = function(e) { // skipping some code here that checks on location and top frame checking // Reference window object var windowObj = e.target.defaultView; if (windowObj.wrappedJSObject) windowObj = windowObj.wrappedJSObject; loadScript(....) } loadScript = function(fileName, window) { // Create element var newScript = window.document.createElement('script'); // Mark type as javascript newScript.setAttribute('type', 'text/javascript'); // Reference local plug-in script file newScript.setAttribute('src', 'chrome://igeenstijlplugin/content/scripts/' + fileName); // Append it to the DOM window.document.body.appendChild(newScript); } Thanks in advance for your help solving this.
I don't see anything obviously wrong with the code attached in comment 11. What makes you think this is where the problem lies?
(In reply to Justin Lebar [:jlebar] from comment #12) > I don't see anything obviously wrong with the code attached in comment 11. > What makes you think this is where the problem lies? Correct me if I am wronge, but this problem can only happin when some reference exist between the (zombie) compartment and the plug-in, causing the garbage collector to think the zombie compartment is still in use, right? But the only references I make, is injecting these script the way I showed comment #11, and the previously mentioned (Hitched) functions I add to the Dom Window. These (hitched) functions, are then later set to NULL by an Unload handler (I checked by inserting alert()'s, these are always called) Besides these two things, the only thing that is left, are the fact that I keep copies of objects in my plug-in code (passed as json strings between injected scripts and plugin-code). Anyway, I have no idea what it could be. Please use this latest version of the plug-in if you want to see my code, this version isn't uploaded to Mozilla yet, but includes all the namespace clean-ups I made: http://search-geenstijl.thruhere.net/search/Geenstijl_Extension_2.11.0.3.xpi
Whiteboard: [MemShrink] → [MemShrink:P3]
I just tested the version posted on comment #13 and I can't reproduce the leaks. Please submit it to AMO so other editors can have a look. Thanks!
fyi, I just reviewed the version uploaded to AMO and had to preliminary review it due to the memory leak still existing. Fingers crossed that this time its just a syntax error on the removeEventListener call though.
I would also like to point out that the STR in previous comments didn't work consistently for me in order to find the leak. I can consistently reproduce the leak by going to the home page (http://www.geenstijl.nl) and then closing the tab.
I uploaded a new version in which the syntax error is corrected. Now, I can not reproduce the zombie compartment anymore. :) Other issue, if you open (with plug-in) an url like: http://www.geenstijl.nl/mt/archieven/2012/02/rijkman_groenink_284_miljoen_i.html#c148976571 Wait for the page to completely load, then select the combobox that says "toon: alles" and change it to "> -2", the page becomes empty, something goes horribly wronge :) What it does, is start a new WebWorker that only regenerates the HTML, using Venkman I can see the right data being serialized and send using Postmessage() to the worker, but it does not allow me to debug the worker js files. Does anyone know, how to debug this? I think this code broke with some Firefox version somewhere around version 6 or so.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.