1 files changed, 50 insertions, 17 deletions

diff --git a/src/main/java/com/android/volley/Request.java b/src/main/java/com/android/volley/Request.java
index a98277e..358a327 100644
--- a/src/main/java/com/android/volley/Request.java
+++ b/src/main/java/com/android/volley/Request.java
@@ -83,8 +83,12 @@ public abstract class Request<T> implements Comparable<Request<T>> {
     /** Default tag for {@link TrafficStats}. */
     private final int mDefaultTrafficStatsTag;
 
+    /** Lock to guard state which can be mutated after a request is added to the queue. */
+    private final Object mLock = new Object();
+
     /** Listener interface for errors. */
-    private final Response.ErrorListener mErrorListener;
+    // @GuardedBy("mLock")
+    private Response.ErrorListener mErrorListener;
 
     /** Sequence number of this request, used to enforce FIFO ordering. */
     private Integer mSequence;
@@ -96,9 +100,11 @@ public abstract class Request<T> implements Comparable<Request<T>> {
     private boolean mShouldCache = true;
 
     /** Whether or not this request has been canceled. */
+    // @GuardedBy("mLock")
     private boolean mCanceled = false;
 
     /** Whether or not a response has been delivered for this request yet. */
+    // @GuardedBy("mLock")
     private boolean mResponseDelivered = false;
 
     /** Whether the request should be retried in the event of an HTTP 5xx (server) error. */
@@ -118,11 +124,9 @@ public abstract class Request<T> implements Comparable<Request<T>> {
     private Object mTag;
 
     /** Listener that will be notified when a response has been delivered. */
+    // @GuardedBy("mLock")
     private NetworkRequestCompleteListener mRequestCompleteListener;
 
-    /** Object to guard access to mRequestCompleteListener. */
-    private final Object mLock = new Object();
-
     /**
      * Creates a new request with the given URL and error listener.  Note that
      * the normal response listener is not provided here as delivery of responses
@@ -320,17 +324,34 @@ public abstract class Request<T> implements Comparable<Request<T>> {
     }
 
     /**
-     * Mark this request as canceled.  No callback will be delivered.
+     * Mark this request as canceled.
+     *
+     * <p>No callback will be delivered as long as either:
+     * <ul>
+     *     <li>This method is called on the same thread as the {@link ResponseDelivery} is running
+     *     on. By default, this is the main thread.
+     *     <li>The request subclass being used overrides cancel() and ensures that it does not
+     *     invoke the listener in {@link #deliverResponse} after cancel() has been called in a
+     *     thread-safe manner.
+     * </ul>
+     *
+     * <p>There are no guarantees if both of these conditions aren't met.
      */
+    // @CallSuper
     public void cancel() {
-        mCanceled = true;
+        synchronized (mLock) {
+            mCanceled = true;
+            mErrorListener = null;
+        }
     }
 
     /**
      * Returns true if this request has been canceled.
      */
     public boolean isCanceled() {
-        return mCanceled;
+        synchronized (mLock) {
+            return mCanceled;
+        }
     }
 
     /**
@@ -549,14 +570,18 @@ public abstract class Request<T> implements Comparable<Request<T>> {
      * later in the request's lifetime for suppressing identical responses.
      */
     public void markDelivered() {
-        mResponseDelivered = true;
+        synchronized (mLock) {
+            mResponseDelivered = true;
+        }
     }
 
     /**
      * Returns true if this request has had a response delivered for it.
      */
     public boolean hasHadResponseDelivered() {
-        return mResponseDelivered;
+        synchronized (mLock) {
+            return mResponseDelivered;
+        }
     }
 
     /**
@@ -597,8 +622,12 @@ public abstract class Request<T> implements Comparable<Request<T>> {
      * @param error Error details
      */
     public void deliverError(VolleyError error) {
-        if (mErrorListener != null) {
-            mErrorListener.onErrorResponse(error);
+        Response.ErrorListener listener;
+        synchronized (mLock) {
+            listener = mErrorListener;
+        }
+        if (listener != null) {
+            listener.onErrorResponse(error);
         }
     }
 
@@ -619,10 +648,12 @@ public abstract class Request<T> implements Comparable<Request<T>> {
      * @param response received from the network
      */
     /* package */ void notifyListenerResponseReceived(Response<?> response) {
+        NetworkRequestCompleteListener listener;
         synchronized (mLock) {
-            if (mRequestCompleteListener != null) {
-               mRequestCompleteListener.onResponseReceived(this, response);
-            }
+            listener = mRequestCompleteListener;
+        }
+        if (listener != null) {
+            listener.onResponseReceived(this, response);
         }
     }
 
@@ -631,10 +662,12 @@ public abstract class Request<T> implements Comparable<Request<T>> {
      * a response which can be used for other, waiting requests.
      */
     /* package */ void notifyListenerResponseNotUsable() {
+        NetworkRequestCompleteListener listener;
         synchronized (mLock) {
-            if (mRequestCompleteListener != null) {
-                mRequestCompleteListener.onNoUsableResponseReceived(this);
-            }
+            listener = mRequestCompleteListener;
+        }
+        if (listener != null) {
+            listener.onNoUsableResponseReceived(this);
         }
     }