...

View Full Version : IE DOM memory leak strikes again ...



brothercake
04-02-2005, 12:24 AM
I've been using my encapsulated anonymous functions (http://www.codingforums.com/showthread.php?t=47379) technique more and more, and I think it exposes another memory leak in IE6 .. but cleaning it up is not so simple..

Here's how it goes - this construct leaks a small amount of memory:


window.onload = function()
{
var body = document.body;

document.body.onmousemove = function()
{
document.title = Math.random();
};
};

We can clean this (and all similar examples) up by iterating through document.all and setting the relevant expando property to null; in this case it's a mousemove:


window.attachEvent('onunload', function()
{
var dlen = document.all.length;
for(var i=0; i<dlen; i++)
{
document.all[i]['onmousemove'] = null;
}
});

Okay, so far so good. But if the original handler is written as an encapsulated anonymous function instead of an expando property, like this:


body.attachEvent('onmousemove', function()
{
document.title = Math.random();
});

then it's not exposed through document.all, and therefore isn't cleaned in the unload method. I discovered eventually that detachEvent works, except of course that it requires a reference to the function, which means this:


body.attachEvent('onmousemove', attachment = function()
{
document.title = Math.random();
});

And then this:


body.detachEvent('onmousemove', attachment);

And that works, but it's impractical. What I really want is another way to find these handlers and their function references, without having to save them in advance.

So is there any object or collection this information might reside in? Since it's only for IE6, any ridiculous proprietary arcana would be an acceptible solution.

liorean
04-03-2005, 04:07 PM
The best solution is AFAIK to make sure you never encapsulate a DOM reference in a closure.



1. In iew one solution is to try to always use functions declared in the global scope as event handlers.

2. Another trick is to set all variables that did contain DOM references to null before the outer function has closed.

3. A third trick is to store IDs instead of DOM references where possible and to dynamically look them up when needed; or using an array of DOM references in the global scope which you store the index for locally to the same effect.

4. A fourth trick is separation of scopes by using separate closures for all DOM references and event handlers - in other words using inner functions for everything except the event handler itself.



As long as you can manage the single thing: No object or closure that is a property or event handler of a DOM object may contain DOM references, with the exception of when that reference is in the global scope.

