[FIXr]xmlhttprequest fails when called for 2nd time from readystatehandler

RESOLVED FIXED in mozilla1.8beta1

Status

()

Core
XML
P1
normal
RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: Lars Hofhansl, Assigned: bz)

Tracking

Trunk
mozilla1.8beta1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

14 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.2) Gecko/20040803
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.2) Gecko/20040803

When xmlhttprequest is used from a readystatehandler of the *same* xmlhttp
object, the second call fails.
There's no error message and no exception, there server trip is performed, but
the readystatehandler is never called for the 2nd time.

This is more common than you'd expect, as it would occur for client side event
queuing, when upon finishing of one request there may be new event in the
outgoing queue that'd have to be sent.

This problem obly occurs when the same xmlhttp object is used, and the call is
made "directly" from the readystatehandler (with setTimeout this is problem can
be worked around).
However, if a new xmlhttp object is created every time, the browser leaks memory.

Reproducible: Always
Steps to Reproduce:
client code (xmlhttpbug.html):

<html>
<head>
<script>
var httpReq = {};
// if this is placed inside the sendXMLHTTP function (i.e. the object is
recreated every time, it works
if (window.ActiveXObject)
    httpReq = new ActiveXObject("Microsoft.XMLHTTP");
else
    httpReq = new XMLHttpRequest();

function sendXMLHTTP() {

    httpReq.open("POST", "server.jsp", true, null, null);  
    httpReq.onreadystatechange = receive
    httpReq.send("Some Test String");
}

var n;
function receive() {
  if( httpReq.readyState != 4 ) return;
  alert( httpReq.responseText + " n:"+ n  );
  if( --n>0 ) sendXMLHTTP();
  // this way it works... if( --n ) setTimeout( "sendXMLHTTP();", 0 );
}

</script>
</head>
<body>
<input type=button onclick="n=10;sendXMLHTTP()" value="SendXMLHTTP">
</body>
</html>

Server echoing JSP (server.jsp):
<%@ page language="java" import="java.io.Reader" %>
<%
    System.out.println( "Request..." );
    // just echo the request
    Reader is = request.getReader();
    char buf[] = new char[1024];
    int n;
    while( (n=is.read( buf )) != -1 )
        out.write( buf, 0, n ); 

    out.flush();
%>

1. place both xmlhttpbug.html and server.jsp in the same directory of JSP
capable web container.
2. open xmlhttpbug.html, click on the "SendXMLHTTP" button
3. 

Actual Results:  
The browser displays the alert box only once, indicating that the second call to
the readystatehandler is never made (however the servertrip does occur).

Expected Results:  
The alert box should be displayed 10 times, showing that every request made it
to the server, and calls the readystatehandler upon completion.

This is the result in IE aswell.


This seems to be a timing issue between executing the readystatehandler, calling
the open method and setting the readystatehandler. It seems the request is not
considered done until after the readystatehandler returns (even when readystate
== 4 "completed").

For correct behavior a new xmlhttp object has to be created every time in which
case the browser leaks memory.

Tried with Mozilla 1.7.2 on Windows 2k and Linux.
The problem here is that when the load completes we fire the ready state event
and then clear all listeners.  In this case, that means we clear the listener
the new sendXMLHTTP() just set.

I think the right course of action here is to clear the listeners before making
the ChangeState() call and to pass the listener to the call (making it default
to mOnReadystatechangeListener if nothing is passed).
(Reporter)

Comment 2

14 years ago
Yes, that'd explains the problem.
Here's a perhaps interesting twist:

If I put 'alert( "sent" );' at the end of sendXMLHTTP (after the
httpReq.send()), it works fine (i.e. all 10 alerts in the receive functions are
displayed).
Comment on attachment 158483 [details] [diff] [review]
Possible approach

One problem with this approach is that in the JS event handler the
xmlhttprequest object will have null onprogress, onload, onerror, and
onreadystatechange properties.	Will that break existing content?  If so, we
need a cleverer approach...
Attachment #158483 - Flags: superreview?(jst)
Attachment #158483 - Flags: review?(doronr)
(Reporter)

Comment 5

14 years ago
(In reply to comment #4)

According to the MSFT documentation the onreadystatechange property is
write-only, probably for this very problem.
Comment on attachment 158483 [details] [diff] [review]
Possible approach

Looks reasonable to me, it's hard to know if this breaks something, but it
seems unlikely to me. I'd be fine with testing this out on the trunk...
Attachment #158483 - Flags: superreview?(jst) → superreview+
(Reporter)

Comment 7

14 years ago
I tested the version that re-creates a new object on every request again with
1.8a3. The memory leak is gone, hence there's less reason to fix this problem.

The fix looks good to me, and does not violate the spec (assuming the MSFT
XMLHTTP spec is what we strive for). It would also be good to make all handlers
(error, load, readystatechange) write-only for JS code.

I can test again with a nightly.
Severity: major → normal

Comment 8

14 years ago
Comment on attachment 158483 [details] [diff] [review]
Possible approach

looks fine, shouldn't break  the web, though if it does, we should hear about
it quickly.
Attachment #158483 - Flags: review?(doronr) → review+
Taking.  I guess I'll land this once the tree reopens...
Assignee: hjtoi-bugzilla → bzbarsky
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: xmlhttprequest fails when called for 2nd time from readystatehandler → [FIXr]xmlhttprequest fails when called for 2nd time from readystatehandler
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta
Fixed on trunk for 1.8a5.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
*** Bug 280859 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.