Closures + XMLHttpRequest + Memory Leak

I'm aware of the circular reference memory leak problem with IE/closures.
I'm not sure exactly how to resolve it in this situation. Also, Firefox
appears to grow its memory size with the same code. So I'm wondering if I'm
missing something?

My test code is as follows:

function myObj() {
var req = new Object();
req.temp = 0;
if (window.XMLHttpRequest) { req.xmlHttpRequest = new XMLHttpRequest(); }
else if (window.ActiveXObject) { req.xmlHttpRequest = new
ActiveXObject("Msxml2.XMLHTTP"); }
req.xmlHttpRequest.onreadystatechange =
function() {
if (req.readyState==4) {
req.temp = req.xmlHttpRequest.responseText;
}
};
req.xmlHttpRequest.open("GET","/",true);
req.xmlHttpRequest.send(null);
return req;
}
// Create a whole bunch of these objects to check for memory leak
for (var i=0; i<1000; i++) {
var x = new myObj();
}

What is the best way to avoid memory leaking in this example?

--
Matt Kruse
http://www.JavascriptToolbox.com

Matt Kruse wrote:

I'm aware of the circular reference memory leak problem with IE/closures.
I'm not sure exactly how to resolve it in this situation. Also, Firefox
appears to grow its memory size with the same code. So I'm wondering if I'm
missing something?

My test code is as follows:

function myObj() {
var req = new Object();
req.temp = 0;
if (window.XMLHttpRequest) { req.xmlHttpRequest = new XMLHttpRequest(); }
else if (window.ActiveXObject) { req.xmlHttpRequest = new
ActiveXObject("Msxml2.XMLHTTP"); }
req.xmlHttpRequest.onreadystatechange =
function() {
if (req.readyState==4) {
req.temp = req.xmlHttpRequest.responseText;
}
};
req.xmlHttpRequest.open("GET","/",true);
req.xmlHttpRequest.send(null);
return req;
}
// Create a whole bunch of these objects to check for memory leak
for (var i=0; i<1000; i++) {
var x = new myObj();
}

What is the best way to avoid memory leaking in this example?

--
Matt Kruse
http://www.JavascriptToolbox.com

Not sure if this will solve your problem, but I noticed a few wasted
resources and unclear code.

First, when you say:

