Closed Bug 258768 Opened 21 years ago Closed 21 years ago

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

Categories

(Core :: XML, defect, P1)

Product:
Core
Component:
XML
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.8beta1

People

(Reporter: lhofhansl, Assigned: bzbarsky)

References

Details

Attachments

(1 file)

Possible approach
5.88 KB, patch
doronr
: review+
jst
: superreview+
Details | Diff | Splinter Review
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).
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)
(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+
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 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
Closed: 21 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.

Attachment

General

Creator:
Created:
Updated:
Size: