Skip to content

Commit

Permalink
Clean up DataFetchers even if DecodeJobs are cancelled prior to run().
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sjudd committed Sep 8, 2017
1 parent a365dab commit d482b8e
Showing 1 changed file with 9 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -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();
}
Expand Down

0 comments on commit d482b8e

Please sign in to comment.