loliball/media
1
0
Fork 0
mirror of https://github.com/androidx/media.git synced 2025-04-30 06:46:50 +08:00
Code Issues Actions Packages Projects Releases Wiki Activity

Ensure DataSource is closed in CacheWriter at non-IOException

Browse Source 
We currently only catch IOExceptions to close the opened data source.
This means any other type of exception leaves the data source open,
which can caused SimpleCache to be blocked forever when another
data source tries to access the same cache area (which is still
locked by the open data source).

This problem was the cause for flakiness in DownloadManagerDashTest.

#cherrypick

Issue: google/ExoPlayer#9760
PiperOrigin-RevId: 729441817
(cherry picked from commit 72c85aa48393e47642688c8147bdf18d6e3d4bbf)
This commit is contained in:
tonihei 2025-02-21 02:09:21 -08:00 committed by oceanjules
parent 8e5368a807
commit 073e50a893
4 changed files with 123 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
RELEASENOTES.md
View File

@ -27,6 +27,10 @@
* Session:
* UI:
* Downloads:
* Fix bug in `CacheWriter` that leaves data sources open and cache areas
locked in case the data source throws an `Exception` other than
`IOException`
([#9760](https://github.com/google/ExoPlayer/issues/9760)).
* OkHttp Extension:
* Cronet Extension:
* RTMP Extension:

6
libraries/datasource/src/main/java/androidx/media3/datasource/cache/CacheWriter.java vendored
View File

@ -159,7 +159,7 @@ public final class CacheWriter {
try {
resolvedLength = dataSource.open(boundedDataSpec);
isDataSourceOpen = true;
} catch (IOException e) {
} catch (Exception e) {
DataSourceUtil.closeQuietly(dataSource);
}
}
@ -172,7 +172,7 @@ public final class CacheWriter {
dataSpec.buildUpon().setPosition(position).setLength(C.LENGTH_UNSET).build();
try {
resolvedLength = dataSource.open(unboundedDataSpec);
} catch (IOException e) {
} catch (Exception e) {
DataSourceUtil.closeQuietly(dataSource);
throw e;
}
@ -195,7 +195,7 @@ public final class CacheWriter {
if (isLastBlock) {
onRequestEndPosition(position + totalBytesRead);
}
} catch (IOException e) {
} catch (Exception e) {
DataSourceUtil.closeQuietly(dataSource);
throw e;
}

115
libraries/datasource/src/test/java/androidx/media3/datasource/cache/CacheWriterTest.java vendored
View File

@ -22,8 +22,10 @@ import static org.junit.Assert.assertThrows;
import android.net.Uri;
import androidx.media3.common.C;
import androidx.media3.common.util.Util;
import androidx.media3.datasource.DataSource;
import androidx.media3.datasource.DataSpec;
import androidx.media3.datasource.FileDataSource;
import androidx.media3.datasource.TransferListener;
import androidx.media3.test.utils.FailOnCloseDataSink;
import androidx.media3.test.utils.FakeDataSet;
import androidx.media3.test.utils.FakeDataSource;
@ -263,6 +265,119 @@ public final class CacheWriterTest {
assertCachedData(cache, fakeDataSet);
}
@Test
public void cache_ioExceptionDuringOpen_closesDataSource() {
FakeDataSet fakeDataSet = new FakeDataSet().newData("test_data").appendReadData(1).endData();
FakeDataSource dataSource = new FakeDataSource(fakeDataSet);
dataSource.addTransferListener(
new TransferListener() {
@Override
public void onTransferInitializing(
DataSource source, DataSpec dataSpec, boolean isNetwork) {
Util.sneakyThrow(new IOException());
}
@Override
public void onTransferStart(DataSource source, DataSpec dataSpec, boolean isNetwork) {}
@Override
public void onBytesTransferred(
DataSource source, DataSpec dataSpec, boolean isNetwork, int bytesTransferred) {}
@Override
public void onTransferEnd(DataSource source, DataSpec dataSpec, boolean isNetwork) {}
});
CacheWriter cacheWriter =
new CacheWriter(
new CacheDataSource(cache, dataSource),
new DataSpec(Uri.parse("test_data")),
/* temporaryBuffer= */ null,
new CachingCounters());
assertThrows(IOException.class, cacheWriter::cache);
assertThat(dataSource.isOpened()).isFalse();
}
@Test
public void cache_ioExceptionDuringRead_closesDataSource() {
FakeDataSet fakeDataSet =
new FakeDataSet()
.newData("test_data")
.appendReadError(new IOException())
.appendReadData(1)
.endData();
FakeDataSource dataSource = new FakeDataSource(fakeDataSet);
CacheWriter cacheWriter =
new CacheWriter(
new CacheDataSource(cache, dataSource),
new DataSpec(Uri.parse("test_data")),
/* temporaryBuffer= */ null,
new CachingCounters());
assertThrows(IOException.class, cacheWriter::cache);
assertThat(dataSource.isOpened()).isFalse();
}
@Test
public void cache_nonIoExceptionDuringOpen_closesDataSource() {
FakeDataSet fakeDataSet = new FakeDataSet().newData("test_data").appendReadData(1).endData();
FakeDataSource dataSource = new FakeDataSource(fakeDataSet);
dataSource.addTransferListener(
new TransferListener() {
@Override
public void onTransferInitializing(
DataSource source, DataSpec dataSpec, boolean isNetwork) {
throw new IllegalStateException();
}
@Override
public void onTransferStart(DataSource source, DataSpec dataSpec, boolean isNetwork) {}
@Override
public void onBytesTransferred(
DataSource source, DataSpec dataSpec, boolean isNetwork, int bytesTransferred) {}
@Override
public void onTransferEnd(DataSource source, DataSpec dataSpec, boolean isNetwork) {}
});
CacheWriter cacheWriter =
new CacheWriter(
new CacheDataSource(cache, dataSource),
new DataSpec(Uri.parse("test_data")),
/* temporaryBuffer= */ null,
new CachingCounters());
assertThrows(IllegalStateException.class, cacheWriter::cache);
assertThat(dataSource.isOpened()).isFalse();
}
@Test
public void cache_nonIoExceptionDuringRead_closesDataSource() {
FakeDataSet fakeDataSet =
new FakeDataSet()
.newData("test_data")
.appendReadAction(
() -> {
throw new IllegalStateException();
})
.appendReadData(1)
.endData();
FakeDataSource dataSource = new FakeDataSource(fakeDataSet);
CacheWriter cacheWriter =
new CacheWriter(
new CacheDataSource(cache, dataSource),
new DataSpec(Uri.parse("test_data")),
/* temporaryBuffer= */ null,
new CachingCounters());
assertThrows(IllegalStateException.class, cacheWriter::cache);
assertThat(dataSource.isOpened()).isFalse();
}
private static final class CachingCounters implements CacheWriter.ProgressListener {
private long contentLength = C.LENGTH_UNSET;

2
libraries/exoplayer_dash/src/test/java/androidx/media3/exoplayer/dash/offline/DownloadManagerDashTest.java
View File

@ -165,7 +165,7 @@ public class DownloadManagerDashTest {
}
@Test
public void handleDownloadRequest_withInferringRequest_success() throws Throwable {
public void handleDownloadRequest_withInterferingRequest_success() throws Throwable {
fakeDataSet
.newData("audio_segment_2")
.appendReadAction(() -> handleDownloadRequest(fakeStreamKey2))