-
12 Aug 2010 4:11 AM #1
[OPEN-1198] EditorGrid Pseudoleak in Chrome/Firefox due to EventManager.js line 97
[OPEN-1198] EditorGrid Pseudoleak in Chrome/Firefox due to EventManager.js line 97
Ext version tested:
- Ext 3.2.1 rev ____
Adapter used:
- ext
css used:
- only default ext-all.css
Browser versions tested against:
- IE8 //doesn't leak in IE since it uses attachEvent
- FF3 (firebug 1.5.4 installed)
- Chrome
Operating System:
- Ubuntu 9.10
- WinXP Pro
Description:
- The internal addListener in EventManager.js line 97 has the code
Ext.EventManager.addListener(WINDOW, 'unload', function(){
el.removeEventListener.apply(el, args);
})
which causes a pseudoleak (the memory isn't released until the window unloads).
This is problematic since we have a one-page application using property grids with lots of properties, and the grid (and therefore store) stays in memory despite being destroyed due to EditorGrid.js line 154
this.getGridEl().on('mousewheel', this.stopEditing.createDelegate(this, [true]), this);
Test Case:
See this URL : http://Code:<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8" /> <title>Property Grid Leak Test</title> <link rel="stylesheet" type="text/css" href="../../resources/css/ext-all.css" /> <link rel="stylesheet" type="text/css" href="../shared/examples.css" /> <style type="text/css"> </style> <script type="text/javascript" src="../../adapter/ext/ext-base.js"></script> <script type="text/javascript" src="../../ext-all-debug.js"></script> <script type="text/javascript" src="../shared/examples.js"></script> <script type="text/javascript"> Ext.onReady(function(){ Ext.fly('create').on('click', make, window, {single:true}); // This is just a marker to show in Chrome if the property grid is leaked var ZZZ_LeakMarker = function(){}; function make(){ var share = {}; share.propertiesGrid = new Ext.grid.PropertyGrid({}); // Set a property on the grid so we know whether the grid is still in memory // otherwise in Chrome heap debugger it'll just be listed as another 'constructor' share.propertiesGrid._leakMarker = new ZZZ_LeakMarker(); share.panel = new Ext.Panel({ height: 400, renderTo: 'prop-grid', items: share.propertiesGrid, tbar: [{ text: 'Destroy', handler: function(){ window.setTimeout(function(){ share.propertiesGrid = null; share.panel.destroy(); share.panel = null; // Point the static Ext.fly instance at a div element to avoid false positives about leaks in sIEve Ext.fly('irrelevant'); }, 100); } }] }); } var applyFix = false; if (applyFix) { // Patch createInterceptor not to leak references to .target and .method, is mentioned as fixed in 3.2.2 changelog Function.prototype.createInterceptor = function(fcn, scope){ var method = this; return !Ext.isFunction(fcn) ? this : function() { var me = this, args = arguments; fcn.target = me; fcn.method = method; var ret = (fcn.apply(scope || me || window, args) !== false); // If you don't null the method and target, then closures will stay and keep a reference to all // objects scoped to the closure until the next time the intercepted method is called. fcn.target = fcn.method = null; return ret ? method.apply(me || window, args) : null; }; }; // Essentially want to comment out line 97 of Ext 3.2.1 EventManager.js which pseudoleaks. e.g. Ext.grid.EditorGridPanel // initEvents this.getGridEl().on('mousewheel', this.stopEditing.createDelegate(this, [true]), this); // // Seems to be a workaround for jQuery which we don't need since we don't use the jQuery adaptor; // mousewheel doesn't leak in sIEve in IE7 nor IE8 // Ext.EventManager.addListener(WINDOW, 'unload', function(){ // el.removeEventListener.apply(el, args); // }); // Only affects Chrome/Firefox, IE doesn't have an addEventListener so doesn't go into that part of the code. if (document.addEventListener) { Ext.EventManager.addListener = Ext.EventManager.on = Ext.EventManager.addListener.createInterceptor(function(element, eventName, fn, scope, options){ if (element === window && eventName === 'unload') { var stringFn = String(fn); // Testing for both normal and minified version of the line 97 code , e.g. Ext-all-debug and Ext-all // 'function (){\n el.removeEventListener.apply(el, args);\n}' // 'function (){\n E.removeEventListener.apply(E,I);\n}' if (/^\s*function\s*\(\)\s*\{\s*(\w+)\.removeEventListener\.apply\(\1,\s*\w+\);?\s*\}$/.test(stringFn)) { return false; } } }); } } }); </script> </head> <body> <p><a id="create">Create PropertyGrid</a></p> <div id="prop-grid"></div> <div id="irrelevant"></div> </body> </html>
Steps to reproduce the problem:
- Copy test code to /ext-3.2.1/examples/grid/property-grid-leak-test.html
- View test code in Chrome, press Ctrl-Shift-J to bring up debugger tools, go to Profiles tab, press 'Take Heap Snapshot' (the eye icon).
- In page, click 'Create PropertyGrid'. Verify that property grid appears, go back to Profile tab and take another heap snapshot. Verify that an instance of ZZZ_LeakMarker was created and is referred to by the EditorGrid (it appears as J.I.constructor.e.I.constructor in the heap debugger).
- In page, press 'Destroy' in tab button. Wait ~30 seconds to ensure the Ext.elCache garbage collection code has run, and go back to Profile tab and take heap snapshot. Verify that the ZZZ_LeakMarker still remains in memory, since the EditorGrid is still in memory (held in the closure scope of
EventManager.js line 97)
The result that was expected:
- ZZZ_LeakMarker and the EditorGrid should be freed, the count should go back to zero.
The result that occurs instead:
- ZZZ_LeakMarker and the EditorGrid stay in memory , count is one.
Screenshot or Video:
Debugging already done:
- Tracing down pseudoleak, trying various fixes for pseudoleak. There's a var applyFix=false; if you set it true it overrides Ext.EventManager.addListener to avoid registering the window.unload callback, which avoids the pseudoleak in Chrome (and presumably Firefox). This doesn't affect IE since it uses attachEvent. If you repeat the test with applyFix=true, the ZZZ_LeakMarker and EditorGrid are freed.
Possible fix:
- Comment out the lines EventManager.js line 97. Are they really required when using ext-base; I thought only IE required removing event listeners from DOM elements before removing them from the DOM? (please correct me if I'm wrong) Setting applyFix=true essentially comments out those lines without violating our commercial license with a really hacky override; but would slow down all event registration.
- The code mentions it's a workaround for JQuery, so presumably it can't be just commented out. I've attached a version of EventManager that keeps track of the registered window.onunload listeners and deregisters them on event remove/removeAll with addCleanup/doCleanup. It seems to allow the property grid to get freed.
Last edited by tungj; 12 Aug 2010 at 4:22 AM. Reason: Fix type, ZZZ_LeakManager for ZZZ_LeakMarker
-
12 Aug 2010 10:12 AM #2Sencha - Sencha Touch Dev Team
- Join Date
- Mar 2007
- Location
- Redwood City, California
- Posts
- 3,659
- Vote Rating
- 14
My guess is there is some legacy code that was not well documented and left in instead of being removed.
-
24 Sep 2010 4:51 PM #3
We see memory leaks using Ext.grid.EditorGridPanel as well - we tested proposed above solution (the first one - commented out code inside EventManager) and it fixed the issue. In which version of Extjs it's going to be fixed? Thanks.
Thank you for reporting this bug. We will make it our priority to review this report.
Similar Threads
-
[OPEN-1200] Ext.ux.form.ItemSelector leak due to ItemSelector.js line 93/117
By tungj in forum Ext 3.x: BugsReplies: 0Last Post: 12 Aug 2010, 7:11 AM -
Chrome slower than Firefox o.O ?
By Vepe in forum Ext 3.x: Help & DiscussionReplies: 10Last Post: 24 Feb 2010, 11:54 PM -
[OPEN] [2.3.0] Chrome hangs - EditorGrid,JsonStore, ComboBox
By aleczapka in forum Ext 2.x: BugsReplies: 2Last Post: 16 Sep 2009, 12:46 AM -
[Help]Ext.EventManager.addListener,IE is ok, but Firefox not display!
By greco in forum Ext 2.x: Help & DiscussionReplies: 4Last Post: 4 Mar 2008, 12:43 AM -
extra () on the last line of EventManager.js
By sjivan in forum Community DiscussionReplies: 8Last Post: 28 Nov 2006, 8:33 PM


Reply With Quote