Release Extractors on the loading thread

Releasing the player released the internal playback thread once renderers were
released. Releasing a MediaPeriod queued a Loader.ReleaseTask on the loading
thread which would post back to the playback thread. If the playback thread had
been quit by the time this happened, the release task wouldn't be run.

Release on the loading thread instead of the playback thread. This avoids
needing to block releasing the player until the loading threads have ended, and
ensures that release tasks will run eventually. As part of this change,
ExtractorMediaPeriod's call to Extractor.release will now run on the loading
thread (which means that all Extractor methods are called on that thread) and
other cleanup in ReleaseCallback will run on the loading thread instead of the
playback thread.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=185651320

RELEASENOTES.md

@@ -99,6 +99,9 @@
   ([#3796](https://github.com/google/ExoPlayer/issues/3796)).
 * Check `sys.display-size` on Philips ATVs
   ([#3807](https://github.com/google/ExoPlayer/issues/3807)).
+* Release `Extractor`s on the loading thread to avoid potentially leaking
+  resources when the playback thread has quit by the time the loading task has
+  completed.
 
 ### 2.6.1 ###
 

library/core/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java

@@ -179,24 +179,24 @@ import java.util.Arrays;
   }
 
   public void release() {
-    boolean releasedSynchronously = loader.release(this);
-    if (prepared && !releasedSynchronously) {
+    if (prepared) {
       // Discard as much as we can synchronously. We only do this if we're prepared, since otherwise
       // sampleQueues may still be being modified by the loading thread.
       for (SampleQueue sampleQueue : sampleQueues) {
         sampleQueue.discardToEnd();
       }
     }
+    loader.release(this);
     handler.removeCallbacksAndMessages(null);
     released = true;
   }
 
   @Override
   public void onLoaderReleased() {
-    extractorHolder.release();
     for (SampleQueue sampleQueue : sampleQueues) {
       sampleQueue.reset();
     }
+    extractorHolder.release();
   }
 
   @Override

library/core/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkSampleStream.java

@@ -73,7 +73,7 @@ public class ChunkSampleStream<T extends ChunkSource> implements SampleStream, S
   private final BaseMediaChunkOutput mediaChunkOutput;
 
   private Format primaryDownstreamTrackFormat;
-  private ReleaseCallback<T> releaseCallback;
+  private @Nullable ReleaseCallback<T> releaseCallback;
   private long pendingResetPositionUs;
   private long lastSeekPositionUs;
   /* package */ long decodeOnlyUntilPositionUs;
@@ -306,20 +306,17 @@ public class ChunkSampleStream<T extends ChunkSource> implements SampleStream, S
    * <p>This method should be called when the stream is no longer required. Either this method or
    * {@link #release()} can be used to release this stream.
    *
-   * @param callback A callback to be called when the release ends. Will be called synchronously
-   *     from this method if no load is in progress, or asynchronously once the load has been
-   *     canceled otherwise.
+   * @param callback An optional callback to be called on the loading thread once the loader has
+   *     been released.
    */
   public void release(@Nullable ReleaseCallback<T> callback) {
     this.releaseCallback = callback;
-    boolean releasedSynchronously = loader.release(this);
-    if (!releasedSynchronously) {
-      // Discard as much as we can synchronously.
-      primarySampleQueue.discardToEnd();
-      for (SampleQueue embeddedSampleQueue : embeddedSampleQueues) {
-        embeddedSampleQueue.discardToEnd();
-      }
+    // Discard as much as we can synchronously.
+    primarySampleQueue.discardToEnd();
+    for (SampleQueue embeddedSampleQueue : embeddedSampleQueues) {
+      embeddedSampleQueue.discardToEnd();
     }
+    loader.release(this);
   }
 
   @Override

library/core/src/main/java/com/google/android/exoplayer2/upstream/Loader.java

@@ -20,6 +20,7 @@ import android.os.Handler;
 import android.os.Looper;
 import android.os.Message;
 import android.os.SystemClock;
+import android.support.annotation.Nullable;
 import android.util.Log;
 import com.google.android.exoplayer2.util.Assertions;
 import com.google.android.exoplayer2.util.TraceUtil;
@@ -197,25 +198,17 @@ public final class Loader implements LoaderErrorThrower {
    * Releases the {@link Loader}. This method should be called when the {@link Loader} is no longer
    * required.
    *
-   * @param callback A callback to be called when the release ends. Will be called synchronously
-   *     from this method if no load is in progress, or asynchronously once the load has been
-   *     canceled otherwise. May be null.
-   * @return True if {@code callback} was called synchronously. False if it will be called
-   *     asynchronously or if {@code callback} is null.
+   * @param callback An optional callback to be called on the loading thread once the loader has
+   *     been released.
    */
-  public boolean release(ReleaseCallback callback) {
-    boolean callbackInvoked = false;
+  public void release(@Nullable ReleaseCallback callback) {
     if (currentTask != null) {
       currentTask.cancel(true);
-      if (callback != null) {
-        downloadExecutorService.execute(new ReleaseTask(callback));
-      }
-    } else if (callback != null) {
-      callback.onLoaderReleased();
-      callbackInvoked = true;
+    }
+    if (callback != null) {
+      downloadExecutorService.execute(new ReleaseTask(callback));
     }
     downloadExecutorService.shutdown();
-    return callbackInvoked;
   }
 
   // LoaderErrorThrower implementation.
@@ -419,7 +412,7 @@ public final class Loader implements LoaderErrorThrower {
 
   }
 
-  private static final class ReleaseTask extends Handler implements Runnable {
+  private static final class ReleaseTask implements Runnable {
 
     private final ReleaseCallback callback;
 
@@ -429,13 +422,6 @@ public final class Loader implements LoaderErrorThrower {
 
     @Override
     public void run() {
-      if (getLooper().getThread().isAlive()) {
-        sendEmptyMessage(0);
-      }
-    }
-
-    @Override
-    public void handleMessage(Message msg) {
       callback.onLoaderReleased();
     }
 

library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DashMediaPeriod.java

@@ -153,7 +153,7 @@ import java.util.List;
   // ChunkSampleStream.ReleaseCallback implementation.
 
   @Override
-  public void onSampleStreamReleased(ChunkSampleStream<DashChunkSource> stream) {
+  public synchronized void onSampleStreamReleased(ChunkSampleStream<DashChunkSource> stream) {
     PlayerTrackEmsgHandler trackEmsgHandler = trackEmsgHandlerBySampleStream.remove(stream);
     if (trackEmsgHandler != null) {
       trackEmsgHandler.release();
@@ -565,7 +565,10 @@ import java.util.List;
             positionUs,
             minLoadableRetryCount,
             eventDispatcher);
-    trackEmsgHandlerBySampleStream.put(stream, trackPlayerEmsgHandler);
+    synchronized (this) {
+      // The map is also accessed on the loading thread so synchronize access.
+      trackEmsgHandlerBySampleStream.put(stream, trackPlayerEmsgHandler);
+    }
     return stream;
   }
 

library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java

@@ -386,14 +386,14 @@ import java.util.Arrays;
   }
 
   public void release() {
-    boolean releasedSynchronously = loader.release(this);
-    if (prepared && !releasedSynchronously) {
+    if (prepared) {
       // Discard as much as we can synchronously. We only do this if we're prepared, since otherwise
       // sampleQueues may still be being modified by the loading thread.
       for (SampleQueue sampleQueue : sampleQueues) {
         sampleQueue.discardToEnd();
       }
     }
+    loader.release(this);
     handler.removeCallbacksAndMessages(null);
     released = true;
   }