Kor
04-04-2005, 09:01 AM
See also this article (http://www.bazon.net/mishoo/articles.epl?art_id=824) to go deep into the problem and find some interesting solutions

brothercake
04-04-2005, 09:42 AM
But all of these solutions ultimately boil down to reducing the use of closures, and I'm not prepared to pollute the global scope with more functions, I want my entire script inside a single object.

Is there no way I can clean this data without changing my coding style? What about your idea No. 2 - would that do it - if I nullified the whole object, would that clean up their inner closures?

Kor
04-04-2005, 10:13 AM
boil down to reducing the use of closures,


Not exactly... To my surprise, Mishoo (if you saw his article (http://www.bazon.net/mishoo/articles.epl?art_id=824) ) found a tricky solution by using a "nested/hidden" closure whithin another closure.... This way the handler have no direct acces to tha variable, so that the closure is not an active closure. It looks quite clever to me...

liorean
04-04-2005, 05:11 PM
But all of these solutions ultimately boil down to reducing the use of closures, and I'm not prepared to pollute the global scope with more functions, I want my entire script inside a single object. Well, either you do 1 or 3 and remove closures, or you do point 4 and add closures. Or you do 2 and simply remove references. A smart compiler would also have shadowing variables as an alternative for 2, but I wouldn't expect JScript 5 to be that smart.

Remember that closures mean shared scopes in which identifiers are looked up in, not shared values - you don't need to do anything to the actual values, only the identifier binding. This is what all these tricks try to attack.

Another possibility that I just thought of that might work, is to use a non-variable to store the data - the arguments vector could be usable for that.


Is there no way I can clean this data without changing my coding style? What about your idea No. 2 - would that do it - if I nullified the whole object, would that clean up their inner closures?Set the variables that contain references to any kind of JScript native and the closure no longer references a non-GC object. The trick is, as I said, to use whichever way you feel comfortable with to make sure you never close over a scope that contains a DOM reference. Clearing all DOM references out of the scope is one way to solve the problem. Making sure all DOM references are out of scope is what the three other tricks try to do.

brothercake
04-04-2005, 05:15 PM
Yes I did see that, wrapping the closures in another anonymous function, but that only avoids the memory leak by making the method contents hidden to the containing object ... but then there isn't a reference to it, so how can it be re-used by the script? Or am I missing something?

I have seen something by Mark Wubben that would do the job - http://novemberborn.net/javascript/event-cache - it's a sort of DOM reference controller that cleans up by having you passing all event constructors through it. It does essentially the same as my original construct, but designed for function references, and with this additional cleaning benefit.

But ... I still can't see how that's any easier than assigning manual references myself as I build the functions - it wouldn't work as it is for anonymous functions because the cleaner requires a string reference to the function, and adapting it would be more complex than just making that one adaption to the approach I've already got ...

What's the standard model? Is a handler added using addEventListener reference-able from any other interface beside the assignment that created it?

brothercake
04-04-2005, 05:28 PM
Set the variables that contain references to any kind of JScript native and the closure no longer references a non-GC object. The trick is, as I said, to use whichever way you feel comfortable with to make sure you never close over a scope that contains a DOM reference. Clearing all DOM references out of the scope is one way to solve the problem. Making sure all DOM references are out of scope is what the three other tricks try to do.
I don't understand that ... how can I ever avoid closing over a scope? Surely that would mean that method calls within OO constructors can't pass arguments? ie - aren't I limited to going


this.obj.onmouseover = this.mouseover;

without ever being to pass arguments?

liorean
04-04-2005, 06:17 PM
I don't understand that ... how can I ever avoid closing over a scope?You can't, if you want to nest functions. The important part was the "that contains a DOM reference".

IE users COM objects for it's internals, and those have one garbage collector. JScript uses native JScript objects and has it's own mark-and-sweep garbage collector. The problem is that the JScript GC and the COM GC don't share values, and will not communicate to get around circular references. Mozilla has four different types of garbage collectors, and quite a lot of code written by a GC unaware person for the application will be leaky, as well as some window/frame crossing code for documents. They solve this by manually communicating for application and by using the same GC for JavaScript and for DOM for document. I don't know how Lars and Jens solves this for op, but I know both konq/saf leaks even worse than iew.
Surely that would mean that method calls within OO constructors can't pass arguments? ie - aren't I limited to going


this.obj.onmouseover = this.mouseover;

without ever being to pass arguments?Not really, you just have to make sure that the arguments, if they are references, are either not formal parameters (and thus not stored in variables) or are cleared at some time.

Something you can do is to create a cleansing function in each outer function, which will nullify all the variables in the scope. Add this function as listener on the document's beforeunload event and there you go, manual clearing of circular references when it should take place.


Yes I did see that, wrapping the closures in another anonymous function, but that only avoids the memory leak by making the method contents hidden to the containing object ... but then there isn't a reference to it, so how can it be re-used by the script? Or am I missing something?If your purpose for creating the closure is just to have that reference, then it doesn't help one bit. But if you don't need the reference in the inner function it works perfectly fine.

What's the standard model? Is a handler added using addEventListener reference-able from any other interface beside the assignment that created it?Nope, there's no such inspection facility. But it's still subject for the same possibly leaky garbage collection problem. And the fix is still the same.

DHTML Kitchen
04-13-2005, 11:13 PM
This is very interesting, though it does bother me. If I have a DOM object that is referenced by a JS object, is a memory leak caused? If so, then why is a memory leak not caused by creating a variable that points to an DOM object?

If the above is true, e.g. if memory leak is created by storing a DOM object in a js object instance, then I must use document.getElementById in the instance's methods, which could be slow for mousemove operations or heavier mouseover/out operations.

Does this simple class suffer any memory leaks? What if I were to use this.el = el; ? Would that create a memory leak?
Should I instead store a reference to the element's ID in the object and use a callback in the handler? (as I have done below).


// Button class -------------------------------------------------
Button = function(el) {

if(el.hideFocus)
el.hideFocus = true;
this.id = el.id;
var div = el.getElementsByTagName("div")[0];
this.divID = (div.id || (div.id = el.id + "Div"));

addListener(el, "click", Button.toggle );
addListener(el, "mouseover", Button.mouseover );
addListener(el, "mouseout", Button.mouseout );
};

Button.instances = { };

Button.getInstance = function(el) {
return Button.instances[el.id] || (Button.instances[el.id] = new Button(el));
};

Button.prototype = {

isDepressed : false,

isDisabled : false,

ondepress : function(e) { },

onrelease : function(e) { },

toggle : function(e) {

var el = document.getElementById(this.id);
if(this.isDisabled) return;

if(el.blur)
el.blur();

if(this.isDepressed) {
removeClass(document.getElementById(this.divID), "button-active");
this.isDepressed = false;
this.onrelease(e);
}
else {
document.getElementById(this.divID).className += " button-active";
this.isDepressed = true;
this.ondepress(e)
}
},

mouseover : function(e) {
if(!this.isDisabled && !this.isDepressed)
document.getElementById(this.divID).className += " button-hover";
},

mouseout : function(e) {
if(!this.isDisabled && !this.isDepressed)
removeClass(document.getElementById(this.divID), "button-hover");
},

setEnabled : function(bEnable) {
if(this.isDisabled == !bEnable) return;

if(this.isDisabled)
removeClass(document.getElementById(this.id), "disabled");
else
document.getElementById(this.id).className += " disabled";

this.isDisabled = !bEnable;
}

};

Button.mouseover = function() { Button.getInstance(this).mouseover(); };
Button.mouseout = function() { Button.getInstance(this).mouseout(); };
Button.toggle = function() { Button.getInstance(this).toggle(); };

DHTML Kitchen
04-13-2005, 11:18 PM
I would much rather have used a closure, like this, right in the constructor:


var ths = this;
addListener(el, "click", function(e) { ths.toggle(e); } );


That would solve scope issues and keep it clean and simple. I like using closures this way.

Will it cause a mem leak? If so, how so?

Thank you,

Garrett

DHTML Kitchen
04-19-2005, 07:28 PM
^^^

anyone??

brothercake
04-19-2005, 07:34 PM
My instinct is to say yes - that would cause a memory leak - but I must admit I don't fully understand this whole concept, and ... well ...

There's come a point where the line has to be drawn - this far, no farther. All of this is a bug in IE, so I'll still add these cleaning functions to clear the exapando handlers they can, but I can't be bothered to care about it any more than that .. so IE has leaks .. so does Safari and Firefox (though over different things). Lots of APIs have leaks, and it's not the responsibility of individual application developers to jump through hoops trying to cater for them.

JavaScript is an interpreted language - you shouldn't have to think about garbage collection in an interpreted language; period.


Sorry .. bit grumpy I know. But I've so had enough of catering to IE's wilful non-compliance ... it just never ends ...

DHTML Kitchen
04-19-2005, 11:18 PM
Hey man,

yeah F. IE , I know. Personally I've been wishing MS'd stop making it since years ago.

When you say expando handlers, do you mean assigned event handlers, as in
span.onclick = handleClick; ?

Have you done any testing?

Is that causing any memory leak? Does it need to be explicitly cleared by span.onclick = null? We some test cases for that so we can see what to avoid -- and what we don't have to avoid -- so we can understand the situation clearer.

Kor
04-20-2005, 08:50 AM
I guess that
span.onclick = handleClick;
will not cause a lack of memory, but
span.onclick = function(){handleClick(this)}
is a closure, as the object refereres to itself in a sort of self loop.

brothercake
04-20-2005, 01:47 PM
My understanding is that a closure is where an anonymous function is inside another anonymous function, and so the interpretor can't determine whether the inner function can be garbage collection, because it's private to an anoymous function and therefore unreferenceable.

I think that's it anyway. Here's the simplest structure that causes the problem:


element.onevent = function()
{
this.onevent = function() { }
}

The cleaning method we're currently using works on any event which is bound as an expando - an "expando" is an event-handling property of an element, so we can always reference it as element.onevent or element['onevent'] and thereby have it garbage collected by setting it to null

The problem is that functions added using attachEvent still leak in the same circumstances, but can't be cleaned the same way because they don't show up in the document.all (or indeed, any) collection

So this construct leaks but can't be cleaned:


element.onevent = function()
{
element.attachEvent('onevent', function() { });
}

To clean that we need to use detachEvent, which requires a reference to the function, which we don't have. We could go like this:


var inner;
element.onevent = function()
{
element.attachEvent('onevent', inner = function() { });
}

And then remove it onunload like this:


element.detachEvent('onevent', inner);

But therein is the beginning of the tediousness - in a complex script it just becomes ridiculous, the amount of management you have to do just to eradicate what is fundamentally not your problem.

Now I think that what liorean is saying in his explanation is that these handlers could all be cleaned by stepping back up, to the original element.onevent that surrounds it, and setting that to null ... but I don't understand that - surely the original DOM cleaner has already done that .. and the leak remains?

Maybe it could be cleaned by going urther up - to the original master object - nullifying all of them. Is that possible - can you identify instantions of an object, or would that require a global to store a set of references as they're constructed? And would that work anyway?

I'd really to find a solution, but it has to be a simple one like the onunload DOM cleaner in the original example - something you can just plug in without having to think about it as you write the script. I mean, there's nothing wrong with this style of coding, is there? I evolved this style because it's cleaner, more manageable and well-encapsulated, and easier to degrade gracefully.

codegoboom
04-20-2005, 11:40 PM
As if this hasn't yet been attempted... what about:



Now, you can force the JScript garbage collector to run with the CollectGarbage() method, but I don't recommend it.


I'm not actually paying close attention... ;)

brothercake
04-22-2005, 10:20 AM
Oh well .. if he doesn't recommend it then it's probably the right solution .. coming as it does, from the guy who originally tried to claim that retaining the memory used by closures was a feature of JScript :rolleyes:

I shall investigate..

brothercake
04-22-2005, 10:33 AM
Doesn't seem to do anything though ... I tried it as an onunload, and I tried running it continually on a timeout, but it doesn't stop the leak

codegoboom
04-22-2005, 11:53 AM
Oh well, maybe that's why it's undocumented... :D

DHTML Kitchen
04-29-2005, 01:54 AM
Edit note: the link to the test is working again!

I made a Listener class and tested the performance of it in IE. The test is very heavy, but there does not appear to be a memory leak. I actually suspected there would be a leak and had planned a workaround, but it appears that it is not necessary.

Warning: this is a CPU-heavy test! Do not blame me if it crashes your machine!

http://dhtmlkitchen.com/experiment/listener/test-perf.html

Steps to test:
1) quit browser
2) launch activity monitor/process viewer

3) note mem and cpu and mem usage and in process viewer
4) navigate to test page -> http://dhtmlkitchen.com/experiment/listener/test-perf.html
5) note mem and cpu and mem usage and in process viewer
6) navigate to another page (ebay|google|yahoo|msn), wait for 1 second after the page finishes loading.
7) repeat steps 3-6 repeatedly.

