46

Here's a dead-simple webpage that leaks memory in IE8 using jQuery (I detect memory leaks by watching the memory usage of my iexplore.exe process grow over time in the Windows Task Manager):

<html>
<head>
    <title>Test Page</title>
    <script type="text/javascript" src="jquery.js"></script>
</head>
<body>
<script type="text/javascript">
    function resetContent() {
        $("#content div").remove();
        for(var i=0; i<10000; i++) {
            $("#content").append("<div>Hello World!</div>");
        }
        setTimeout(resetTable, 2000);
    }
    $(resetContent);
</script>
<div id="content"></div>
</body>
</html>

Apparently even when calling the jQuery.remove() function I still experience some memory leakage. I can write my own remove function that experiences no memory leak as follows:

$.fn.removeWithoutLeaking = function() {
    this.each(function(i,e){
        if( e.parentNode )
            e.parentNode.removeChild(e);
    });
};

This works just fine and doesn't leak any memory. So why does jQuery leak memory? I created another remove function based on jQuery.remove() and this does indeed cause a leak:

$.fn.removeWithLeakage = function() {
    this.each(function(i,e) {
        $("*", e).add([e]).each(function(){
            $.event.remove(this);
            $.removeData(this);
        });
        if (e.parentNode)
            e.parentNode.removeChild(e);
    });
};

Interestingly, the memory leak seems to be caused by the each call which jQuery includes to prevent memory leaks from events and data associated with the DOM elements being deleted. When I call the removeWithoutLeaking function then my memory stays constant over time, but when I call removeWithLeakage instead then it just keeps growing.

My question is, what about that each call

$("*", e).add([e]).each(function(){
    $.event.remove(this);
    $.removeData(this);
});

could possibly be causing the memory leak?

EDIT: Fixed typo in code which, upon retesting, proved to have no effect on the results.

FURTHER EDIT: I have filed a bug report with the jQuery project, since this does seem to be a jQuery bug: http://dev.jquery.com/ticket/5285

Eli Courtwright
  • 186,300
  • 67
  • 213
  • 256
  • 1
    It's a good question, and kudos for finding this if it really is a bug. However you might get a better response from the actual jQuery guys, I would suggest starting a ticket with them if you're sure. http://dev.jquery.com/ – womp Sep 22 '09 at 21:24
  • 1
    Interesting. I can't immediately see any of the usual IE-memory-leak suspects where you get a reference loop between a host object and a native object, maybe this is some new kind of leak? Do you know if it happen only in IE8 Standards Mode, or in IE7-compatibility mode, or Quirks, or all three? – bobince Sep 22 '09 at 21:38
  • @Eli - shall I post the source for each() onto the end of the question? – Russ Cam Sep 22 '09 at 21:48
  • @bobince: It leaks in both Standards and IE7-compatibility mode. I haven't had a chance to test it out in quirks mode yet. – Eli Courtwright Sep 22 '09 at 22:20
  • @Russ: If you think that the cause must be in the each method, then go ahead and add the source. I'm not convinced though; using "*" as a selector could just as easily be the culprit, or the fact that we're using a closure. – Eli Courtwright Sep 22 '09 at 22:25
  • It'd be nice to edit the original q to indicate which jquery version. I am also researching some mem leak issues in IE w.r.t. jquery but it's hard to know if an issue if already fixed or not. – Jiho Han Feb 14 '11 at 21:21

6 Answers6

59

I thought David might be onto something with the alleged removeChild leak, but I can't reproduce it in IE8... it may well happen in earlier browsers, but that's not what we have here. If I manually removeChild the divs there is no leak; if I alter jQuery to use outerHTML= '' (or move-to-bin followed by bin.innerHTML) instead of removeChild there is still a leak.

In a process of elimination I started hacking at bits of remove in jQuery. line 1244 in 1.3.2:

//jQuery.event.remove(this);
jQuery.removeData(this);

Commenting out that line resulted in no leak.

So, let's look at event.remove, it calls data('events') to see if there are any events attached to the element. What is data doing?

// Compute a unique ID for the element
if ( !id )
    id = elem[ expando ] = ++uuid;

Oh. So it's adding one of jQuery's uuid-to-data-lookup entry hack properties for every element it even tries to read data on, which includes every single descendent of an element you're removing! How silly. I can short-circuit that by putting this line just before it:

// Don't create ID/lookup if we're only reading non-present data
if (!id && data===undefined)
    return undefined;

which appears to fix the leak for this case in IE8. Can't guarantee it won't break something else in the maze that is jQuery, but logically it makes sense.

As far as I can work out, the leak is simply the jQuery.cache Object (which is the data store, not a really a cache as such) getting bigger and bigger as a new key is added for every removed element. Although removeData should be removing those cache entries OK, IE does not appear to recover the space when you delete a key from an Object.

