From 0480eff6a15152ae965129e5357d70507c792658 Mon Sep 17 00:00:00 2001
From: christosts <christosts@google.com>
Date: Fri, 23 Feb 2024 05:51:18 -0800
Subject: [PATCH] ExoPlayerImplInternal.releaseInternal(): unblock the app
 thread

This change makes ExoPlayerImplInternal.releaseInternal() unblock the
app thread if a runtime exception is thrown while releasing components
from the playback thread.

Before this change, if a runtime exception occurred during releasing
components in the playback thread, ExoPlayer.release() would wait for
`releaseTimeoutMs` and then raise a player error. With this change,
the player error is reported only when the playback thread is blocked
but if there is a runtime exception, the application thread is
unblocked.

The impact of this change is potentially fewer ANRs on
ExoPlayer.release() at the expense of less error reporting.

PiperOrigin-RevId: 609702549
---
 .../exoplayer/ExoPlayerImplInternal.java      | 31 +++++++++--------
 .../media3/exoplayer/ExoPlayerTest.java       | 34 +++++++++++++++++++
 2 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java
index 6628bcf6a0..26e6b63274 100644
--- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java
+++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java
@@ -1478,20 +1478,23 @@ import java.util.concurrent.atomic.AtomicBoolean;
   }
 
   private void releaseInternal() {
-    resetInternal(
-        /* resetRenderers= */ true,
-        /* resetPosition= */ false,
-        /* releaseMediaSourceList= */ true,
-        /* resetError= */ false);
-    releaseRenderers();
-    loadControl.onReleased();
-    setState(Player.STATE_IDLE);
-    if (internalPlaybackThread != null) {
-      internalPlaybackThread.quit();
-    }
-    synchronized (this) {
-      released = true;
-      notifyAll();
+    try {
+      resetInternal(
+          /* resetRenderers= */ true,
+          /* resetPosition= */ false,
+          /* releaseMediaSourceList= */ true,
+          /* resetError= */ false);
+      releaseRenderers();
+      loadControl.onReleased();
+      setState(Player.STATE_IDLE);
+    } finally {
+      if (internalPlaybackThread != null) {
+        internalPlaybackThread.quit();
+      }
+      synchronized (this) {
+        released = true;
+        notifyAll();
+      }
     }
   }
 
diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java
index 7999271b33..fcaf562552 100644
--- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java
+++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java
@@ -85,6 +85,7 @@ import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
@@ -12934,6 +12935,39 @@ public final class ExoPlayerTest {
     verify(listener).onVolumeChanged(anyFloat());
   }
 
+  /**
+   * This test verifies that {@link ExoPlayer#release()} will return without a timeout reported when
+   * there is a {@link RuntimeException} thrown on the playback thread during releasing the internal
+   * components.
+   */
+  @Test
+  public void release_internalFailure_noTimeoutError() {
+    Player.Listener listener = mock(Player.Listener.class);
+    LoadControl loadControl =
+        spy(
+            new DefaultLoadControl() {
+              @Override
+              public void onReleased() {
+                // Emulate a failure during player release.
+                throw new RuntimeException();
+              }
+            });
+    ExoPlayer player =
+        new TestExoPlayerBuilder(ApplicationProvider.getApplicationContext())
+            .setLoadControl(loadControl)
+            .build();
+    player.addListener(listener);
+    // Ensure load control has not thrown the exception yet.
+    verify(loadControl, never()).onReleased();
+
+    player.release();
+    ShadowLooper.idleMainLooper();
+
+    // Verify load control threw the exception.
+    verify(loadControl).onReleased();
+    verify(listener, never()).onPlayerError(any());
+  }
+
   @Test
   public void releaseAfterVolumeChanges_triggerPendingDeviceVolumeEventsInListener() {
     ExoPlayer player =