function myObj() {
var req = new Object();

You are actually instantiating two objects for every call to new myObj.

Second:

req.xmlHttpRequest.onreadystatechange =
function() {
if (req.readyState==4) {
req.temp = req.xmlHttpRequest.responseText;
}
};

For everything within the anonymous function, req should be 'this'.

This is how I would I would rewrite this:
function makeXMLRequest( URI ) {
var x = (
window.XMLHttpRequest ?
( new XMLHttpRequest() )
: ( new ActiveXObject( 'Msxml2.XMLHTTP' ) )
);

x.open( 'GET', encodeURI( URI || 'about:blank' ), true );
x.send( null );

return x;
}


Now a call to x = makeXMLRequest (NOT new makeXMLRequest) will return
an instance of either XMLHttpRequest OR a new ActiveXObject of type
'Msxml2.XMLHTTP'.

This should result in more efficient garbage collection as well.

The real problem (I think) is that you are creating all of these
objects and not necessarily giving them time to complete the HTTP
request. This is probably preventing the necessary garbage collection
and creating 1000 simultaneous HTTP requests. Ouch.

Hope that helps.

Random wrote:

First, when you say:
function myObj() {
var req = new Object();
You are actually instantiating two objects for every call to new
myObj.

This is a simplified example of my real case, which needs to do this. But
that's unrelated...

This is how I would I would rewrite this:

Well, that doesn't something entirely different. You can't really solve a
problem by rewriting it and removing functionality, can you?

Now a call to x = makeXMLRequest (NOT new makeXMLRequest) will return
an instance of either XMLHttpRequest OR a new ActiveXObject of type
'Msxml2.XMLHTTP'.
This should result in more efficient garbage collection as well.

Well that's because you've removed the handling function completely!

--
Matt Kruse
http://www.JavascriptToolbox.com

Matt Kruse wrote:

if (req.readyState==4) {

Oops, this is actually:

if (req.xmlHttpRequest.readyState==4) {

of course

--
Matt Kruse
http://www.JavascriptToolbox.com

Matt Kruse wrote:

I'm aware of the circular reference memory leak problem with
IE/closures. I'm not sure exactly how to resolve it in this
situation. Also, Firefox appears to grow its memory size with
the same code. So I'm wondering if I'm missing something?
snip
function myObj() {
var req = new Object();
req.temp = 0;
if (window.XMLHttpRequest) { req.xmlHttpRequest = new
XMLHttpRequest(); } else if (window.ActiveXObject) {
req.xmlHttpRequest = new ActiveXObject("Msxml2.XMLHTTP"); }
req.xmlHttpRequest.onreadystatechange =
function() {
if (req.readyState==4) {
req.temp = req.xmlHttpRequest.responseText;

req.xmlHttpRequest.onreadystatechange = null;

- At this point will remove the curricular reference and free the
closure.

}
};
req.xmlHttpRequest.open("GET","/",true);
req.xmlHttpRequest.send(null);
return req;
}
snip
What is the best way to avoid memory leaking in this example?

Belt and braces would have you nulling the reference to the
XMLHttpRequest object once it is finished with.

Richard.

Richard Cornford wrote:
<snip>

- At this point will remove the curricular reference ...
snip> ^^^^^^^^^^

That should have been 'circular'.

Richard.

Richard Cornford wrote:

req.xmlHttpRequest.onreadystatechange = null;
- At this point will remove the curricular reference and free the
closure.

That's what I thought too, but no luck. IE says "type mismatch".

Instead, I found that this appears to work:

delete req.xmlHttpRequest['onreadystatechange'];

And in fact, I'm doing this just to be safe:

delete req.xmlHttpRequest['onreadystatechange'];
req.xmlHttpRequest = null;
req = null;
CollectGarbage();

That *appears* to work in IE. I can see the memory usage grow to over 150MB,
then drop back to 30MB, so I assume the leak is gone. However, I don't know
of any way to test for sure. Do you?

--
Matt Kruse
http://www.JavascriptToolbox.com

Matt Kruse wrote:

Random wrote:
First, when you say:
function myObj() {
var req = new Object();
You are actually instantiating two objects for every call to new
myObj.

This is a simplified example of my real case, which needs to do this. But
that's unrelated...

Clearly I misunderstood. Sorry I didn't catch that based on your
original post.

This is how I would I would rewrite this:

Well, that doesn't something entirely different. You can't really solve a
problem by rewriting it and removing functionality, can you?

See above.

Now a call to x = makeXMLRequest (NOT new makeXMLRequest) will return
an instance of either XMLHttpRequest OR a new ActiveXObject of type
'Msxml2.XMLHTTP'.
This should result in more efficient garbage collection as well.

Well that's because you've removed the handling function completely!

See above.

Matt Kruse wrote:

Richard Cornford wrote:
req.xmlHttpRequest.onreadystatechange = null;
- At this point will remove the curricular reference and
free the closure.

That's what I thought too, but no luck. IE says
"type mismatch".

Yes, you are right. It looks like I went for assigning a reference to a
small harmless (and not 'inner') function to clear the closure in my
version.

<snip>

And in fact, I'm doing this just to be safe:

delete req.xmlHttpRequest['onreadystatechange'];
req.xmlHttpRequest = null;
req = null;
CollectGarbage();
snip

Calling CollectGarbage without verifying its existence will result in
errors on browsers that do not support it.

Richard.

i ussing xmlhttprequest for few years. Typicaly i use sinchronus data
trander

example of my uses

senddata(method,data,debug,url)
debug = shows respondense content

myResult = sendData('update',{id:1,name:34});

after this call i get myResult javascript array (or string)

for asyncronus can be added callback function