Skip to content

Commit

Permalink
Avoid IOExceptions in delete() when the cache dir is deleted externally.
Browse files Browse the repository at this point in the history
listFiles() will return null if the directory has been deleted. This 
scenario isn’t totally far fetched because it’s fairly common for 
Android developers to place their disk caches in the Android application 
cache directory. The cache directory can be cleared at any time by the 
system, including while the app is open, which in turn can cause 
unexpected IOExceptions when delete() is called.

See bumptech/glide#2465 for additional 
context.
  • Loading branch information
sjudd committed Nov 27, 2017
1 parent 3e01635 commit 14c9137
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/main/java/com/jakewharton/disklrucache/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ static String readFully(Reader reader) throws IOException {
* could not be deleted, or if {@code dir} is not a readable directory.
*/
static void deleteContents(File dir) throws IOException {
if (!dir.exists()) {
return;
}
File[] files = dir.listFiles();
if (files == null) {
throw new IOException("not a readable directory: " + dir);
Expand Down
37 changes: 37 additions & 0 deletions src/test/java/com/jakewharton/disklrucache/DiskLruCacheTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.io.FileReader;
import java.io.FileWriter;
import java.io.InputStream;
import java.io.IOException;
import java.io.Reader;
import java.io.StringWriter;
import java.io.Writer;
Expand Down Expand Up @@ -885,6 +886,42 @@ public final class DiskLruCacheTest {
assertThat(cache.get("a")).isNull();
}

@Test public void deleteAfterCacheDirectoryIsDeletedExternallyDoesNotThrow()
throws IOException {
FileUtils.deleteDirectory(cacheDir);
cache.delete();
}

@Test public void deleteWithNotReadableCacheDirectoryThowsIOException()
throws IOException {
try {
if (!cacheDir.setReadable(false)) {
fail("Unable to set writable on: " + cacheDir);
}
cache.delete();
fail("Expected delete to throw");
} catch (IOException e) {
// Expected.
} finally {
cacheDir.setReadable(true);
}
}

@Test public void deleteWithNotWritableCacheDirectoryThowsIOException()
throws IOException {
try {
if (!cacheDir.setWritable(false)) {
fail("Unable to set writable on: " + cacheDir);
}
cache.delete();
fail("Expected delete to throw");
} catch (IOException e) {
// Expected.
} finally {
cacheDir.setWritable(true);
}
}

private void assertJournalEquals(String... expectedBodyLines) throws Exception {
List<String> expectedLines = new ArrayList<String>();
expectedLines.add(MAGIC);
Expand Down

0 comments on commit 14c9137

Please sign in to comment.