Merge pull request #10570 from Artemych:fix/progressive_downloader_infinite_loop

PiperOrigin-RevId: 472475124

RELEASENOTES.md

@@ -11,6 +11,10 @@
         ([#10057](https://github.com/google/ExoPlayer/issues/10057)).
         ([#10057](https://github.com/google/ExoPlayer/issues/10057)).
     *   Limit parallel download removals to 1 to avoid excessive thread creation
     *   Limit parallel download removals to 1 to avoid excessive thread creation
         ([#10458](https://github.com/google/ExoPlayer/issues/10458)).
         ([#10458](https://github.com/google/ExoPlayer/issues/10458)).
+*   Downloads:
+    *   Fix potential infinite loop in `ProgressiveDownloader` caused by
+        simultaneous download and playback with the same `PriorityTaskManager`
+        ([#10570](https://github.com/google/ExoPlayer/pull/10570)).
 *   Audio:
 *   Audio:
     *   Adds `AudioOffloadListener.onExperimentalOffloadedPlayback` for the
     *   Adds `AudioOffloadListener.onExperimentalOffloadedPlayback` for the
         AudioTrack offload state.
         AudioTrack offload state.

libraries/exoplayer/src/main/java/androidx/media3/exoplayer/offline/ProgressiveDownloader.java

@@ -15,6 +15,8 @@
  */
  */
 package androidx.media3.exoplayer.offline;
 package androidx.media3.exoplayer.offline;
 
 
+import static androidx.media3.common.util.Assertions.checkNotNull;
+
 import androidx.annotation.Nullable;
 import androidx.annotation.Nullable;
 import androidx.media3.common.C;
 import androidx.media3.common.C;
 import androidx.media3.common.MediaItem;
 import androidx.media3.common.MediaItem;
@@ -90,26 +92,26 @@ public final class ProgressiveDownloader implements Downloader {
   public void download(@Nullable ProgressListener progressListener)
   public void download(@Nullable ProgressListener progressListener)
       throws IOException, InterruptedException {
       throws IOException, InterruptedException {
     this.progressListener = progressListener;
     this.progressListener = progressListener;
-    downloadRunnable =
-        new RunnableFutureTask<Void, IOException>() {
-          @Override
-          protected Void doWork() throws IOException {
-            cacheWriter.cache();
-            return null;
-          }
-
-          @Override
-          protected void cancelWork() {
-            cacheWriter.cancel();
-          }
-        };
-
     if (priorityTaskManager != null) {
     if (priorityTaskManager != null) {
       priorityTaskManager.add(C.PRIORITY_DOWNLOAD);
       priorityTaskManager.add(C.PRIORITY_DOWNLOAD);
     }
     }
     try {
     try {
       boolean finished = false;
       boolean finished = false;
       while (!finished && !isCanceled) {
       while (!finished && !isCanceled) {
+        // Recreate downloadRunnable on each loop iteration to avoid rethrowing a previous error.
+        downloadRunnable =
+            new RunnableFutureTask<Void, IOException>() {
+              @Override
+              protected Void doWork() throws IOException {
+                cacheWriter.cache();
+                return null;
+              }
+
+              @Override
+              protected void cancelWork() {
+                cacheWriter.cancel();
+              }
+            };
         if (priorityTaskManager != null) {
         if (priorityTaskManager != null) {
           priorityTaskManager.proceed(C.PRIORITY_DOWNLOAD);
           priorityTaskManager.proceed(C.PRIORITY_DOWNLOAD);
         }
         }
@@ -132,7 +134,7 @@ public final class ProgressiveDownloader implements Downloader {
     } finally {
     } finally {
       // If the main download thread was interrupted as part of cancelation, then it's possible that
       // If the main download thread was interrupted as part of cancelation, then it's possible that
       // the runnable is still doing work. We need to wait until it's finished before returning.
       // the runnable is still doing work. We need to wait until it's finished before returning.
-      downloadRunnable.blockUntilFinished();
+      checkNotNull(downloadRunnable).blockUntilFinished();
       if (priorityTaskManager != null) {
       if (priorityTaskManager != null) {
         priorityTaskManager.remove(C.PRIORITY_DOWNLOAD);
         priorityTaskManager.remove(C.PRIORITY_DOWNLOAD);
       }
       }

libraries/exoplayer/src/test/java/androidx/media3/exoplayer/offline/ProgressiveDownloaderTest.java

@@ -19,7 +19,9 @@ import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertThrows;
 
 
 import android.net.Uri;
 import android.net.Uri;
+import androidx.media3.common.C;
 import androidx.media3.common.MediaItem;
 import androidx.media3.common.MediaItem;
+import androidx.media3.common.PriorityTaskManager;
 import androidx.media3.common.util.Util;
 import androidx.media3.common.util.Util;
 import androidx.media3.database.DatabaseProvider;
 import androidx.media3.database.DatabaseProvider;
 import androidx.media3.datasource.DataSource;
 import androidx.media3.datasource.DataSource;
@@ -126,6 +128,51 @@ public class ProgressiveDownloaderTest {
     assertThat(progressListener.bytesDownloaded).isEqualTo(1024);
     assertThat(progressListener.bytesDownloaded).isEqualTo(1024);
   }
   }
 
 
+  @Test
+  public void download_afterPriorityTooLow_succeeds() throws Exception {
+    PriorityTaskManager priorityTaskManager = new PriorityTaskManager();
+    AtomicBoolean hasSetPlaybackPriority = new AtomicBoolean(/* initialValue= */ false);
+    Uri uri = Uri.parse("test:///test.mp4");
+    // Fake data which briefly sets the playback priority while downloading for the first time.
+    FakeDataSet data = new FakeDataSet();
+    data.newData(uri)
+        .appendReadAction(
+            () -> {
+              if (!hasSetPlaybackPriority.getAndSet(true)) {
+                // This only interrupts the download the next time the DataSource checks for the
+                // priority, so delay the removal of the playback priority slightly. As we can't
+                // check when the DataSource throws the PriorityTooLowException exactly, we need to
+                // use an arbitrary delay.
+                priorityTaskManager.add(C.PRIORITY_PLAYBACK);
+                new Thread(
+                        () -> {
+                          try {
+                            Thread.sleep(200);
+                          } catch (InterruptedException e) {
+                            // Ignore.
+                          }
+                          priorityTaskManager.remove(C.PRIORITY_PLAYBACK);
+                        })
+                    .start();
+              }
+            })
+        .appendReadData(2_000_000);
+    DataSource.Factory upstreamDataSource = new FakeDataSource.Factory().setFakeDataSet(data);
+    MediaItem mediaItem = MediaItem.fromUri(uri);
+    CacheDataSource.Factory cacheDataSourceFactory =
+        new CacheDataSource.Factory()
+            .setCache(downloadCache)
+            .setUpstreamDataSourceFactory(upstreamDataSource)
+            .setUpstreamPriorityTaskManager(priorityTaskManager);
+    ProgressiveDownloader downloader = new ProgressiveDownloader(mediaItem, cacheDataSourceFactory);
+    TestProgressListener progressListener = new TestProgressListener();
+
+    // Download expected to finish (despite the interruption by the higher playback priority).
+    downloader.download(progressListener);
+
+    assertThat(progressListener.bytesDownloaded).isEqualTo(2_000_000);
+  }
+
   private static final class TestProgressListener implements Downloader.ProgressListener {
   private static final class TestProgressListener implements Downloader.ProgressListener {
 
 
     public long bytesDownloaded;
     public long bytesDownloaded;