Skip to content

Commit

Permalink
Fix race condition in XHR and handle other abort/open scenarios
Browse files Browse the repository at this point in the history
This fixes issue #3630
A short summary of the changes:
* Use generation id to cancel inflight requests
* Handles nested calls to abort, open, send inside handlers
* Adds XHRReleaseMsg to delay freeing XHR object till all
  inflight events are processed
* Change the ErroredMsg enum to be more symmetric with the returned
  Error enum
  • Loading branch information
mukilan committed Nov 3, 2014
1 parent 1a3ff87 commit 7435db2
Show file tree
Hide file tree
Showing 3 changed files with 240 additions and 133 deletions.
7 changes: 5 additions & 2 deletions components/script/dom/dedicatedworkerglobalscope.rs
Expand Up @@ -19,7 +19,7 @@ use dom::workerglobalscope::DedicatedGlobalScope;
use dom::workerglobalscope::{WorkerGlobalScope, WorkerGlobalScopeHelpers};
use dom::xmlhttprequest::XMLHttpRequest;
use script_task::{ScriptTask, ScriptChan};
use script_task::{ScriptMsg, FromWorker, DOMMessage, FireTimerMsg, XHRProgressMsg, WorkerRelease};
use script_task::{ScriptMsg, FromWorker, DOMMessage, FireTimerMsg, XHRProgressMsg, XHRReleaseMsg, WorkerRelease};
use script_task::WorkerPostMessage;
use script_task::StackRootTLS;

Expand Down Expand Up @@ -134,7 +134,10 @@ impl DedicatedWorkerGlobalScope {
global.delayed_release_worker();
},
Ok(XHRProgressMsg(addr, progress)) => {
XMLHttpRequest::handle_xhr_progress(addr, progress)
XMLHttpRequest::handle_progress(addr, progress)
},
Ok(XHRReleaseMsg(addr)) => {
XMLHttpRequest::handle_release(addr)
},
Ok(WorkerPostMessage(addr, data, nbytes)) => {
Worker::handle_message(addr, data, nbytes);
Expand Down

4 comments on commit 7435db2

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from jdm
at mukilan@7435db2

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging mukilan/servo/xhr-issue-3630 = 7435db2 into auto

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mukilan/servo/xhr-issue-3630 = 7435db2 merged ok, testing candidate = 096452c

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.