From d482b8ed2b4551a35ba5edca16bc439f6242abf7 Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Fri, 8 Sep 2017 08:13:32 -0700 Subject: [PATCH] Clean up DataFetchers even if DecodeJobs are cancelled prior to run(). Previously some of the logic in run() might null out the data fetcher instance variable, even if it had been set when run() was first called. As a result, the data fetcher would not be cleared. Now the initial data fetcher is held on to as a local variable so the instance variable can be freed without affecting the behavior of cleanup(). Progress towards #1996, #2352 --- .../com/bumptech/glide/load/engine/DecodeJob.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/library/src/main/java/com/bumptech/glide/load/engine/DecodeJob.java b/library/src/main/java/com/bumptech/glide/load/engine/DecodeJob.java index 9f715bb454..d1eb3619b6 100644 --- a/library/src/main/java/com/bumptech/glide/load/engine/DecodeJob.java +++ b/library/src/main/java/com/bumptech/glide/load/engine/DecodeJob.java @@ -18,6 +18,7 @@ import com.bumptech.glide.load.engine.cache.DiskCache; import com.bumptech.glide.load.resource.bitmap.Downsampler; import com.bumptech.glide.util.LogTime; +import com.bumptech.glide.util.Preconditions; import com.bumptech.glide.util.Synthetic; import com.bumptech.glide.util.pool.FactoryPools.Poolable; import com.bumptech.glide.util.pool.StateVerifier; @@ -218,6 +219,9 @@ public void run() { // swallows all otherwise fatal exceptions, this will at least make it obvious to developers // that something is failing. TraceCompat.beginSection("DecodeJob#run"); + // Methods in the try statement can invalidate currentFetcher, so set a local variable here to + // ensure that the fetcher is cleaned up either way. + DataFetcher localFetcher = currentFetcher; try { if (isCancelled) { notifyFailed(); @@ -238,8 +242,11 @@ public void run() { throw e; } } finally { - if (currentFetcher != null) { - currentFetcher.cleanup(); + Preconditions.checkArgument( + localFetcher == null || currentFetcher == null || localFetcher.equals(currentFetcher), + "Fetchers don't match!, old: " + localFetcher + " new: " + currentFetcher); + if (localFetcher != null) { + localFetcher.cleanup(); } TraceCompat.endSection(); }