This project is archived and is in readonly mode.

#445 ✓wontfix
Declan Conlon

IE Memory leak when Adding/Removing table rows

Reported by Declan Conlon | October 28th, 2008 @ 05:54 PM | in 2.0 (closed)

I have a very simple test case that adds and removes a row in a table every second. A memory leak is reported by sIEve. I have tried playing about a lot with different methods of deleting table rows that have worked in the past for me with divs. The question I ask is where does the problem lie, Mootools, sIEve, IE itself or how my code is using Mootools?

I have the following html,


<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
        "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html lang="en">
<head>
<title>TEST</title>
<meta http-equiv="content-type" content="text/html; charset=UTF-8"/>
<link rel="stylesheet" href="main.css" type="text/css" media="screen"/>
<script src="mootools.js" type="text/javascript"></script>
<script src="mootools_more.js" type="text/javascript"></script>
<script src="main.js" type="text/javascript"></script>
</head>

<body>
   <div class="main">
   <table id="content_table" class="t1style">
      <tbody id="cbody">
         <tr id="row0"><td>Cell 1</td><td>Cell 2</td></tr>
      </tbody>
   </table>
Some content!
   </div>
</body>

</html>

and the following fragment of js using mootools 1.2.1


var counter = 1;
var lastrow = 0;

var updater = function(){
   var td1 = new Element('td', { 'html' : 'td'+counter++ } );
   var td2 = new Element('td', { 'html' : 'td'+counter++ } );
   var tr = new Element('tr', { 'id' : 'row'+(lastrow+1) } );
   td1.inject(tr);
   td2.inject(tr);

   tr.inject($('cbody'),'top');
   $('row'+lastrow++).destroy();
}

window.addEvent('domready', function() {
   updater.periodical( 1000 );
} );

Comments and changes to this ticket

  • Declan Conlon
  • Daniel Steigerwald

    Daniel Steigerwald October 28th, 2008 @ 08:29 PM

    Do not trust any leak detector, nor sieve, nor drip, nor IE leak. They all lie all the time. If you want to be sure, if code leaks, you have to test it with F5 pressing and seeing process explorer memory consumption.

  • Declan Conlon

    Declan Conlon October 29th, 2008 @ 11:38 AM

    I can confirm that this leak is not just a leak checker issue. Changing the javascript to loop and add/remove a lot magnifies the problem.

    
    var counter = 1;
    var lastrow = 0;
    
    var updater = function(){
       for( var i = 0; i < 100; i++ ){
          var td1 = new Element('td', { 'html' : 'td'+counter++ } );
          var td2 = new Element('td', { 'html' : 'td'+counter++ } );
          var tr = new Element('tr', { 'id' : 'row'+(lastrow+1) } );
          td1.inject(tr);
          td2.inject(tr);
    
          tr.inject($('cbody'),'top');
          $('row'+lastrow++).destroy();
       }
    }
    
    window.addEvent('domready', function() {
       updater.periodical( 1000 );
    } );
    
  • Declan Conlon

    Declan Conlon October 29th, 2008 @ 02:03 PM

    This leaking doesn't seem limited to tables. Perhaps I am missing something here. Does Element.destroy() actually go about totally removing all reverences to an element and make it available for garbage collection?

    Attached is a test case which exhibits the issue with a couple of plain divs being added and removed repeatedly.

    I have in the past used the innerHTML hack to prevent leaks,

    
    element.set( 'html', '' );
    

    This doesn't work for tables but it does for this div only case. If I replace the call to destroy,

    
    $('row'+lastrow++).destroy();
    

    with,

    
    IEremoveChild($('row'+lastrow++);
    

    whose implementation is,

    
    function IEremoveChild(elem)
    {
       var garbageBin = $('FilthyIEGarbageBin');
       if(!garbageBin){
          garbageBin = new Element('div',{'id' : 'FilthyIEGarbageBin', 'style' : 'display: none' });
          document.body.appendChild(garbageBin);
       }
    
       // move the element to the garbage bin
       garbageBin.adopt(elem);
       garbageBin.set( 'html', '' );
       delete elem;
    }
    

    no memory is leaked at all. Any suggestions on where to go next with this?

  • Matt B

    Matt B December 23rd, 2008 @ 05:33 PM

    • Assigned user cleared.

    Is there any followup to this issue reported by Declan? I am having a similar issue.

  • Kenton

    Kenton December 23rd, 2008 @ 06:09 PM

    Just to chime in I'm also having this problem.

  • Daniel Steigerwald

    Daniel Steigerwald December 23rd, 2008 @ 09:13 PM

    Hey, just before I will test it, is everyone convinced, that memory will not be released after F5?

    FYI: Element.destroy really means Element.dispose + element.removeEvents. Still confusing. Element.dispose should be named as Element.remove. So no destroying, nor disposing, only removing from DOM and removing of events.

    Only what moo can do, is to call clearAttributes for IE, which remove all events, but it is called only for unload event. From my point of view, it is sufficient for browser memory management between web pages. If somebody has a problem with memory consumption in/during one page, call Bill G.

  • Declan Conlon

    Declan Conlon January 5th, 2009 @ 11:42 AM

    W.R.T. in page memory management, while I agree that the buck ultimately stops with IE's inherently poor implementation and I agree that we should be keeping Moo clean, it would be useful in some instances to provide 'stinky' functions which allow work-arounds for major browser issues. See my IE hack above for what I had in mind, and yes I do agree that it is vile.

    Memory seems to be lost on page reloads in this case but going forward I would hope Moo could help people who are trying to write web apps that would benefit from memory management in-page.

  • fakedarren

    fakedarren February 8th, 2010 @ 04:56 PM

    • State changed from “new” to “hold”
    • Assigned user set to “fakedarren”

    I'll speak to the other devs about this but I assume the implementation will be improved / change in 2.0 - for the better hopefully. Put on hold. Apologies for the delayed response.

  • Christoph Pojer

    Christoph Pojer November 9th, 2010 @ 07:25 PM

    • State changed from “hold” to “wontfix”
    • Milestone order changed from “0” to “0”

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Shared Ticket Bins

Attachments

Tags

Referenced by

Pages