From 371f3acea87f6685ee65173ae30cbba5f2b6dd6b Mon Sep 17 00:00:00 2001 From: eguven Date: Mon, 19 Mar 2018 03:44:07 -0700 Subject: [PATCH] Prevent multiple instances of SimpleCache in the same folder ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=189562865 --- RELEASENOTES.md | 4 + .../exoplayer2/upstream/cache/Cache.java | 6 + .../upstream/cache/SimpleCache.java | 118 ++++++++++++------ .../upstream/cache/SimpleCacheTest.java | 56 +++++++-- 4 files changed, 137 insertions(+), 47 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index f059768ed3..d6fa51daf5 100644 --- a/RELEASENOTES.md +++ b/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 ### diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/Cache.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/Cache.java index bd76a8e1d0..5071163969 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/Cache.java +++ b/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. * diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/SimpleCache.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/SimpleCache.java index dd7d10d64d..7e92bd153b 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/SimpleCache.java +++ b/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 lockedCacheDirs = new HashSet<>(); private final File cacheDir; private final CacheEvictor evictor; private final CachedContentIndex index; private final HashMap> 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 addListener(String key, Listener listener) { + Assertions.checkState(!released); ArrayList 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 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 getCachedSpans(String key) { + Assertions.checkState(!released); CachedContent cachedContent = index.get(key); return cachedContent == null || cachedContent.isEmpty() ? new TreeSet() @@ -139,11 +169,13 @@ public final class SimpleCache implements Cache { @Override public synchronized Set 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.createCacheEntry(file, index) : null; + SimpleCacheSpan span = + 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 - * no longer exist. + * Scans all of the cached spans in the in-memory representation, removing any for which files no + * longer exist. */ private void removeStaleSpansAndCachedContents() throws CacheException { ArrayList spansToBeRemoved = new ArrayList<>(); @@ -389,29 +451,13 @@ public final class SimpleCache implements Cache { evictor.onSpanTouched(this, oldSpan, newSpan); } - @Override - public synchronized boolean isCached(String key, long position, long length) { - CachedContent cachedContent = index.get(key); - return cachedContent != null && cachedContent.getCachedBytesLength(position, length) >= length; + private static synchronized boolean lockFolder(File cacheDir) { + return lockedCacheDirs.add(cacheDir.getAbsoluteFile()); } - @Override - public synchronized long getCachedLength(String key, long position, long length) { - CachedContent cachedContent = index.get(key); - return cachedContent != null ? cachedContent.getCachedBytesLength(position, length) : -length; + private static synchronized void releaseFolder(File cacheDir) { + lockedCacheDirs.remove(cacheDir.getAbsoluteFile()); } - - @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 { diff --git a/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/SimpleCacheTest.java b/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/SimpleCacheTest.java index 89ace34edc..15e2b80f59 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/SimpleCacheTest.java +++ b/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 keys = simpleCache.getKeys(); - Set 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)); - } + assertThat(simpleCache2.getContentLength(KEY_1)).isEqualTo(150); // 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()); }