You'll notice that CPUs get really high when the page is loading. Mem usage does not increase drastically, even after several repeats.

Is this a good test?

===

I don't like making browser specific workarounds (IE. They're always for IE). Anyway, there's one really nice thing about the Listener class: It can be used with custom objects.

It's similar Aaron's listener (which is great), but it doesn't use eval and doesn't use Aaron's Function.prototype.apply. I acheived the scope trick by using a closure, and this is why I thought it would leak memory. The garbage collector seems to be working.

DHTML Kitchen
04-29-2005, 06:45 PM
Fixed the link to the test case mentioned in my previous post. :thumbsup:

brothercake
04-30-2005, 01:14 AM
It's a good test, but it fails - I'm losing about 7MB every time (in IE6, XP Home SP2)

What was your test environment?

DHTML Kitchen
04-30-2005, 02:12 AM
Win2k Professional, here.

Here's a testcase with a cleanup method.
http://dhtmlkitchen.com/experiment/listener/test-perf2.html


// Unbind listeners.
Listener.cleanUp = function () {
for(var type in Listeners) {
var bucket = Listeners[type];
for(var i = bucket.length-1; i >= 0; i--)
bucket[i][type] = null;
}
if(window.CollectGarbage)
window.CollectGarbage();
};

Well, I hope this should take care of all refs. I think forcing the GC on unload is useful for larger apps. Maybe better to count the number of listeners and invoke the GC if it's large (maybe more than 20).



EZ Archive Ads Plugin for vBulletin Copyright 2006 Computer Help Forum