(Either way, this is an example of the sort of jQuery behaviour I don't appreciate. It is doing far too much under the hood for what should be a trivially simple operation... some of which is pretty questionable stuff. The whole thing with the expando and what jQuery does to innerHTML via regex to prevent that showing as an attribute in IE is just broken and ugly. And the habit of making the getter and setter the same function is confusing and, here, results in the bug.)

[Weirdly, leaving the leaktest for extended periods of time ended up occasionally giving totally spurious errors in jquery.js before the memory actually ran out... there was something like ‘unexpected command’, and I noted a ‘nodeName is null or not an object’ at line 667, which as far as I can see shouldn't even have been run, let alone that there is a check there for nodeName being null! IE is not giving me much confidence here...]

Community
  • 1
  • 1
bobince
  • 528,062
  • 107
  • 651
  • 834
  • Yep, that definitely fixes it. I'll probably end up posting another question asking why, since as you point out the jQuery.removeData function should be getting rid of these entries. Oh well, thanks a bunch for the great help. – Eli Courtwright Sep 23 '09 at 13:19
  • 1
    It gets rid of the entries but the cache Object, even with fewer properties, fails to shrink its memory usage. Perhaps try re-using ids by keeping a ‘lowest free id’ counter rather than making a new uuid each time? Either way, reading data from an object without data shouldn't add an empty data container. – bobince Sep 23 '09 at 13:28
  • 1
    +1 and congratulations for being able to read inot jQuery maze. – Marco Demaio Sep 03 '10 at 13:14
  • 4
    @bobince do you knjow if this was fixed in later versions of jQuery? I'm having the same problem as described, but now the code looks different, so I can't use your fix as is. I will have to re-figure the maze all again... – Mohoch Dec 26 '12 at 08:48
5

Seems to be fixed in jQuery 1.5 (23. february version). I ran into the same problem with 1.4.2 and fixed it first with dom removal as above and then by trying the new version.

Ben
  • 188
  • 4
  • 15
4

Element removal is an inherent DOM problem. Which is going to stay with us. Ditto.

jQuery.fn.flush = function()
/// <summary>
/// $().flush() re-makes the current element stack inside $() 
/// thus flushing-out the non-referenced elements
/// left inside after numerous remove's, append's etc ...
/// </summary>
{ return jQuery(this.context).find(this.selector); }

Instead of hacking jQ, I use this extension. Especially in the pages with lot of removes() and clones() :

$exact = $("whatever").append("complex html").remove().flush().clone();

And also the next one does help :

// remove all event bindings , 
// and the jQ data made for jQ event handling
jQuery.unbindall = function () { jQuery('*').unbind(); }
//
$(document).unload(function() { 
  jQuery.unbindall();
});
Frumples
  • 425
  • 1
  • 4
  • 18
2

See the jQuery 1.4 roadmap at http://docs.jquery.com/JQuery_1.4_Roadmap. Specifically, the section "Use .outerHTML to cleanup after .remove()" deals with memory leak issues occurring in IE due to the remove function being called.

Perhaps your issues will be resolved with the next release.

David Andres
  • 31,351
  • 7
  • 46
  • 36
1

JQuery 1.4.1 has following:

    cleanData: function (elems) {
        for (var i = 0, elem, id; (elem = elems[i]) != null; i++) {
            jQuery.event.remove(elem);
            jQuery.removeData(elem);
        }
    }

Here is what I had to modify to eliminate leaking problem:

    cleanData: function (elems) {
        for (var i = 0, elem, id; (elem = elems[i]) != null; i++) {
            jQuery.event.remove(elem);
            jQuery.removeData(elem);
            jQuery.purge(elem);
        }
    }

added function:

    purge: function (d) {
        var a = d.childNodes;
        if (a) {
            var remove = false;
            while (!remove) {
                var l = a.length;
                for (i = 0; i < l; i += 1) {
                    var child = a[i];
                    if (child.childNodes.length == 0) {
                        jQuery.event.remove(child);
                        d.removeChild(child);
                        remove = true;
                        break;
                    }
                    else {
                        jQuery.purge(child);
                    }
                }
                if (remove) {
                    remove = false;
                } else {
                    break;
                }
            }
        }
    },
Basil
  • 15
  • 2
  • I changed purge to make it even more efficient (at least 30%) – Basil Dec 13 '12 at 18:15
  • 1
    Not only does this NOT work, on large sets of elements it makes IE chug. It sat for 10 minutes while memory usage, thread count, and CPU usage climbed. Works fine for small amount of elements but not for large amounts that use a lot of memory...which is the problem in the first place. – AS7K Jun 01 '16 at 21:50
1

Does it still leak if you call empty instead of remove?

$("#content").empty();
Josh Stodola
  • 81,538
  • 47
  • 180
  • 227