Bug 106937 - Cannot abort multiple XHR POSTs made to same url
Summary: Cannot abort multiple XHR POSTs made to same url
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nate Chapin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-15 12:34 PST by Chris Cinelli
Modified: 2013-01-18 10:26 PST (History)
6 users (show)

See Also:


Attachments
patch (4.57 KB, patch)
2013-01-16 16:14 PST, Nate Chapin
ap: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Fix ews failure (5.54 KB, patch)
2013-01-17 14:05 PST, Nate Chapin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cinelli 2013-01-15 12:34:46 PST
Chrome Version       : Version 22.0.1229.92 m
URLs (if applicable) :
Other browsers tested:
 Safari 6.0.1: FAIL
  Firefox  15: OK
         IE 9: OK

What steps will reproduce the problem?
1. Make multiple XHR Post requests to the same URL
2. Note that the payload should be unique
3. Note that each XHR request is made
4. try aborting them (before they complete), note that only the last one is actually aborted

Workaround: make the url unique by adding a query string

What is the expected result?
Each XHR call should be aborted

What happens instead?
Only the last XHR call can be successfully aborted

We had problems when one of our logging server was taking longer than expected to answer. All the logging requests from the frontend were logged to the same URL. When the jQuery ajax requested timed out, the XHR was not aborted and remained pending. It did not take too long that the browser run out of connections and was not able to send any more requests. 

Code sample:

// Code sample instructions:
1) navigate to http://posttestserver.com 
       + this prevents access denied in IE testing
2) Open developer console
3) Ensure that network logging is enabled
4) Run the code below in the console
5) Note that 3 requests are made, only the last is canceled
6) Run it again, this time with workAroundOn = true
7) Note that all 3 requests are made and all three are canceled

// start code sample
var workAroundOn = false,
    baseUrl = 'http://posttestserver.com/post.php?sleep=10';

for (var requestNumber in [0,1,2]){
    (function( requestNumber  ){

	var xhr = new XMLHttpRequest()
            postUrl = baseUrl;

        // if the url is unique, everything works as expected
        if (workAroundOn){
            postUrl = postUrl + '&r=' + requestNumber;
        }

        // open the async post request
        xhr.open("POST", postUrl, true);   

        // send the request with a unique payload
        //    if the payload is not different, chrome only sends one post...
        xhr.send(requestNumber); 

	console.log('request made, requestNumber:'+requestNumber,xhr);

        // abort the call after other calls have been made
        setTimeout(function(){
            console.log('abort upload, requestNumber:'+requestNumber,xhr);
            xhr.abort();
        },2500);

    }( requestNumber ));
}



See http://code.google.com/p/chromium/issues/detail?id=154643
Comment 1 Alexey Proskuryakov 2013-01-15 22:27:24 PST
Confirmed with trunk.

What I'm seeing is that XHR.abort() gets down to CachedResource::removeClient(). For first two requests, inCache() is false, so we don't call allClientsRemoved(), and thus don't actually cancel ResourceHandle. Only the third request has inCache() return true, and gets actually canceled.

Nate, is this something you'd be willing to look into? To me, it just looks like allClientsRemoved() call should be made regardless of whether the resource is in cache, but perhaps there was a reason to make it conditional.
Comment 2 Nate Chapin 2013-01-16 11:06:02 PST
(In reply to comment #1)
> Confirmed with trunk.
> 
> What I'm seeing is that XHR.abort() gets down to CachedResource::removeClient(). For first two requests, inCache() is false, so we don't call allClientsRemoved(), and thus don't actually cancel ResourceHandle. Only the third request has inCache() return true, and gets actually canceled.
> 
> Nate, is this something you'd be willing to look into? To me, it just looks like allClientsRemoved() call should be made regardless of whether the resource is in cache, but perhaps there was a reason to make it conditional.

I agree, it seems obvious that allClientsRemoved() should be called even if the resource is in cache, though it's not immediately obvious whether that holds true for everything else in the if (!deleted && !hasClients() && inCache()) block.

I'll look into it.
Comment 3 Nate Chapin 2013-01-16 16:14:03 PST
Created attachment 183055 [details]
patch
Comment 4 WebKit Review Bot 2013-01-16 16:50:20 PST
Comment on attachment 183055 [details]
patch

Attachment 183055 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15901745

New failing tests:
http/tests/inspector/network/network-disable-cache-xhrs.html
Comment 5 Nate Chapin 2013-01-17 14:05:08 PST
Created attachment 183274 [details]
Fix ews failure

Not convinced this is the best fix to the network-disable-cache-xhrs.html failure, but it's simple at least. :/
Comment 6 WebKit Review Bot 2013-01-18 10:26:43 PST
Comment on attachment 183274 [details]
Fix ews failure

Clearing flags on attachment: 183274

Committed r140174: <http://trac.webkit.org/changeset/140174>
Comment 7 WebKit Review Bot 2013-01-18 10:26:47 PST
All reviewed patches have been landed.  Closing bug.