From 99f2bafe1c0abb2791131dde5a702d3bb3ba28f3 Mon Sep 17 00:00:00 2001 From: Neil Fuller Date: Mon, 12 Jan 2015 12:01:30 +0000 Subject: [PATCH] Fix isClosed() when a cache has not been initialized isClosed() now means "has close() been called", after commit ea565b2e30e15cd52ddfc2ddc6db4ea8f1c3de88 it meant "has not been initialized or close() has been called". Introduced explicit closed state. Minor tweak to use initialized state to determine whether cleanup will do anything. Added a test. --- .../okhttp/internal/DiskLruCacheTest.java | 10 ++++++++++ .../okhttp/internal/DiskLruCache.java | 20 ++++++++++++------- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/internal/DiskLruCacheTest.java b/okhttp-tests/src/test/java/com/squareup/okhttp/internal/DiskLruCacheTest.java index cf3d2acd0..23d3cbb06 100644 --- a/okhttp-tests/src/test/java/com/squareup/okhttp/internal/DiskLruCacheTest.java +++ b/okhttp-tests/src/test/java/com/squareup/okhttp/internal/DiskLruCacheTest.java @@ -1081,6 +1081,16 @@ public final class DiskLruCacheTest { assertFalse(iterator.hasNext()); } + @Test public void isClosed_uninitializedCache() throws Exception { + // Create an uninitialized cache. + cache = new DiskLruCache(cacheDir, appVersion, 2, Integer.MAX_VALUE, executor); + toClose.add(cache); + + assertFalse(cache.isClosed()); + cache.close(); + assertTrue(cache.isClosed()); + } + private void assertJournalEquals(String... expectedBodyLines) throws Exception { List expectedLines = new ArrayList<>(); expectedLines.add(MAGIC); diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/DiskLruCache.java b/okhttp/src/main/java/com/squareup/okhttp/internal/DiskLruCache.java index 061fafed9..37e5fe3ef 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/DiskLruCache.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/DiskLruCache.java @@ -152,6 +152,7 @@ public final class DiskLruCache implements Closeable { // Must be read and written when synchronized on 'this'. private boolean initialized; + private boolean closed; /** * To differentiate between old and current snapshots, each entry is given @@ -165,8 +166,8 @@ public final class DiskLruCache implements Closeable { private final Runnable cleanupRunnable = new Runnable() { public void run() { synchronized (DiskLruCache.this) { - if (isClosed()) { - return; // Closed. + if (!initialized | closed) { + return; // Nothing to do } try { trimToSize(); @@ -221,6 +222,7 @@ public final class DiskLruCache implements Closeable { Platform.get().logW("DiskLruCache " + directory + " is corrupt: " + journalIsCorrupt.getMessage() + ", removing"); delete(); + closed = false; } } @@ -490,7 +492,9 @@ public final class DiskLruCache implements Closeable { */ public synchronized void setMaxSize(long maxSize) { this.maxSize = maxSize; - executor.execute(cleanupRunnable); + if (initialized) { + executor.execute(cleanupRunnable); + } } /** @@ -615,7 +619,7 @@ public final class DiskLruCache implements Closeable { /** Returns true if this cache has been closed. */ public synchronized boolean isClosed() { - return journalWriter == null; + return closed; } private synchronized void checkNotClosed() { @@ -635,8 +639,9 @@ public final class DiskLruCache implements Closeable { /** Closes this cache. Stored values will remain on the filesystem. */ public synchronized void close() throws IOException { - if (isClosed()) { - return; // Already closed. + if (!initialized || closed) { + closed = true; + return; } // Copying for safe iteration. for (Entry entry : lruEntries.values().toArray(new Entry[lruEntries.size()])) { @@ -647,6 +652,7 @@ public final class DiskLruCache implements Closeable { trimToSize(); journalWriter.close(); journalWriter = null; + closed = true; } private void trimToSize() throws IOException { @@ -726,7 +732,7 @@ public final class DiskLruCache implements Closeable { synchronized (DiskLruCache.this) { // If the cache is closed, truncate the iterator. - if (isClosed()) return false; + if (closed) return false; while (delegate.hasNext()) { Entry entry = delegate.next();