Bug Tracker

Ticket #2935 (closed bug: fixed)

Opened 6 years ago

Last modified 6 years ago

Aborting $.ajax causes uncleared setInterval in $.ajax & ongoing CPU usage

Reported by: kuralj Owned by: flesler
Priority: major Milestone: 1.3
Component: ajax Version: 1.2.5
Keywords: Cc:
Blocking: Blocked by:

Description

Issue:

Aborting an ajax request results in $.ajax having an uncleared setInterval which causes ongoing CPU usage (jQuery 1.2.5).

Test code:

var myajax = $.ajax(...);
myajax.abort();

Details:

When using abort() on a returned xhr object from $.ajax(), the xhr will be successfully aborted, but the 'ival' setInterval for onreadystatechange in $.ajax will never be cleared because the onreadystatechange condition that contains the clearInterval will never be met now that the xhr has been aborted.

// from onreadystatechange in $.ajax
if ( !requestDone && xhr && (xhr.readyState == 4 || isTimeout == "timeout") ) {
	// this will never be reached after issueing myaxjax.abort()
	// and the clearInterval will never happen
}

The existence of the uncleared interval causes ongoing CPU usage and becomes magnified as more requests are aborted and their intervals left uncleared.

Suggested fix:

readyState will be is set to 0 when abort() is called. Add a check for readyState == 0 to onreadystatechange and clear the interval on this condition.

var onreadystatechange = function(isTimeout){
	if (xhr.readyState == 0) 
	{
		// xhr has been aborted, clear the interval
		if (ival) {
			clearInterval(ival);
			ival = null;
		}	
	}
	else if ( !requestDone && xhr && (xhr.readyState == 4 || isTimeout == "timeout") ) {
		// current existing code, including clear poll interval
	}

Attachments

ajax-abort.diff Download (1.4 KB) - added by joern 6 years ago.

Change History

comment:1 Changed 6 years ago by flesler

  • Owner set to flesler
  • Status changed from new to assigned

comment:2 Changed 6 years ago by ygirouard

I'm having an issue and I'm wondering if this could be related... Please confirm:

What I'm doing:

  1. I declare global ajaxStart and ajaxStop events as follow:
$().ajaxStart(function(){
 // Do something here
});
$().ajaxStop(function(){
 // Do something here
});
  1. I then have a function that does something like this:
function start_search(){
  var options = {
    (... ajax options here ...)
  }

  // Assigns the ajax return to a global variable
  g_last_xhr_request = $.ajax(options);
}
  1. I then bind the click event of a button to the following function:
function abort_last_request(){
  g_last_xhr_request.abort();
  // Do some other stuff...
}

Expected results:

It when I call the start_search() function, it should call the ajax, and therefore trigger the ajaxStart and ajaxStop callbacks. If while the request is still running I call the abort_last_request() function, the request should abort. If I then call the function start_search() again, it should still run and still trigger ajaxStart and ajaxStop.

What it's doing instead:

If I just keep calling the start_search() function without aborting it, the ajaxStart and ajaxStop events always trigger properly. However, if I abort it, and I try to run it again soon after, it runs the request, but never triggers the ajaxStart and ajaxStop events. Curiously however, it seems that if I just wait long enough after aborting, and try again, it eventually works again (something times out).

WOuld the problem above be the same one causing my issue?

comment:3 Changed 6 years ago by ygirouard

I did some debugging with firebug (using 1.2.6) and found not only it was related, but there was something else it didn't do right. When aborting a request, the jQuery.active counter was not decremented so this code:

// Watch for a new set of requests
  if ( s.global && jQuery.active++ )
    jQuery.event.trigger( "ajaxStart" );

Was never executed since jQuery.active was still set to 1 even after I aborted the last request.

So I changed a part of the code as follows:

// Wait for a response to come back
var onreadystatechange = function(isTimeout){

  // The request was aborted, clear the interval and decrement jQuery.active
  if (xhr.readyState == 0) {
    if (ival) {
      clearInterval(ival);
      ival = null;
      jQuery.active--;
    }
  }
  // The transfer is complete and the data is available, or the request timed out
  else if ( !requestDone && xhr && (xhr.readyState == 4 || isTimeout == "timeout") ) {
    requestDone = true;

    // clear poll interval
    if (ival) {
      clearInterval(ival);
      ival = null;
    }

    status = isTimeout == "timeout" && "timeout" ||
      !jQuery.httpSuccess( xhr ) && "error" ||
      s.ifModified && jQuery.httpNotModified( xhr, s.url ) && "notmodified" ||
      "success";

    if ( status == "success" ) {
      // Watch for, and catch, XML document parse errors
      try {
        // process the data (runs the xml through httpData regardless of callback)
        data = jQuery.httpData( xhr, s.dataType, s.dataFilter );
      } catch(e) {
        status = "parsererror";
      }
    }

    // Make sure that the request was successful or notmodified
    if ( status == "success" ) {
      // Cache Last-Modified header, if ifModified mode.
      var modRes;
      try {
        modRes = xhr.getResponseHeader("Last-Modified");
      } catch(e) {} // swallow exception thrown by FF if header is not available

      if ( s.ifModified && modRes )
        jQuery.lastModified[s.url] = modRes;

      // JSONP handles its own success callback
      if ( !jsonp )
        success();
    } else
      jQuery.handleError(s, xhr, status);

    // Fire the complete handlers
    complete();

    // Stop memory leaks
    if ( s.async )
      xhr = null;
  }
};

The part I added is:

  // The request was aborted, clear the interval and decrement jQuery.active
  if (xhr.readyState == 0) {
    if (ival) {
      clearInterval(ival);
      ival = null;
      jQuery.active--;
    }
  }

I've tested it as much as I could, and it seems to work fine now. Was there a reason why this wasn't done like this before?

comment:4 Changed 6 years ago by joern

Related to #2688, closed as duplicate. Attached patch handles both tickets.

Changed 6 years ago by joern

comment:5 Changed 6 years ago by tamlyn

Any intention of getting this patch into the core? It's a problem I've hit on twice in different projects.

comment:7 Changed 6 years ago by john

  • Status changed from assigned to closed
  • Resolution set to fixed

Committed in SVN rev [5944].

Note: See TracTickets for help on using tickets.