Commit 6174d047 by olly Committed by Oliver Woodman

Fix CacheDataSource hold-lock-forever problem

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=183098172
parent 8716c0ee
...@@ -26,6 +26,7 @@ import com.google.android.exoplayer2.upstream.DataSpec; ...@@ -26,6 +26,7 @@ import com.google.android.exoplayer2.upstream.DataSpec;
import com.google.android.exoplayer2.upstream.FileDataSource; import com.google.android.exoplayer2.upstream.FileDataSource;
import com.google.android.exoplayer2.upstream.TeeDataSource; import com.google.android.exoplayer2.upstream.TeeDataSource;
import com.google.android.exoplayer2.upstream.cache.Cache.CacheException; import com.google.android.exoplayer2.upstream.cache.Cache.CacheException;
import com.google.android.exoplayer2.util.Assertions;
import java.io.IOException; import java.io.IOException;
import java.io.InterruptedIOException; import java.io.InterruptedIOException;
import java.lang.annotation.Retention; import java.lang.annotation.Retention;
...@@ -280,8 +281,14 @@ public final class CacheDataSource implements DataSource { ...@@ -280,8 +281,14 @@ public final class CacheDataSource implements DataSource {
* {@link #cacheReadDataSource} is opened to read from it. Else {@link #upstreamDataSource} is * {@link #cacheReadDataSource} is opened to read from it. Else {@link #upstreamDataSource} is
* opened to read from the upstream source and write into the cache. * opened to read from the upstream source and write into the cache.
* *
* @param checkCache If true tries to switch reading from or writing to cache instead of reading * <p>There must not be a currently open source when this method is called, except in the case
* from upstream. If the switch isn't possible then returns without changing source. * that {@code checkCache} is true. If {@code checkCache} is true then there must be a currently
* open source, and it must be {@link #upstreamDataSource}. It will be closed and a new source
* opened if it's possible to switch to reading from or writing to the cache. If a switch isn't
* possible then the current source is left unchanged.
*
* @param checkCache If true tries to switch to reading from or writing to cache instead of
* reading from {@link #upstreamDataSource}, which is the currently open source.
*/ */
private void openNextSource(boolean checkCache) throws IOException { private void openNextSource(boolean checkCache) throws IOException {
CacheSpan nextSpan; CacheSpan nextSpan;
...@@ -335,22 +342,33 @@ public final class CacheDataSource implements DataSource { ...@@ -335,22 +342,33 @@ public final class CacheDataSource implements DataSource {
} }
} }
if (!currentRequestIgnoresCache && nextDataSource == upstreamDataSource) { checkCachePosition =
checkCachePosition = readPosition + MIN_READ_BEFORE_CHECKING_CACHE; !currentRequestIgnoresCache && nextDataSource == upstreamDataSource
if (checkCache) { ? readPosition + MIN_READ_BEFORE_CHECKING_CACHE
: Long.MAX_VALUE;
if (checkCache) {
Assertions.checkState(currentDataSource == upstreamDataSource);
if (nextDataSource == upstreamDataSource) {
// Continue reading from upstream.
return; return;
} }
} else { // We're switching to reading from or writing to the cache.
checkCachePosition = Long.MAX_VALUE; try {
closeCurrentSource();
} catch (Throwable e) {
if (nextSpan.isHoleSpan()) {
// Release the hole span before throwing, else we'll hold it forever.
cache.releaseHoleSpan(nextSpan);
}
throw e;
}
} }
closeCurrentSource();
if (nextSpan != null && nextSpan.isHoleSpan()) { if (nextSpan != null && nextSpan.isHoleSpan()) {
currentHoleSpan = nextSpan; currentHoleSpan = nextSpan;
} }
currentDataSource = nextDataSource; currentDataSource = nextDataSource;
currentDataSpecLengthUnset = nextDataSpec.length == C.LENGTH_UNSET; currentDataSpecLengthUnset = nextDataSpec.length == C.LENGTH_UNSET;
long resolvedLength = nextDataSource.open(nextDataSpec); long resolvedLength = nextDataSource.open(nextDataSpec);
if (currentDataSpecLengthUnset && resolvedLength != C.LENGTH_UNSET) { if (currentDataSpecLengthUnset && resolvedLength != C.LENGTH_UNSET) {
setBytesRemaining(resolvedLength); setBytesRemaining(resolvedLength);
...@@ -388,9 +406,9 @@ public final class CacheDataSource implements DataSource { ...@@ -388,9 +406,9 @@ public final class CacheDataSource implements DataSource {
} }
try { try {
currentDataSource.close(); currentDataSource.close();
} finally {
currentDataSource = null; currentDataSource = null;
currentDataSpecLengthUnset = false; currentDataSpecLengthUnset = false;
} finally {
if (currentHoleSpan != null) { if (currentHoleSpan != null) {
cache.releaseHoleSpan(currentHoleSpan); cache.releaseHoleSpan(currentHoleSpan);
currentHoleSpan = null; currentHoleSpan = null;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment