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 5a6a76bce..cf3d2acd0 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 @@ -72,7 +72,9 @@ public final class DiskLruCacheTest { private void createNewCacheWithSize(int maxSize) throws IOException { cache = new DiskLruCache(cacheDir, appVersion, 2, maxSize, executor); - cache.initialize(); + synchronized (cache) { + cache.initialize(); + } toClose.add(cache); } @@ -643,7 +645,7 @@ public final class DiskLruCacheTest { @Test public void constructorDoesNotAllowZeroCacheSize() throws Exception { try { - DiskLruCache.open(cacheDir, appVersion, 2, 0); + DiskLruCache.create(cacheDir, appVersion, 2, 0); fail(); } catch (IllegalArgumentException expected) { } @@ -651,7 +653,7 @@ public final class DiskLruCacheTest { @Test public void constructorDoesNotAllowZeroValuesPerEntry() throws Exception { try { - DiskLruCache.open(cacheDir, appVersion, 0, 10); + DiskLruCache.create(cacheDir, appVersion, 0, 10); fail(); } catch (IllegalArgumentException expected) { } @@ -772,7 +774,7 @@ public final class DiskLruCacheTest { @Test public void openCreatesDirectoryIfNecessary() throws Exception { cache.close(); File dir = tempDir.newFolder("testOpenCreatesDirectoryIfNecessary"); - cache = DiskLruCache.open(dir, appVersion, 2, Integer.MAX_VALUE); + cache = DiskLruCache.create(dir, appVersion, 2, Integer.MAX_VALUE); set("a", "a", "a"); assertTrue(new File(dir, "a.0").exists()); assertTrue(new File(dir, "a.1").exists()); diff --git a/okhttp/src/main/java/com/squareup/okhttp/Cache.java b/okhttp/src/main/java/com/squareup/okhttp/Cache.java index 231a66436..2b98355e8 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Cache.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Cache.java @@ -138,8 +138,8 @@ public final class Cache { private int hitCount; private int requestCount; - public Cache(File directory, long maxSize) throws IOException { - cache = DiskLruCache.open(directory, VERSION, ENTRY_COUNT, maxSize); + public Cache(File directory, long maxSize) { + cache = DiskLruCache.create(directory, VERSION, ENTRY_COUNT, maxSize); } private static String urlToKey(Request request) { @@ -269,7 +269,7 @@ public final class Cache { *

The iterator supports {@linkplain Iterator#remove}. Removing a URL from the iterator evicts * the corresponding response from the cache. Use this to evict selected responses. */ - public Iterator urls() { + public Iterator urls() throws IOException { return new Iterator() { final Iterator delegate = cache.snapshots(); @@ -320,7 +320,7 @@ public final class Cache { return writeSuccessCount; } - public long getSize() { + public long getSize() throws IOException { return cache.size(); } 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 cab166493..061fafed9 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/DiskLruCache.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/DiskLruCache.java @@ -150,6 +150,9 @@ public final class DiskLruCache implements Closeable { private final LinkedHashMap lruEntries = new LinkedHashMap<>(0, 0.75f, true); private int redundantOpCount; + // Must be read and written when synchronized on 'this'. + private boolean initialized; + /** * To differentiate between old and current snapshots, each entry is given * a sequence number each time an edit is committed. A snapshot is stale if @@ -162,7 +165,7 @@ public final class DiskLruCache implements Closeable { private final Runnable cleanupRunnable = new Runnable() { public void run() { synchronized (DiskLruCache.this) { - if (journalWriter == null) { + if (isClosed()) { return; // Closed. } try { @@ -191,6 +194,12 @@ public final class DiskLruCache implements Closeable { // Visible for testing. void initialize() throws IOException { + assert Thread.holdsLock(this); + + if (initialized) { + return; // Already initialized. + } + // If a bkp file exists, use it instead. if (journalFileBackup.exists()) { // If journal file also exists just delete backup file. @@ -206,6 +215,7 @@ public final class DiskLruCache implements Closeable { try { readJournal(); processJournal(); + initialized = true; return; } catch (IOException journalIsCorrupt) { Platform.get().logW("DiskLruCache " + directory + " is corrupt: " @@ -216,19 +226,19 @@ public final class DiskLruCache implements Closeable { directory.mkdirs(); rebuildJournal(); + + initialized = true; } /** - * Opens the cache in {@code directory}, creating a cache if none exists - * there. + * Create a cache which will reside in {@code directory}. This cache is lazily initialized on + * first access and will be created if it does not exist. * * @param directory a writable directory * @param valueCount the number of values per cache entry. Must be positive. * @param maxSize the maximum number of bytes this cache should use to store - * @throws IOException if reading or writing the cache directory fails */ - public static DiskLruCache open(File directory, int appVersion, int valueCount, long maxSize) - throws IOException { + public static DiskLruCache create(File directory, int appVersion, int valueCount, long maxSize) { if (maxSize <= 0) { throw new IllegalArgumentException("maxSize <= 0"); } @@ -240,9 +250,7 @@ public final class DiskLruCache implements Closeable { Executor executor = new ThreadPoolExecutor(0, 1, 60L, TimeUnit.SECONDS, new LinkedBlockingQueue(), Util.threadFactory("OkHttp DiskLruCache", true)); - DiskLruCache cache = new DiskLruCache(directory, appVersion, valueCount, maxSize, executor); - cache.initialize(); - return cache; + return new DiskLruCache(directory, appVersion, valueCount, maxSize, executor); } private void readJournal() throws IOException { @@ -410,6 +418,8 @@ public final class DiskLruCache implements Closeable { * the head of the LRU queue. */ public synchronized Snapshot get(String key) throws IOException { + initialize(); + checkNotClosed(); validateKey(key); Entry entry = lruEntries.get(key); @@ -436,6 +446,8 @@ public final class DiskLruCache implements Closeable { } private synchronized Editor edit(String key, long expectedSequenceNumber) throws IOException { + initialize(); + checkNotClosed(); validateKey(key); Entry entry = lruEntries.get(key); @@ -486,7 +498,8 @@ public final class DiskLruCache implements Closeable { * this cache. This may be greater than the max size if a background * deletion is pending. */ - public synchronized long size() { + public synchronized long size() throws IOException { + initialize(); return size; } @@ -568,6 +581,8 @@ public final class DiskLruCache implements Closeable { * @return true if an entry was removed. */ public synchronized boolean remove(String key) throws IOException { + initialize(); + checkNotClosed(); validateKey(key); Entry entry = lruEntries.get(key); @@ -599,18 +614,20 @@ public final class DiskLruCache implements Closeable { } /** Returns true if this cache has been closed. */ - public boolean isClosed() { + public synchronized boolean isClosed() { return journalWriter == null; } - private void checkNotClosed() { - if (journalWriter == null) { + private synchronized void checkNotClosed() { + if (isClosed()) { throw new IllegalStateException("cache is closed"); } } /** Force buffered operations to the filesystem. */ public synchronized void flush() throws IOException { + if (!initialized) return; + checkNotClosed(); trimToSize(); journalWriter.flush(); @@ -618,7 +635,7 @@ public final class DiskLruCache implements Closeable { /** Closes this cache. Stored values will remain on the filesystem. */ public synchronized void close() throws IOException { - if (journalWriter == null) { + if (isClosed()) { return; // Already closed. } // Copying for safe iteration. @@ -654,6 +671,7 @@ public final class DiskLruCache implements Closeable { * normally but their values will not be stored. */ public synchronized void evictAll() throws IOException { + initialize(); // Copying for safe iteration. for (Entry entry : lruEntries.values().toArray(new Entry[lruEntries.size()])) { removeEntry(entry); @@ -691,7 +709,8 @@ public final class DiskLruCache implements Closeable { * *

The returned iterator supports {@link Iterator#remove}. */ - public synchronized Iterator snapshots() { + public synchronized Iterator snapshots() throws IOException { + initialize(); return new Iterator() { /** Iterate a copy of the entries to defend against concurrent modification errors. */ final Iterator delegate = new ArrayList<>(lruEntries.values()).iterator();