2

Edit: On further examination Firefox does not seem to be doing this, but Chrome definitely does. I guess its just a bug with a new browser - for every event an I/O Read also occurs in Chrome but not in FF.

When I load the following page in a browser (I've tested in Chrome and Firefox 3 under Vista) and move the mouse around the memory always increases and does not ever seems to recede.

Is this:

  1. expected behaviour from a browser
  2. a memory leak in the browser or
  3. a memory leak in the presented code?

.

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">
<html>
<head>
    <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1" />
    <title>test</title>
</head>
<body>
   <script>
      var createEl = function (i) {
          var el = document.createElement("div");
          var t = document.createTextNode(i.toString());
          el.appendChild(t);
          t=null;
          el.id=i.toString();

          var fn = function (e) {};
          el.addEventListener("mouseover", fn, false);
          //el.onmouseover = fn;
          fn = null;

          try{
            return el;
          }
          finally{
            el=null;
          }
          //return (el = [el]).pop();
        };

        var i,x;
        for (i= 0; i < 100; i++){
          x = createEl(i)
          document.body.appendChild(x);
          x = null;
        }
   </script>
</body>
</html>

The (el = [el].pop()) and the try/finally ideas are both from here, though they do not either seem to help - understandably since they are only meant to be ie6 fixes.

I have also experimented with using the addEventListener and the onmouseover methods of adding the events. The only way I have found to prevent memory from increasing is to comment out both lines of code.

user31409
  • 23
  • 1
  • 4
  • Do you have anything in the mouseover event handler in your real script? I can't reproduce your problem. (FF 3.0.5 Linux) – meouw Jan 29 '09 at 13:20
  • The above code above is verbatim what I am testing and getting problems with. It doesn't happen right away, it takes about 20 seconds of moving your mouse around for it to start showing. It seems to me as if some buffer is eventually getting overwhelmed. – user31409 Jan 29 '09 at 14:27
  • On further testing FF doesn't seem to have this issue on the above code, only Chrome. – user31409 Jan 29 '09 at 15:05

3 Answers3

2

fn is a closure even with no code in it. E.g. try debug with Firebug and set the breakpoint inside that function. All variables defined in closure (fn code + variables that hang around = closure) are theoretically accessible(though I don't know how to access them on practice).

1

Well this is not how I'd handle this at all, and I can't replicate the behaviour so all I can tell you is the problem you're having memory wise is likely because of whatever is in the fn function, and unless it's a closure you should be defining fn outside of the createEl function and merely referencing it within, so only a single instance of that exists in memory.

You need to handle the event binding better though (this is not xbrowser safe - at this point I hesitate to suggest jQuery), and that whole (el =[el]).pop() reeks of voodoo cruft to me, though I'm happy to be corrected if someone can explain exactly what it achieves.

annakata
  • 74,572
  • 17
  • 113
  • 180
  • This is just meant to be a test to identify whether a browser leaks, not xbrowser safe. I am not considering IE worth testing on for mem leaks. The fn is not a closure. It has no code inside it. – user31409 Jan 29 '09 at 14:01
  • The "voodoo" ensures that no reference to el is maintained. el = [el] makes the value that used to be in el now in an array in el. The subsequent pop, removes the contents of the array, and in the process removes any ref to it. This ensures that there is no hanging refs left to the created DOM el. – user31409 Jan 29 '09 at 14:05
  • so, break fn out and post the code in it if you still have a problem – annakata Jan 29 '09 at 14:05
  • I changed it so that fn was declared at the line above createEl, but it has the same effect. I don't see why it would matter if the function is created per dom element or only once, so long as it is not being created for each event. My guess is that event objects themselves leak memory. – user31409 Jan 29 '09 at 14:18
  • @wbecker - is there any evidence of this? google yields nothing so far, and I fail to see why the GC would be any more interested in this than the var dropping out of function scope – annakata Jan 29 '09 at 14:21
  • if events leaked memory *everyone* in JS land would know about it. Breaking out fn matters because if you create it inline that's a new instance with each call which is persisted in memory by it's association with the dom element created. Broken out it's just one instance with a lot of references – annakata Jan 29 '09 at 14:24
  • No evidence per se. Just observing that it is occuring. I have googled too - thats why I am posting here! – user31409 Jan 29 '09 at 14:29
  • If you create it as I originally did, there is a new instance with each dom element, not each event. Each function will persist in memory, but this memory allocation only occurs at the start. No more memory should be created for each event. – user31409 Jan 29 '09 at 14:33
  • I didn't say each event, I said each call - i.e. 100 instances in memory vs 1. That's not going to grow, but it's not scaling, and bad enough. I don't think we can take this much further without seeing the contents of "fn" – annakata Jan 29 '09 at 14:42
  • There is nothing in fn! I can see a marked increase in Windows Task Manager (100-400kB per minute) with fn just as it is in that example. I will try it on another persons computer running XP and see if the problem exists there too. – user31409 Jan 29 '09 at 14:46
  • how exactly are you benchmarking this? Have you tried this tool - http://outofhanwell.com/ieleak/index.php?title=Main_Page - although I've just benchmarked the pop technique on Drip and found no benefit – annakata Jan 29 '09 at 15:10
1

Memory leaks related to event handlers are, generally speaking, related to enclosures. In other words, attaching a function to an event handler which points back to its element can prevent browsers from garbage-collecting either. (Thankfully, most newer browsers have "learned the trick" and no longer leak memory in this scenario, but there are a lot of older browsers floating around out there!)

Such an enclosure could look like this:

var el = document.createElement("div");
var fnOver = function(e) {
    el.innerHTML = "Mouse over!";
};
var fnOut = function(e) {
    el.innerHTML = "Mouse out.";
};

el.addEventListener("mouseover", fnOver, false);
el.addEventListener("mouseout", fnOut, false);

document.getElementsByTagName("body")[0].appendChild(el);

The fact that fnOver and fnOut reach out to their enclosing scope to reference el is what creates an enclosure (two, actually — one for each function) and can cause browsers to leak. Your code doesn't do anything like this, so creates no enclosures, so shouldn't cause a (well-behaved) browser to leak.

Just one of the bummer of beta software, I guess. :-)

Ben Blank
  • 54,908
  • 28
  • 127
  • 156