Thank you for reporting this bug. We will make it our priority to review this report.
  1. #1
    Ext User
    Join Date
    Dec 2009
    Posts
    6
    Vote Rating
    0
    tungj is on a distinguished road

      0  

    Default [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:

    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>
    See this URL : http://


    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.
    If this does get changed, could you please explain which approach you took? I'm working on memory cleanup for a large (>200MB memory) single-page application at present, so it'd be helpful to know more about events and leaks. Thank you!
    Attached Files
    Last edited by tungj; 12 Aug 2010 at 4:22 AM. Reason: Fix type, ZZZ_LeakManager for ZZZ_LeakMarker

  2. #2
    Sencha User Jamie Avins's Avatar
    Join Date
    Mar 2007
    Location
    Redwood City, California
    Posts
    3,661
    Vote Rating
    18
    Jamie Avins is a jewel in the rough Jamie Avins is a jewel in the rough Jamie Avins is a jewel in the rough

      0  

    Default


    My guess is there is some legacy code that was not well documented and left in instead of being removed.

  3. #3
    Ext JS Premium Member
    Join Date
    Sep 2008
    Posts
    7
    Vote Rating
    0
    igorvin is on a distinguished road

      0  

    Default


    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.

Similar Threads

  1. Replies: 0
    Last Post: 12 Aug 2010, 7:11 AM
  2. Chrome slower than Firefox o.O ?
    By Vepe in forum Ext 3.x: Help & Discussion
    Replies: 10
    Last Post: 24 Feb 2010, 11:54 PM
  3. [OPEN] [2.3.0] Chrome hangs - EditorGrid,JsonStore, ComboBox
    By aleczapka in forum Ext 2.x: Bugs
    Replies: 2
    Last Post: 16 Sep 2009, 12:46 AM
  4. [Help]Ext.EventManager.addListener,IE is ok, but Firefox not display!
    By greco in forum Ext 2.x: Help & Discussion
    Replies: 4
    Last Post: 4 Mar 2008, 12:43 AM
  5. extra () on the last line of EventManager.js
    By sjivan in forum Community Discussion
    Replies: 8
    Last Post: 28 Nov 2006, 8:33 PM

Thread Participants: 2