Why does the following code leak?

for (var i = 0; i < 100; i++) {
    var item = {};
    item.elem = document.createElement('div');
    document.body.appendChild(item.elem);
    item.addEvent = function(name,listener) {
        var self = this;
        var wrappedListener = function() {
            return listener.apply(self,arguments);
        }
        //Uh-oh creating a circular reference here!
        //The wrappedListener has a closure on self and therefore on item.elem.
        addEvent(this.elem,name,wrappedListener);
        return wrappedListener;
    }
    var wrap = item.addEvent('eventName',listen);

    //Now remove the eventHandler - this should free up the circular reference.
    removeEvent(item.elem, 'eventName', wrap);
    if (item.elem.parentNode) {
        item.elem.parentNode.removeChild(item.elem);
    }
    //item.elem = null; //With this also un-mented, the leak disappears.
    //The fact that I have to null item.elem tells me that something is holding
    //a reference to item, and therefore elem. Setting elem to null fixes the
    //problem, but since I am removing the event handler, I don't think this
    //should be required.
}

Note: addEvent and removeEvent are just to abstract attachEvent/addEventListener differences between Internet Explorer and other browsers.

I created a jsFiddle project which demonstrates the problem. Just fire up Internet Explorer 8 and watch it go in Task Manager or Process Explorer. Also, you will see the definition of addEvent and removeEvent there.

/

EDIT: Well, I came up with the following solution. It is not pretty, but it works! /

var item = {};
item.elem = document.createElement('div');
document.body.appendChild(item.elem);
item.addEvent = function(name,listener) {
    var wrappedListener = function() {
        //Access the scope through the callee properties.
        return listener.apply( arguments.callee.scope, arguments);
    }
    addEvent(this.elem,name,wrappedListener);
    //Save the scope not as a closure, but as a property on the handler.
    wrappedListener.scope = this
    return wrappedListener;
}
var wrap = item.addEvent('eventName',listen);
removeEvent(item.elem, 'eventName', wrap);
//Force the circular reference to GO AWAY.
wrap.scope = null
if (item.elem.parentNode) {
    item.elem.parentNode.removeChild(item.elem);
}
//item.elem = null; //No longer needed.

Why does the following code leak?

for (var i = 0; i < 100; i++) {
    var item = {};
    item.elem = document.createElement('div');
    document.body.appendChild(item.elem);
    item.addEvent = function(name,listener) {
        var self = this;
        var wrappedListener = function() {
            return listener.apply(self,arguments);
        }
        //Uh-oh creating a circular reference here!
        //The wrappedListener has a closure on self and therefore on item.elem.
        addEvent(this.elem,name,wrappedListener);
        return wrappedListener;
    }
    var wrap = item.addEvent('eventName',listen);

    //Now remove the eventHandler - this should free up the circular reference.
    removeEvent(item.elem, 'eventName', wrap);
    if (item.elem.parentNode) {
        item.elem.parentNode.removeChild(item.elem);
    }
    //item.elem = null; //With this also un-mented, the leak disappears.
    //The fact that I have to null item.elem tells me that something is holding
    //a reference to item, and therefore elem. Setting elem to null fixes the
    //problem, but since I am removing the event handler, I don't think this
    //should be required.
}

Note: addEvent and removeEvent are just to abstract attachEvent/addEventListener differences between Internet Explorer and other browsers.

I created a jsFiddle project which demonstrates the problem. Just fire up Internet Explorer 8 and watch it go in Task Manager or Process Explorer. Also, you will see the definition of addEvent and removeEvent there.

http://jsfiddle/rJ8x5/34/

EDIT: Well, I came up with the following solution. It is not pretty, but it works! http://jsfiddle/rJ8x5/43/

var item = {};
item.elem = document.createElement('div');
document.body.appendChild(item.elem);
item.addEvent = function(name,listener) {
    var wrappedListener = function() {
        //Access the scope through the callee properties.
        return listener.apply( arguments.callee.scope, arguments);
    }
    addEvent(this.elem,name,wrappedListener);
    //Save the scope not as a closure, but as a property on the handler.
    wrappedListener.scope = this
    return wrappedListener;
}
var wrap = item.addEvent('eventName',listen);
removeEvent(item.elem, 'eventName', wrap);
//Force the circular reference to GO AWAY.
wrap.scope = null
if (item.elem.parentNode) {
    item.elem.parentNode.removeChild(item.elem);
}
//item.elem = null; //No longer needed.

• javascript
• internet-explorer
• memory-leaks

• I know that IE used to have a bug where it would use separate garbage collection for native objects and javascript objects, so any time you form a cycle between javascript objects and native objects, neither garbage collector could clean it up. Could that be it? I'm thinking that item points to item.elem which is a div that has handlers that refer back to item. – btilly Commented Feb 4, 2011 at 22:13
• — Not really pertinent to your question, but you can eliminate the self variable by using listener.apply(item, arguments) instead. :-) – Ben Blank Commented Feb 4, 2011 at 22:15
• Totally! But this is actaully a much simplified version of a class that I wrote where the addEvent method does not have access to the instance so conveniently - hence var self = this – jordancpaul Commented Feb 4, 2011 at 23:00
• Opening post has been updated with a solution to the problem – jordancpaul Commented Mar 2, 2011 at 22:38

2 Answers 2

The problem is the events (as almost always in Internet Explorer, BTW).

Look at http://jsfiddle/rJ8x5/39/ and notice how it garbage collects fine.

You are creating circular references when you attach the events. Read more about it in Circular references to DOM objects on an HTML page cause a memory leak.

The code leaks because you are calling attachEvent improperly - Microsoft's documentation insists that the event name be a standard DHTML event.

If you change 'eventName' to 'click', it does not leak.

A more robust solution would be to change your event attaching code to check 'on'+eventname in domElement and refuse to attach the event if this is false.