diff --git a/src/main/java/com/android/volley/ExecutorDelivery.java b/src/main/java/com/android/volley/ExecutorDelivery.java
index 1babfcd1..8c6e0940 100644
--- a/src/main/java/com/android/volley/ExecutorDelivery.java
+++ b/src/main/java/com/android/volley/ExecutorDelivery.java
@@ -88,31 +88,7 @@ public ResponseDeliveryRunnable(Request request, Response response, Runnable run
@SuppressWarnings("unchecked")
@Override
public void run() {
- // If this request has canceled, finish it and don't deliver.
- if (mRequest.isCanceled()) {
- mRequest.finish("canceled-at-delivery");
- return;
- }
-
- // Deliver a normal response or error, depending.
- if (mResponse.isSuccess()) {
- mRequest.deliverResponse(mResponse.result);
- } else {
- mRequest.deliverError(mResponse.error);
- }
-
- // If this is an intermediate response, add a marker, otherwise we're done
- // and the request can be finished.
- if (mResponse.intermediate) {
- mRequest.addMarker("intermediate-response");
- } else {
- mRequest.finish("done");
- }
-
- // If we have been provided a post-delivery runnable, run it.
- if (mRunnable != null) {
- mRunnable.run();
- }
+ mRequest.deliverResponse(mResponse, mRunnable);
}
}
}
diff --git a/src/main/java/com/android/volley/Request.java b/src/main/java/com/android/volley/Request.java
index a98277eb..c4d52516 100644
--- a/src/main/java/com/android/volley/Request.java
+++ b/src/main/java/com/android/volley/Request.java
@@ -56,6 +56,12 @@ public interface Method {
int PATCH = 7;
}
+ /** Callback interface to notify listeners when a request has been canceled. */
+ public interface CancelListener {
+ /** Called when a request was canceled before the response could be delivered. */
+ void onRequestCanceled();
+ }
+
/**
* Callback to notify when the network request returns.
*/
@@ -83,8 +89,11 @@ public interface Method {
/** 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;
+ private Response.ErrorListener mErrorListener;
/** Sequence number of this request, used to enforce FIFO ordering. */
private Integer mSequence;
@@ -96,9 +105,11 @@ public interface Method {
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,10 +129,11 @@ public interface Method {
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();
+ /** Optional listener for request cancel events. */
+ private CancelListener mCancelListener;
/**
* Creates a new request with the given URL and error listener. Note that
@@ -184,6 +196,18 @@ public Response.ErrorListener getErrorListener() {
return mErrorListener;
}
+ /**
+ * @return this request's {@link CancelListener}, if any.
+ */
+ public CancelListener getCancelListener() {
+ return mCancelListener;
+ }
+
+ /** Sets a {@link CancelListener} which will be notified when the request is canceled. */
+ public void setCancelListener(CancelListener listener) {
+ mCancelListener = listener;
+ }
+
/**
* @return A tag for use with {@link TrafficStats#setThreadStatsTag(int)}
*/
@@ -320,17 +344,81 @@ public Cache.Entry getCacheEntry() {
}
/**
- * Mark this request as canceled. No callback will be delivered.
+ * Mark this request as canceled.
+ *
+ *
No callback will be delivered as long as {@link ExecutorDelivery} is used as the
+ * {@link ResponseDelivery} and as long as this method is not overridden. There are no
+ * guarantees otherwise.
+ *
+ *
Subclasses should generally override {@link #onCanceled()} instead to perform an action
+ * when the request is canceled (such as clearing response listeners to avoid leaks).
*/
public void cancel() {
- mCanceled = true;
+ synchronized (mLock) {
+ mCanceled = true;
+ onCanceledInternal();
+ }
}
/**
* Returns true if this request has been canceled.
*/
public boolean isCanceled() {
- return mCanceled;
+ synchronized (mLock) {
+ return mCanceled;
+ }
+ }
+
+ private void onCanceledInternal() {
+ mErrorListener = null;
+ if (mCancelListener != null) {
+ mCancelListener.onRequestCanceled();
+ mCancelListener = null;
+ }
+ onCanceled();
+ }
+
+ /**
+ * Called when a request is canceled.
+ *
+ *
The default implementation does nothing. Subclasses may override this to perform custom
+ * actions, such as clearing response listeners to avoid leaks.
+ */
+ protected void onCanceled() {
+ }
+
+ /** Deliver the given response as long as the request isn't canceled. */
+ void deliverResponse(Response response, Runnable runnable) {
+ // To uphold the contract of cancel(), we hold the same lock here around both the check of
+ // isCanceled() as well as the actual response delivery. This ensures that after a call
+ // to cancel() returns, we will never deliver the response.
+ synchronized (mLock) {
+ // If this request has canceled, finish it and don't deliver.
+ if (isCanceled()) {
+ finish("canceled-at-delivery");
+ return;
+ }
+
+ // Deliver a normal response or error, depending.
+ if (response.isSuccess()) {
+ deliverResponse(response.result);
+ } else {
+ deliverError(response.error);
+ }
+
+ // If this is an intermediate response, add a marker, otherwise we're done
+ // and the request can be finished.
+ if (response.intermediate) {
+ addMarker("intermediate-response");
+ } else {
+ finish("done");
+ }
+
+ // If we have been provided a post-delivery runnable, run it.
+ if (runnable != null) {
+ runnable.run();
+ }
+ }
}
/**
diff --git a/src/main/java/com/android/volley/toolbox/ImageRequest.java b/src/main/java/com/android/volley/toolbox/ImageRequest.java
index 0f33cd8c..7ef77851 100644
--- a/src/main/java/com/android/volley/toolbox/ImageRequest.java
+++ b/src/main/java/com/android/volley/toolbox/ImageRequest.java
@@ -42,7 +42,7 @@ public class ImageRequest extends Request {
/** Default backoff multiplier for image requests */
public static final float DEFAULT_IMAGE_BACKOFF_MULT = 2f;
- private final Response.Listener mListener;
+ private Response.Listener mListener;
private final Config mDecodeConfig;
private final int mMaxWidth;
private final int mMaxHeight;
@@ -221,6 +221,11 @@ protected void deliverResponse(Bitmap response) {
}
}
+ @Override
+ protected void onCanceled() {
+ mListener = null;
+ }
+
/**
* Returns the largest power-of-two divisor for use in downscaling a bitmap
* that will not result in the scaling past the desired dimensions.
diff --git a/src/main/java/com/android/volley/toolbox/JsonRequest.java b/src/main/java/com/android/volley/toolbox/JsonRequest.java
index 40877b11..66c5d7f7 100644
--- a/src/main/java/com/android/volley/toolbox/JsonRequest.java
+++ b/src/main/java/com/android/volley/toolbox/JsonRequest.java
@@ -39,7 +39,7 @@ public abstract class JsonRequest extends Request {
private static final String PROTOCOL_CONTENT_TYPE =
String.format("application/json; charset=%s", PROTOCOL_CHARSET);
- private final Listener mListener;
+ private Listener mListener;
private final String mRequestBody;
/**
@@ -68,6 +68,11 @@ protected void deliverResponse(T response) {
}
}
+ @Override
+ protected void onCanceled() {
+ mListener = null;
+ }
+
@Override
abstract protected Response parseNetworkResponse(NetworkResponse response);
diff --git a/src/main/java/com/android/volley/toolbox/RequestFuture.java b/src/main/java/com/android/volley/toolbox/RequestFuture.java
index 173c44cc..adf9407f 100644
--- a/src/main/java/com/android/volley/toolbox/RequestFuture.java
+++ b/src/main/java/com/android/volley/toolbox/RequestFuture.java
@@ -20,6 +20,7 @@
import com.android.volley.Response;
import com.android.volley.VolleyError;
+import java.util.concurrent.CancellationException;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
@@ -35,8 +36,8 @@
*
* // If you want to be able to cancel the request:
* future.setRequest(requestQueue.add(request));
+ * // Also be sure not to call Request.setCancelListener.
*
- * // Otherwise:
* requestQueue.add(request);
*
* try {
@@ -51,9 +52,10 @@
*
* @param The type of parsed response this future expects.
*/
-public class RequestFuture implements Future, Response.Listener,
- Response.ErrorListener {
+public class RequestFuture implements Future, Response.Listener, Response.ErrorListener,
+ Request.CancelListener {
private Request mRequest;
+ private boolean mRequestCanceled = false;
private boolean mResultReceived = false;
private T mResult;
private VolleyError mException;
@@ -66,6 +68,11 @@ private RequestFuture() {}
public void setRequest(Request request) {
mRequest = request;
+ if (mRequest.getCancelListener() != null) {
+ throw new IllegalArgumentException("Provided request already has a cancel listener");
+ }
+ mRequest.setCancelListener(this);
+ // Not much we can do if a different cancel listener is set after this point.
}
@Override
@@ -107,6 +114,10 @@ private synchronized T doGet(Long timeoutMs)
return mResult;
}
+ if (isCancelled()) {
+ throw new CancellationException();
+ }
+
if (timeoutMs == null) {
wait(0);
} else if (timeoutMs > 0) {
@@ -121,6 +132,10 @@ private synchronized T doGet(Long timeoutMs)
throw new TimeoutException();
}
+ if (isCancelled()) {
+ throw new CancellationException();
+ }
+
return mResult;
}
@@ -129,7 +144,7 @@ public boolean isCancelled() {
if (mRequest == null) {
return false;
}
- return mRequest.isCanceled();
+ return mRequestCanceled;
}
@Override
@@ -149,5 +164,11 @@ public synchronized void onErrorResponse(VolleyError error) {
mException = error;
notifyAll();
}
+
+ @Override
+ public synchronized void onRequestCanceled() {
+ mRequestCanceled = true;
+ notifyAll();
+ }
}
diff --git a/src/main/java/com/android/volley/toolbox/StringRequest.java b/src/main/java/com/android/volley/toolbox/StringRequest.java
index 05a62f60..0d46772b 100644
--- a/src/main/java/com/android/volley/toolbox/StringRequest.java
+++ b/src/main/java/com/android/volley/toolbox/StringRequest.java
@@ -28,7 +28,7 @@
* A canned request for retrieving the response body at a given URL as a String.
*/
public class StringRequest extends Request {
- private final Listener mListener;
+ private Listener mListener;
/**
* Creates a new request with the given method.
@@ -62,6 +62,11 @@ protected void deliverResponse(String response) {
}
}
+ @Override
+ protected void onCanceled() {
+ mListener = null;
+ }
+
@Override
protected Response parseNetworkResponse(NetworkResponse response) {
String parsed;
diff --git a/src/test/java/com/android/volley/RequestTest.java b/src/test/java/com/android/volley/RequestTest.java
index d5beca5f..84436522 100644
--- a/src/test/java/com/android/volley/RequestTest.java
+++ b/src/test/java/com/android/volley/RequestTest.java
@@ -21,6 +21,8 @@
import org.junit.runner.RunWith;
import org.robolectric.RobolectricTestRunner;
+import java.util.concurrent.atomic.AtomicBoolean;
+
import static org.junit.Assert.*;
@RunWith(RobolectricTestRunner.class)
@@ -45,6 +47,24 @@ public class RequestTest {
assertTrue(immediate.compareTo(high) < 0);
}
+ @Test public void cancelListener() {
+ TestRequest request = new TestRequest(Priority.NORMAL);
+ final AtomicBoolean listenerCalled = new AtomicBoolean(false);
+ request.setCancelListener(new Request.CancelListener() {
+ @Override
+ public void onRequestCanceled() {
+ listenerCalled.set(true);
+ }
+ });
+ assertFalse(request.isCanceled());
+ assertFalse(listenerCalled.get());
+
+ request.cancel();
+
+ assertTrue(request.isCanceled());
+ assertTrue(listenerCalled.get());
+ }
+
private class TestRequest extends Request