Hi Hugh,
I am not sure if this directly bring out a new release. But I would be
very happy to fix this in the current branch.
Thank you for your investigation. If you can prepare a patch or send the
the modified files I will do my best to integrate them as soon as
possible into the SVN.
Thanks again for your help.
Sebastian
Hugh Gibson schrieb:
> 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
-------------------------------------------------------------------------
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
