Sebastian/Andreas E, I would strongly suggest that this is a very good
reason to bring forward 0.7.3.
I think you should look at your testing procedures as something as
fundamental as this shouldn't be allowed to slip through the net for so
long.
> The problem is in this code in the destructor in XmlHttp.js:
Actually qx.io.remote.XmlHttpTransport.js
> It seems like someone somewhere got an exception when setting
> onreadystatechange to null. Unfortunately, not setting it to null
> leaks memory in IE. I've tested it, and I never received an
> exception (but could easily see the leak).
0.5 has code similar to that in 0.7 (I just happened to download all of
SVN yesterday, including all the tags and branches...)
0.1.5 has this code:
if (this.req)
{
// Next line of code was supposed to fix a IE memory leak.
Unfortunately,
// it lets IE consume more and more memory by each request.
// this.req.onreadystatechange = null;
// this seems to be ok
this.req = null;
};
this.__onreadystatechange = null;
Note that __onreadystatechange is the closure.
> I would propose to fix it like this:
Apart from the syntax error - (Exception e) should be just (e) - that
works but it doesn't seem to be quite correct.
See
http://groups.google.com/group/Google-Web-Toolkit-Contributors/browse_thre
ad/thread/7e7ee67c191a6324
"Log:
Fixes issue #1406; there was a memory leak on IE due to very subtle
nuances of XHR on IE. Turns out the only legal way to "unhook" an
onreadystatechange handler is to set it to a dummy func!"
That issue is re-opened later at
http://groups.google.com/group/Google-Web-Toolkit/browse_thread/thread/c8a
4bf841a9c2638/ab68898bb352985c?#ab68898bb352985c
The new problems there may not be specific for onreadystatechange though.
See issue 1406 at
http://code.google.com/p/google-web-toolkit/issues/detail?id=1406&start=50
0
See also http://www.phpclasses.org/browse/file/17742.html -
dummy: function() {}, // empty constant function for ActiveX leak
minimization
later:
// Avoid memory leak by removing closure.
req.onreadystatechange = th.dummy;
So I suggest that the correct code is to set it to a dummy func. Also,
that implies that any exception doesn't happen immediately but later on,
when onreadystatechange is called when it shouldn't be. It should be
straightforward to fix this.
In looking into this, I found the hack fix for qooxdoo issue 190 (at
http://bugzilla.qooxdoo.org/show_bug.cgi?id=190 ) where the
onreadystatechange function is called on a timeout. But look at the
constructor:
construct : function()
{
this.base(arguments);
this._req = qx.io.remote.XmlHttpTransport.createRequestObject();
this._req.onreadystatechange =
qx.lang.Function.bind(this._onreadystatechange, this);
},
and then later in send() this._onreadystatechange is bound again:
var onreadyStateChangeCallback =
qx.lang.Function.bind(this._onreadystatechange, this);
if (qx.core.Variant.isSet("qx.client", "mshtml") &&
this.getAsynchronous())
{
vRequest.onreadystatechange = function(e)
{
var self = this;
window.setTimeout(function(e) {
onreadyStateChangeCallback(e);
}, 0);
};
}
else
{
vRequest.onreadystatechange = onreadyStateChangeCallback
}
So onreadystatechange shouldn't be set up in the constructor. I've tested
this change in IE7 and FF and it is OK. Obviously more testing in all the
browsers is required.
Is it possible that setting onreadystatechange to a dummy function will
clean up issue 190 and mean that you don't have to use a timeout call for
onreadystatechange in IE? This would be in line with the requirement in
IE to set onreadystatechange to a dummy function - that implies that IE
can call onreadystatechange after the XMLHttpRequest object is finished
with by JS code.
Further testing is needed here.
Hugh
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
qooxdoo-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/qooxdoo-devel
