diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/test/ExternallyClearedDiskCacheTest.java b/instrumentation/src/androidTest/java/com/bumptech/glide/test/ExternallyClearedDiskCacheTest.java new file mode 100644 index 0000000000..34f9b4a79e --- /dev/null +++ b/instrumentation/src/androidTest/java/com/bumptech/glide/test/ExternallyClearedDiskCacheTest.java @@ -0,0 +1,124 @@ +package com.bumptech.glide.test; + +import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Mockito.mock; + +import android.content.Context; +import android.graphics.drawable.Drawable; +import android.support.test.InstrumentationRegistry; +import android.support.test.runner.AndroidJUnit4; +import com.bumptech.glide.Glide; +import com.bumptech.glide.GlideBuilder; +import com.bumptech.glide.load.Key; +import com.bumptech.glide.load.engine.cache.DiskCache; +import com.bumptech.glide.load.engine.cache.DiskCache.Factory; +import com.bumptech.glide.load.engine.cache.DiskLruCacheWrapper; +import com.bumptech.glide.test.ResourceIds.raw; +import java.io.File; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; + +// Tests #2465. +@RunWith(AndroidJUnit4.class) +public class ExternallyClearedDiskCacheTest { + @Rule public final TearDownGlide tearDownGlide = new TearDownGlide(); + private final ConcurrencyHelper concurrency = new ConcurrencyHelper(); + private Context context; + private File cacheDir; + + @Before + public void setUp() { + context = InstrumentationRegistry.getTargetContext(); + cacheDir = context.getCacheDir(); + } + + @After + public void tearDown() { + // Force us to wait until Glide's threads shut down. + Glide.tearDown(); + deleteRecursively(cacheDir); + } + + @Test + public void clearDiskCache_afterOpeningDiskCache_andDeleteDirectoryOutsideGlide_doesNotThrow() { + DiskCache cache = DiskLruCacheWrapper.create(cacheDir, 1024 * 1024); + cache.get(mock(Key.class)); + deleteRecursively(cacheDir); + cache.clear(); + } + + @Test + public void get_afterDeleteDirectoryOutsideGlideAndClose_doesNotThrow() { + DiskCache cache = DiskLruCacheWrapper.create(cacheDir, 1024 * 1024); + cache.get(mock(Key.class)); + deleteRecursively(cacheDir); + cache.clear(); + + cache.get(mock(Key.class)); + } + + @Test + public void loadFromCache_afterDiskCacheDeletedAndCleared_doesNotFail() { + final DiskCache cache = DiskLruCacheWrapper.create(cacheDir, 1024 * 1024); + cache.get(mock(Key.class)); + deleteRecursively(cacheDir); + cache.clear(); + + Glide.init( + context, + new GlideBuilder() + .setDiskCache(new Factory() { + @Override + public DiskCache build() { + return cache; + } + })); + + Drawable drawable = + concurrency.get( + Glide.with(context) + .load(ResourceIds.raw.canonical) + .submit()); + assertThat(drawable).isNotNull(); + } + + @Test + public void loadFromCache_afterDiskCacheDeleted_doesNotFail() { + final DiskCache cache = DiskLruCacheWrapper.create(cacheDir, 1024 * 1024); + cache.get(mock(Key.class)); + deleteRecursively(cacheDir); + + Glide.init( + context, + new GlideBuilder() + .setDiskCache(new Factory() { + @Override + public DiskCache build() { + return cache; + } + })); + + Drawable drawable = + concurrency.get(Glide.with(context) + .load(raw.canonical) + .submit()); + assertThat(drawable).isNotNull(); + } + + private static void deleteRecursively(File file) { + if (file.isDirectory()) { + File[] files = file.listFiles(); + if (files != null) { + for (File f : files) { + deleteRecursively(f); + } + } + } + if (!file.delete() && file.exists()) { + throw new RuntimeException("Failed to delete: " + file); + } + } +} diff --git a/library/src/main/java/com/bumptech/glide/load/engine/cache/DiskLruCacheWrapper.java b/library/src/main/java/com/bumptech/glide/load/engine/cache/DiskLruCacheWrapper.java index a877d47606..364f842892 100644 --- a/library/src/main/java/com/bumptech/glide/load/engine/cache/DiskLruCacheWrapper.java +++ b/library/src/main/java/com/bumptech/glide/load/engine/cache/DiskLruCacheWrapper.java @@ -163,11 +163,15 @@ public void delete(Key key) { public synchronized void clear() { try { getDiskCache().delete(); - resetDiskCache(); } catch (IOException e) { if (Log.isLoggable(TAG, Log.WARN)) { - Log.w(TAG, "Unable to clear disk cache", e); + Log.w(TAG, "Unable to clear disk cache or disk cache cleared externally", e); } + } finally { + // Delete can close the cache but still throw. If we don't null out the disk cache here, every + // subsequent request will try to act on a closed disk cache and fail. By nulling out the disk + // cache we at least allow for attempts to open the cache in the future. See #2465. + resetDiskCache(); } } diff --git a/library/src/test/java/com/bumptech/glide/load/engine/cache/DiskLruCacheWrapperTest.java b/library/src/test/java/com/bumptech/glide/load/engine/cache/DiskLruCacheWrapperTest.java index de91ea572d..cb60eae946 100644 --- a/library/src/test/java/com/bumptech/glide/load/engine/cache/DiskLruCacheWrapperTest.java +++ b/library/src/test/java/com/bumptech/glide/load/engine/cache/DiskLruCacheWrapperTest.java @@ -3,7 +3,9 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; +import com.bumptech.glide.load.Key; import com.bumptech.glide.signature.ObjectKey; import com.bumptech.glide.tests.Util; import java.io.File; @@ -41,21 +43,17 @@ public void tearDown() { } } - private void deleteRecursive(File file) { - if (!file.isDirectory()) { - if (!file.delete()) { - throw new IllegalStateException("Failed to delete: " + file); + private static void deleteRecursive(File file) { + if (file.isDirectory()) { + File[] files = file.listFiles(); + if (files != null) { + for (File f : files) { + deleteRecursive(f); + } } - return; - } - - File[] files = file.listFiles(); - if (files == null) { - return; } - - for (File child : files) { - deleteRecursive(child); + if (!file.delete() && file.exists()) { + throw new RuntimeException("Failed to delete: " + file); } } @@ -136,4 +134,24 @@ public boolean write(File file) { assertArrayEquals(data, received); } + + // Tests #2465. + @Test + public void clearDiskCache_afterOpeningDiskCache_andDeleteDirectoryOutsideGlide_doesNotThrow() { + DiskCache cache = DiskLruCacheWrapper.create(dir, 1024 * 1024); + cache.get(mock(Key.class)); + deleteRecursive(dir); + cache.clear(); + } + + // Tests #2465. + @Test + public void get_afterDeleteDirectoryOutsideGlideAndClose_doesNotThrow() { + DiskCache cache = DiskLruCacheWrapper.create(dir, 1024 * 1024); + cache.get(mock(Key.class)); + deleteRecursive(dir); + cache.clear(); + + cache.get(mock(Key.class)); + } }