From e7e7620f794b62fc53ba43705a73f715a9393df5 Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Tue, 24 Mar 2015 07:29:12 -0700 Subject: [PATCH] Avoid concurrent modification in lifecycle impl. Fixes #375. --- .../manager/ActivityFragmentLifecycle.java | 8 ++++--- .../glide/manager/RequestTracker.java | 22 +++++-------------- .../java/com/bumptech/glide/util/Util.java | 21 +++++++++++++++++- 3 files changed, 30 insertions(+), 21 deletions(-) diff --git a/library/src/main/java/com/bumptech/glide/manager/ActivityFragmentLifecycle.java b/library/src/main/java/com/bumptech/glide/manager/ActivityFragmentLifecycle.java index f91270cd2..940438f0e 100644 --- a/library/src/main/java/com/bumptech/glide/manager/ActivityFragmentLifecycle.java +++ b/library/src/main/java/com/bumptech/glide/manager/ActivityFragmentLifecycle.java @@ -1,5 +1,7 @@ package com.bumptech.glide.manager; +import com.bumptech.glide.util.Util; + import java.util.Collections; import java.util.Set; import java.util.WeakHashMap; @@ -44,21 +46,21 @@ class ActivityFragmentLifecycle implements Lifecycle { void onStart() { isStarted = true; - for (LifecycleListener lifecycleListener : lifecycleListeners) { + for (LifecycleListener lifecycleListener : Util.getSnapshot(lifecycleListeners)) { lifecycleListener.onStart(); } } void onStop() { isStarted = false; - for (LifecycleListener lifecycleListener : lifecycleListeners) { + for (LifecycleListener lifecycleListener : Util.getSnapshot(lifecycleListeners)) { lifecycleListener.onStop(); } } void onDestroy() { isDestroyed = true; - for (LifecycleListener lifecycleListener : lifecycleListeners) { + for (LifecycleListener lifecycleListener : Util.getSnapshot(lifecycleListeners)) { lifecycleListener.onDestroy(); } } diff --git a/library/src/main/java/com/bumptech/glide/manager/RequestTracker.java b/library/src/main/java/com/bumptech/glide/manager/RequestTracker.java index 621579025..0ade50531 100644 --- a/library/src/main/java/com/bumptech/glide/manager/RequestTracker.java +++ b/library/src/main/java/com/bumptech/glide/manager/RequestTracker.java @@ -1,6 +1,7 @@ package com.bumptech.glide.manager; import com.bumptech.glide.request.Request; +import com.bumptech.glide.util.Util; import java.util.ArrayList; import java.util.Collections; @@ -64,7 +65,7 @@ public class RequestTracker { */ public void pauseRequests() { isPaused = true; - for (Request request : getSnapshot()) { + for (Request request : Util.getSnapshot(requests)) { if (request.isRunning()) { request.pause(); pendingRequests.add(request); @@ -77,7 +78,7 @@ public class RequestTracker { */ public void resumeRequests() { isPaused = false; - for (Request request : getSnapshot()) { + for (Request request : Util.getSnapshot(requests)) { if (!request.isComplete() && !request.isCancelled() && !request.isRunning()) { request.begin(); } @@ -89,7 +90,7 @@ public class RequestTracker { * Cancels all requests and clears their resources. */ public void clearRequests() { - for (Request request : getSnapshot()) { + for (Request request : Util.getSnapshot(requests)) { request.clear(); } pendingRequests.clear(); @@ -99,7 +100,7 @@ public class RequestTracker { * Restarts failed requests and cancels and restarts in progress requests. */ public void restartRequests() { - for (Request request : getSnapshot()) { + for (Request request : Util.getSnapshot(requests)) { if (!request.isComplete() && !request.isCancelled()) { // Ensure the request will be restarted in onResume. request.pause(); @@ -111,17 +112,4 @@ public class RequestTracker { } } } - - // Avoids a ConcurrentModificationException when requests are started by another request completing. See #303. - private List getSnapshot() { - // toArray creates a new ArrayList internally and this way we can guarantee entries will not be - // null. See #322. - List result = new ArrayList(requests.size()); - // We could also just call new ArrayList(requests) but that actually creates two new ArrayLists because - // that constructor in ArrayList calls toArray(). - for (Request request : requests) { - result.add(request); - } - return result; - } } diff --git a/library/src/main/java/com/bumptech/glide/util/Util.java b/library/src/main/java/com/bumptech/glide/util/Util.java index b2dff176b..5ea55ac20 100644 --- a/library/src/main/java/com/bumptech/glide/util/Util.java +++ b/library/src/main/java/com/bumptech/glide/util/Util.java @@ -8,6 +8,9 @@ import android.os.Looper; import com.bumptech.glide.request.target.Target; import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; import java.util.Queue; /** @@ -157,9 +160,25 @@ public final class Util { } /** - * Creates a {@link java.util.Queue} of the given size using Glide's preferred implementation. + * Returns a {@link java.util.Queue} of the given size using Glide's preferred implementation. */ public static Queue createQueue(int size) { return new ArrayDeque(size); } + + /** + * Returns a copy of the given list that is safe to iterate over and perform actions that may + * modify the original list. + * + *

See #303 and #375.

+ */ + public static List getSnapshot(Collection other) { + // toArray creates a new ArrayList internally and this way we can guarantee entries will not + // be null. See #322. + List result = new ArrayList(other.size()); + for (T item : other) { + result.add(item); + } + return result; + } } -- GitLab