Bug Tracker

Ticket #8171 (closed bug: invalid)

Opened 4 years ago

Last modified 2 years ago

Memory leak in IE8 with .remove()/jQuery.cleanData()

Reported by: smudgeface Owned by:
Priority: low Milestone: 1.next
Component: manipulation Version: 1.5
Keywords: Cc:
Blocking: Blocked by:

Description

I believe I may have found a memory leak in the latest release of jQuery.

The following code leaks:

var  listen = function(){};
var testLeak = function(){
    for(var i = 0; i<100; i++){
        var item = {};
        item.elem = $(document.createElement('div'));
        item.addListener = function(name,listener){
            var self = this;
            var wrappedListener = function(){
                return listener.apply(self,arguments);
            }
            this.elem.bind(name, wrappedListener);
            wrappedListener = null;
        }
        item.addListener('eventName',listen );
        item.elem.unbind();
        item.elem.remove();  //with this un-commented, the loop leaks
        // item.elem = null; //with this also un-commented, the leak dissapears
    }
};
    
$(document).ready(function(){
    setInterval(testLeak, 100);
}

also, I have created a jsfiddle link which demonstrates the issue:

 http://jsfiddle.net/rJ8x5/5/

It is important to note that if I do not call .remove(), this does not leak, and if I do call .remove() but set the .elem reference to null, it also does not leak. It is as though jQuery is holding a reference to item when I call .remove() which, in-turn, holds a reference to .elem. Any thoughts? As you can see, I have set the wrappedListener to null in an attempt to prevent any inadvertent closures, but this does not help.

Change History

comment:1 Changed 4 years ago by jitter

  • Priority changed from undecided to low
  • Resolution set to invalid
  • Status changed from new to closed
  • Component changed from unfiled to manipulation

Thanks for taking the time to contribute to the jQuery project by writing a bug report and providing a test case!

After checking your report I came to the conclusion that this isn't a bug in jQuery but rather a wrong assumption about what the .remove() method does.

The  .remove() documentation reads:

Description: Remove the set of matched elements from the DOM.

In your case item.elem is a jQuery object which holds a detached div. Thus .remove() can't remove it from the DOM as the div isn't in the DOM.

So just leave away the item.elem.remove() line and use item.elem = null.

comment:2 Changed 4 years ago by smudgeface

Respectfully, I disagree with this conclusion.

if you read further in the documentation you will see

In addition to the elements themselves, all bound events and jQuery data associated with the elements are removed.

this is because before the element is removed, two calls to jQuery.cleanData is made.

the actual remove functionality is performed as

if ( elem.parentNode ) {
    elem.parentNode.removeChild( elem );
}

this will valid whether the Element is in the DOM or not. The real problem here is in jQuery.cleanData, which is referred to in this bug title

Version 0, edited 4 years ago by smudgeface (next)

comment:3 Changed 4 years ago by smudgeface

BTW, the same behavior is exhibited even if the Element is in the DOM

var  listen = function(){};
var testLeak = function(){
    for(var i = 0; i<100; i++){
		item.elem = $(document.createElement('div'));
		$(document.body).append(item.elem);
		item.addListener = function(name,listener){
			var self = this;
			var wrappedListener = function(){
				return listener.apply(self,arguments);
			}
			this.elem.bind(name, wrappedListener);
			wrappedListener = null;
		}
		item.addListener('eventName',listen);
		item.elem.unbind();
		item.elem.remove();  //with this un-commented, the loop leaks
		// // item.elem = null; //with this also un-commented, the leak dissapears
    }
};
    
$(document).ready(function(){
    setInterval(testLeak, 100);
}

in this case I am using .remove() exactly as the documentation specifies

comment:4 Changed 4 years ago by anonymous

Finally! I'm experiencing a similar problem. I wrap my own callback like smudgeface above, and upon the remove() method I'm seeing a leak in IE8.

comment:5 Changed 4 years ago by jitter

  • Status changed from closed to reopened
  • Resolution invalid deleted

Reopening as I missed some points the reporter made.


Replying to smudgeface:

if you read further in the documentation you will see

In addition to the elements themselves, all bound events and jQuery data associated with the elements are removed.

I'm (as you probably can imagine) very well aware of that and how this is done internally. And as you can verify with this  test case by commenting/uncommenting the unbind/remove lines they do exactly that.


Now it would be nice if you could provide more information. Does this also happen with other browsers? Does this only happen with jQuery 1.5 (what about earlier versions 1.4.4, 1.4.2, ...)

comment:6 Changed 4 years ago by smudgeface

Sorry, I should know better than to file partial bug reports.

Methodology:
-load up  http://jsfiddle.net/rJ8x5/8/
-watch memory usage using Sysinternals Process Explorer or Windows Task Manager
-refresh the page

Expected Outcome:
-Stable memory usage

Visible Outcome:
-Memory continues to increase as long as page is open
-Refreshing the page does not free up all consumed memory
-this indicates both a pseudo-leak as well as an IE cross-page leak

Reproducibility:
-This is highly reproducible in IE8 as well as IE8 (compat) and IE7
-I am unable to test in IE6
-Not reproducible in Webkit/Mozilla

jQuery History:
-Using jsfiddle I was able to confirm that the leak exists on all major releases of jQuery, back to 1.3.2 (see note)
-I updated my original link to include actual DOM insertion  http://jsfiddle.net/rJ8x5/8/

note: The fact that this issue exists using old versions of jQuery is alarming to me. Either there is a fundamental problem with the way I am creating the inner closure, or there is a fundamental problem with jQuery. I am unable to see anything wrong with my code, however...

Last edited 4 years ago by smudgeface (previous) (diff)

comment:7 Changed 4 years ago by jitter

Thanks, I really appreciate your willingness to help in resolving this problem and the quick feedback when asked for more information. This saves us big amounts of time when debugging such issues.

After poking around a bit, can you try if the following code changes fixes the leak for you?

[...]
item.addListener = function(name,listener){
    var self = this;
    var wrappedListener = function(){
        return listener.apply(self,arguments);
    }
    this.elem.bind(name, wrappedListener);
    wrappedListener = null;
    self = null; // <-- this is the change
}
[...]

Your addListener code looks similar to this  IE closure leak pattern

comment:8 Changed 4 years ago by smudgeface

After playing around even further I think we can put some closure to this (pun intended).

My recent jsfiddle has completely removed jquery and gotten to the core of this problem. The memory leak is still exhibited and is likely a result of the wrappedListener holding scope to self/this. and is corrected by setting item.elem = null.

 http://jsfiddle.net/rJ8x5/34/

My confusion was caused by the fact that in my previous example ( http://jsfiddle.net/rJ8x5/8/) the memory leak did not occur even if I did not set item.elem = null, but only if I didn't call .remove(). Odd behavior...

Needless to say, I think this issue can be closed. Comments?

comment:9 Changed 4 years ago by jitter

  • Status changed from reopened to closed
  • Resolution set to invalid

Cool, closing (as invalid because not jQuery related).

Good to hear you were able to pin-point the issue and also could exclude jQuery from having something to do with this issue.

You might also want to look at  Understanding and Solving Internet Explorer Leak Patterns for further info on memory leak problems with IE.

Note: See TracTickets for help on using tickets.