Prevent multiple instances of SimpleCache in the same folder

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=189562865

RELEASENOTES.md

@@ -22,6 +22,10 @@
   * Allow clipping of child media sources where the period and window have a
     non-zero offset with `ClippingMediaSource`.
 * Audio: Factor out `AudioTrack` position tracking from `DefaultAudioSink`.
+* Caching:
+  * Add release method to Cache interface.
+  * Prevent multiple instances of SimpleCache in the same folder. 
+    Previous instance must be released.
 
 ### 2.7.1 ###
 

library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/Cache.java

@@ -79,6 +79,12 @@ public interface Cache {
 
   }
 
+  /**
+   * Releases the cache. This method must be called when the cache is no longer required. The cache
+   * must not be used after calling this method.
+   */
+  void release() throws CacheException;
+
   /**
    * Registers a listener to listen for changes to a given key.
    *

library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/SimpleCache.java

@@ -16,6 +16,7 @@
 package com.google.android.exoplayer2.upstream.cache;
 
 import android.os.ConditionVariable;
+import android.support.annotation.NonNull;
 import android.util.Log;
 import com.google.android.exoplayer2.C;
 import com.google.android.exoplayer2.util.Assertions;
@@ -28,17 +29,21 @@ import java.util.Set;
 import java.util.TreeSet;
 
 /**
- * A {@link Cache} implementation that maintains an in-memory representation.
+ * A {@link Cache} implementation that maintains an in-memory representation. Note, only one
+ * instance of SimpleCache is allowed for a given directory at a given time.
  */
 public final class SimpleCache implements Cache {
 
   private static final String TAG = "SimpleCache";
+  private static final HashSet<File> lockedCacheDirs = new HashSet<>();
 
   private final File cacheDir;
   private final CacheEvictor evictor;
   private final CachedContentIndex index;
   private final HashMap<String, ArrayList<Listener>> listeners;
-  private long totalSpace = 0;
+
+  private long totalSpace;
+  private boolean released;
 
   /**
    * Constructs the cache. The cache will delete any unrecognized files from the directory. Hence
@@ -88,10 +93,15 @@ public final class SimpleCache implements Cache {
    * @param index The CachedContentIndex to be used.
    */
   /*package*/ SimpleCache(File cacheDir, CacheEvictor evictor, CachedContentIndex index) {
+    if (!lockFolder(cacheDir)) {
+      throw new IllegalStateException("Another SimpleCache instance uses the folder: " + cacheDir);
+    }
+
     this.cacheDir = cacheDir;
     this.evictor = evictor;
     this.index = index;
     this.listeners = new HashMap<>();
+
     // Start cache initialization.
     final ConditionVariable conditionVariable = new ConditionVariable();
     new Thread("SimpleCache.initialize()") {
@@ -107,8 +117,23 @@ public final class SimpleCache implements Cache {
     conditionVariable.block();
   }
 
+  @Override
+  public synchronized void release() throws CacheException {
+    if (released) {
+      return;
+    }
+    listeners.clear();
+    try {
+      removeStaleSpansAndCachedContents();
+    } finally {
+      releaseFolder(cacheDir);
+      released = true;
+    }
+  }
+
   @Override
   public synchronized NavigableSet<CacheSpan> addListener(String key, Listener listener) {
+    Assertions.checkState(!released);
     ArrayList<Listener> listenersForKey = listeners.get(key);
     if (listenersForKey == null) {
       listenersForKey = new ArrayList<>();
@@ -120,6 +145,9 @@ public final class SimpleCache implements Cache {
 
   @Override
   public synchronized void removeListener(String key, Listener listener) {
+    if (released) {
+      return;
+    }
     ArrayList<Listener> listenersForKey = listeners.get(key);
     if (listenersForKey != null) {
       listenersForKey.remove(listener);
@@ -129,8 +157,10 @@ public final class SimpleCache implements Cache {
     }
   }
 
+  @NonNull
   @Override
   public synchronized NavigableSet<CacheSpan> getCachedSpans(String key) {
+    Assertions.checkState(!released);
     CachedContent cachedContent = index.get(key);
     return cachedContent == null || cachedContent.isEmpty()
         ? new TreeSet<CacheSpan>()
@@ -139,11 +169,13 @@ public final class SimpleCache implements Cache {
 
   @Override
   public synchronized Set<String> getKeys() {
+    Assertions.checkState(!released);
     return new HashSet<>(index.getKeys());
   }
 
   @Override
   public synchronized long getCacheSpace() {
+    Assertions.checkState(!released);
     return totalSpace;
   }
 
@@ -167,6 +199,7 @@ public final class SimpleCache implements Cache {
   @Override
   public synchronized SimpleCacheSpan startReadWriteNonBlocking(String key, long position)
       throws CacheException {
+    Assertions.checkState(!released);
     SimpleCacheSpan cacheSpan = getSpan(key, position);
 
     // Read case.
@@ -191,13 +224,14 @@ public final class SimpleCache implements Cache {
   @Override
   public synchronized File startFile(String key, long position, long maxLength)
       throws CacheException {
+    Assertions.checkState(!released);
     CachedContent cachedContent = index.get(key);
     Assertions.checkNotNull(cachedContent);
     Assertions.checkState(cachedContent.isLocked());
     if (!cacheDir.exists()) {
       // For some reason the cache directory doesn't exist. Make a best effort to create it.
-      removeStaleSpansAndCachedContents();
       cacheDir.mkdirs();
+      removeStaleSpansAndCachedContents();
     }
     evictor.onStartFile(this, key, position, maxLength);
     return SimpleCacheSpan.getCacheFile(
@@ -206,6 +240,7 @@ public final class SimpleCache implements Cache {
 
   @Override
   public synchronized void commitFile(File file) throws CacheException {
+    Assertions.checkState(!released);
     SimpleCacheSpan span = SimpleCacheSpan.createCacheEntry(file, index);
     Assertions.checkState(span != null);
     CachedContent cachedContent = index.get(span.key);
@@ -221,7 +256,7 @@ public final class SimpleCache implements Cache {
       return;
     }
     // Check if the span conflicts with the set content length
-    Long length = cachedContent.getLength();
+    long length = cachedContent.getLength();
     if (length != C.LENGTH_UNSET) {
       Assertions.checkState((span.position + span.length) <= length);
     }
@@ -232,6 +267,7 @@ public final class SimpleCache implements Cache {
 
   @Override
   public synchronized void releaseHoleSpan(CacheSpan holeSpan) {
+    Assertions.checkState(!released);
     CachedContent cachedContent = index.get(holeSpan.key);
     Assertions.checkNotNull(cachedContent);
     Assertions.checkState(cachedContent.isLocked());
@@ -240,6 +276,39 @@ public final class SimpleCache implements Cache {
     notifyAll();
   }
 
+  @Override
+  public synchronized void removeSpan(CacheSpan span) throws CacheException {
+    Assertions.checkState(!released);
+    removeSpan(span, true);
+  }
+
+  @Override
+  public synchronized boolean isCached(String key, long position, long length) {
+    Assertions.checkState(!released);
+    CachedContent cachedContent = index.get(key);
+    return cachedContent != null && cachedContent.getCachedBytesLength(position, length) >= length;
+  }
+
+  @Override
+  public synchronized long getCachedLength(String key, long position, long length) {
+    Assertions.checkState(!released);
+    CachedContent cachedContent = index.get(key);
+    return cachedContent != null ? cachedContent.getCachedBytesLength(position, length) : -length;
+  }
+
+  @Override
+  public synchronized void setContentLength(String key, long length) throws CacheException {
+    Assertions.checkState(!released);
+    index.setContentLength(key, length);
+    index.store();
+  }
+
+  @Override
+  public synchronized long getContentLength(String key) {
+    Assertions.checkState(!released);
+    return index.getContentLength(key);
+  }
+
   /**
    * Returns the cache {@link SimpleCacheSpan} corresponding to the provided lookup {@link
    * SimpleCacheSpan}.
@@ -270,9 +339,7 @@ public final class SimpleCache implements Cache {
     }
   }
 
-  /**
+  /** Ensures that the cache's in-memory representation has been initialized. */
-   * Ensures that the cache's in-memory representation has been initialized.
-   */
   private void initialize() {
     if (!cacheDir.exists()) {
       cacheDir.mkdirs();
@@ -289,8 +356,8 @@ public final class SimpleCache implements Cache {
       if (file.getName().equals(CachedContentIndex.FILE_NAME)) {
         continue;
       }
-      SimpleCacheSpan span = file.length() > 0
+      SimpleCacheSpan span =
-          ? SimpleCacheSpan.createCacheEntry(file, index) : null;
+          file.length() > 0 ? SimpleCacheSpan.createCacheEntry(file, index) : null;
       if (span != null) {
         addSpan(span);
       } else {
@@ -333,14 +400,9 @@ public final class SimpleCache implements Cache {
     }
   }
 
-  @Override
-  public synchronized void removeSpan(CacheSpan span) throws CacheException {
-    removeSpan(span, true);
-  }
-
   /**
-   * Scans all of the cached spans in the in-memory representation, removing any for which files
+   * Scans all of the cached spans in the in-memory representation, removing any for which files no
-   * no longer exist.
+   * longer exist.
    */
   private void removeStaleSpansAndCachedContents() throws CacheException {
     ArrayList<CacheSpan> spansToBeRemoved = new ArrayList<>();
@@ -389,29 +451,13 @@ public final class SimpleCache implements Cache {
     evictor.onSpanTouched(this, oldSpan, newSpan);
   }
 
-  @Override
+  private static synchronized boolean lockFolder(File cacheDir) {
-  public synchronized boolean isCached(String key, long position, long length) {
+    return lockedCacheDirs.add(cacheDir.getAbsoluteFile());
-    CachedContent cachedContent = index.get(key);
-    return cachedContent != null && cachedContent.getCachedBytesLength(position, length) >= length;
   }
 
-  @Override
+  private static synchronized void releaseFolder(File cacheDir) {
-  public synchronized long getCachedLength(String key, long position, long length) {
+    lockedCacheDirs.remove(cacheDir.getAbsoluteFile());
-    CachedContent cachedContent = index.get(key);
-    return cachedContent != null ? cachedContent.getCachedBytesLength(position, length) : -length;
   }
-
-  @Override
-  public synchronized void setContentLength(String key, long length) throws CacheException {
-    index.setContentLength(key, length);
-    index.store();
-  }
-
-  @Override
-  public synchronized long getContentLength(String key) {
-    return index.getContentLength(key);
-  }
-
   @Override
   public void applyContentMetadataMutations(String key, ContentMetadataMutations mutations)
       throws CacheException {

library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/SimpleCacheTest.java

@@ -18,6 +18,7 @@ package com.google.android.exoplayer2.upstream.cache;
 import static com.google.android.exoplayer2.C.LENGTH_UNSET;
 import static com.google.android.exoplayer2.util.Util.toByteArray;
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;
 import static org.mockito.Mockito.doAnswer;
 
 import com.google.android.exoplayer2.C;
@@ -31,7 +32,6 @@ import java.util.NavigableSet;
 import java.util.Random;
 import java.util.Set;
 import org.junit.After;
-import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -111,31 +111,27 @@ public class SimpleCacheTest {
     SimpleCache simpleCache = getSimpleCache();
 
     assertThat(simpleCache.getContentLength(KEY_1)).isEqualTo(LENGTH_UNSET);
+
     simpleCache.setContentLength(KEY_1, 15);
     assertThat(simpleCache.getContentLength(KEY_1)).isEqualTo(15);
 
     simpleCache.startReadWrite(KEY_1, 0);
-
     addCache(simpleCache, KEY_1, 0, 15);
-
     simpleCache.setContentLength(KEY_1, 150);
     assertThat(simpleCache.getContentLength(KEY_1)).isEqualTo(150);
 
     addCache(simpleCache, KEY_1, 140, 10);
 
+    simpleCache.release();
+
     // Check if values are kept after cache is reloaded.
     SimpleCache simpleCache2 = getSimpleCache();
-    Set<String> keys = simpleCache.getKeys();
+    assertThat(simpleCache2.getContentLength(KEY_1)).isEqualTo(150);
-    Set<String> keys2 = simpleCache2.getKeys();
-    assertThat(keys2).isEqualTo(keys);
-    for (String key : keys) {
-      assertThat(simpleCache2.getContentLength(key)).isEqualTo(simpleCache.getContentLength(key));
-      assertThat(simpleCache2.getCachedSpans(key)).isEqualTo(simpleCache.getCachedSpans(key));
-    }
 
     // Removing the last span shouldn't cause the length be change next time cache loaded
     SimpleCacheSpan lastSpan = simpleCache2.startReadWrite(KEY_1, 145);
     simpleCache2.removeSpan(lastSpan);
+    simpleCache2.release();
     simpleCache2 = getSimpleCache();
     assertThat(simpleCache2.getContentLength(KEY_1)).isEqualTo(150);
   }
@@ -148,6 +144,7 @@ public class SimpleCacheTest {
     CacheSpan cacheSpan1 = simpleCache.startReadWrite(KEY_1, 0);
     addCache(simpleCache, KEY_1, 0, 15);
     simpleCache.releaseHoleSpan(cacheSpan1);
+    simpleCache.release();
 
     // Reload cache
     simpleCache = getSimpleCache();
@@ -166,6 +163,7 @@ public class SimpleCacheTest {
     CacheSpan cacheSpan1 = simpleCache.startReadWrite(KEY_1, 0);
     addCache(simpleCache, KEY_1, 0, 15);
     simpleCache.releaseHoleSpan(cacheSpan1);
+    simpleCache.release();
 
     // Reload cache
     simpleCache = getEncryptedSimpleCache(key);
@@ -184,6 +182,7 @@ public class SimpleCacheTest {
     CacheSpan cacheSpan1 = simpleCache.startReadWrite(KEY_1, 0);
     addCache(simpleCache, KEY_1, 0, 15);
     simpleCache.releaseHoleSpan(cacheSpan1);
+    simpleCache.release();
 
     // Reload cache
     byte[] key2 = "Foo12345Foo12345".getBytes(C.UTF8_NAME); // 128 bit key
@@ -203,6 +202,7 @@ public class SimpleCacheTest {
     CacheSpan cacheSpan1 = simpleCache.startReadWrite(KEY_1, 0);
     addCache(simpleCache, KEY_1, 0, 15);
     simpleCache.releaseHoleSpan(cacheSpan1);
+    simpleCache.release();
 
     // Reload cache
     simpleCache = getSimpleCache();
@@ -269,7 +269,7 @@ public class SimpleCacheTest {
     // Adding more content will make LeastRecentlyUsedCacheEvictor evict previous content.
     try {
       addCache(simpleCache, KEY_1, 15, 15);
-      Assert.fail("Exception was expected");
+      assertWithMessage("Exception was expected").fail();
     } catch (CacheException e) {
       // do nothing.
     }
@@ -283,6 +283,40 @@ public class SimpleCacheTest {
     assertThat(cachedSpans.pollFirst().position).isEqualTo(15);
   }
 
+  @Test
+  public void testUsingReleasedSimpleCacheThrowsException() throws Exception {
+    SimpleCache simpleCache = new SimpleCache(cacheDir, new NoOpCacheEvictor());
+    simpleCache.release();
+
+    try {
+      simpleCache.startReadWriteNonBlocking(KEY_1, 0);
+      assertWithMessage("Exception was expected").fail();
+    } catch (RuntimeException e) {
+      // Expected. Do nothing.
+    }
+  }
+
+  @Test
+  public void testMultipleSimpleCacheWithSameCacheDirThrowsException() throws Exception {
+    new SimpleCache(cacheDir, new NoOpCacheEvictor());
+
+    try {
+      new SimpleCache(cacheDir, new NoOpCacheEvictor());
+      assertWithMessage("Exception was expected").fail();
+    } catch (IllegalStateException e) {
+      // Expected. Do nothing.
+    }
+  }
+
+  @Test
+  public void testMultipleSimpleCacheWithSameCacheDirDoesNotThrowsExceptionAfterRelease()
+      throws Exception {
+    SimpleCache simpleCache = new SimpleCache(cacheDir, new NoOpCacheEvictor());
+    simpleCache.release();
+
+    new SimpleCache(cacheDir, new NoOpCacheEvictor());
+  }
+
   private SimpleCache getSimpleCache() {
     return new SimpleCache(cacheDir, new NoOpCacheEvictor());
   }