From a690f38859ebf5f4f06172060596842a35cc16d8 Mon Sep 17 00:00:00 2001
From: Sam Judd <judds@google.com>
Date: Thu, 8 Feb 2018 07:29:49 -0800
Subject: [PATCH] Handle notifications in MultiFetcher after cancellation.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We can’t guarantee that every fetcher implementation will strictly avoid
notifying after they’ve been cancelled.

This also improves the behavior in VolleyStreamFetcher so that it attempts to avoid notifying after cancel, although it still doesn’t make
any strict guarantee.

Fixes #2879
---
 .../volley/VolleyStreamFetcher.java           |  8 +++--
 .../glide/load/model/MultiModelLoader.java    | 30 ++++++++++++++-----
 2 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/integration/volley/src/main/java/com/bumptech/glide/integration/volley/VolleyStreamFetcher.java b/integration/volley/src/main/java/com/bumptech/glide/integration/volley/VolleyStreamFetcher.java
index 97348ecc6e..3137fdcdbe 100644
--- a/integration/volley/src/main/java/com/bumptech/glide/integration/volley/VolleyStreamFetcher.java
+++ b/integration/volley/src/main/java/com/bumptech/glide/integration/volley/VolleyStreamFetcher.java
@@ -133,13 +133,17 @@ protected VolleyError parseNetworkError(VolleyError volleyError) {
       if (Log.isLoggable(TAG, Log.DEBUG)) {
         Log.d(TAG, "Volley failed to retrieve response", volleyError);
       }
-      callback.onLoadFailed(volleyError);
+      if (!isCanceled()) {
+        callback.onLoadFailed(volleyError);
+      }
       return super.parseNetworkError(volleyError);
     }
 
     @Override
     protected Response<byte[]> parseNetworkResponse(NetworkResponse response) {
-      callback.onDataReady(new ByteArrayInputStream(response.data));
+      if (!isCanceled()) {
+        callback.onDataReady(new ByteArrayInputStream(response.data));
+      }
       return Response.success(response.data, HttpHeaderParser.parseCacheHeaders(response));
     }
 
diff --git a/library/src/main/java/com/bumptech/glide/load/model/MultiModelLoader.java b/library/src/main/java/com/bumptech/glide/load/model/MultiModelLoader.java
index 443304fccb..fb9d7206a8 100644
--- a/library/src/main/java/com/bumptech/glide/load/model/MultiModelLoader.java
+++ b/library/src/main/java/com/bumptech/glide/load/model/MultiModelLoader.java
@@ -52,7 +52,7 @@ public LoadData<Data> buildLoadData(@NonNull Model model, int width, int height,
         }
       }
     }
-    return !fetchers.isEmpty()
+    return !fetchers.isEmpty() && sourceKey != null
         ? new LoadData<>(sourceKey, new MultiFetcher<>(fetchers, exceptionListPool)) : null;
   }
 
@@ -80,8 +80,10 @@ static class MultiFetcher<Data> implements DataFetcher<Data>, DataCallback<Data>
     private DataCallback<? super Data> callback;
     @Nullable
     private List<Throwable> exceptions;
+    private boolean isCancelled;
 
-    MultiFetcher(@NonNull List<DataFetcher<Data>> fetchers,
+    MultiFetcher(
+        @NonNull List<DataFetcher<Data>> fetchers,
         @NonNull Pool<List<Throwable>> throwableListPool) {
       this.throwableListPool = throwableListPool;
       Preconditions.checkNotEmpty(fetchers);
@@ -90,7 +92,8 @@ static class MultiFetcher<Data> implements DataFetcher<Data>, DataCallback<Data>
     }
 
     @Override
-    public void loadData(@NonNull Priority priority, @NonNull DataCallback<? super Data> callback) {
+    public synchronized void loadData(
+        @NonNull Priority priority, @NonNull DataCallback<? super Data> callback) {
       this.priority = priority;
       this.callback = callback;
       exceptions = throwableListPool.acquire();
@@ -98,7 +101,7 @@ public void loadData(@NonNull Priority priority, @NonNull DataCallback<? super D
     }
 
     @Override
-    public void cleanup() {
+    public synchronized void cleanup() {
       if (exceptions != null) {
         throwableListPool.release(exceptions);
       }
@@ -109,7 +112,8 @@ public void cleanup() {
     }
 
     @Override
-    public void cancel() {
+    public synchronized void cancel() {
+      isCancelled = true;
       for (DataFetcher<Data> fetcher : fetchers) {
         fetcher.cancel();
       }
@@ -128,7 +132,11 @@ public DataSource getDataSource() {
     }
 
     @Override
-    public void onDataReady(@Nullable Data data) {
+    public synchronized void onDataReady(@Nullable Data data) {
+      if (isCancelled) {
+        return;
+      }
+
       if (data != null) {
         callback.onDataReady(data);
       } else {
@@ -137,12 +145,20 @@ public void onDataReady(@Nullable Data data) {
     }
 
     @Override
-    public void onLoadFailed(@NonNull Exception e) {
+    public synchronized void onLoadFailed(@NonNull Exception e) {
+      if (isCancelled) {
+        return;
+      }
+
       Preconditions.checkNotNull(exceptions).add(e);
       startNextOrFail();
     }
 
     private void startNextOrFail() {
+      if (isCancelled) {
+        return;
+      }
+
       if (currentIndex < fetchers.size() - 1) {
         currentIndex++;
         loadData(priority, callback);