diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 34de0baca8..c43ad5c19e 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -101,6 +101,9 @@ ([#2643](https://github.com/google/ExoPlayer/issues/2643)). * 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 ### diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java index f4f59115eb..c771188e3b 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java +++ b/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 diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkSampleStream.java b/library/core/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkSampleStream.java index e0c5d35996..7096c84c5e 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkSampleStream.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkSampleStream.java @@ -73,7 +73,7 @@ public class ChunkSampleStream implements SampleStream, S private final BaseMediaChunkOutput mediaChunkOutput; private Format primaryDownstreamTrackFormat; - private ReleaseCallback releaseCallback; + private @Nullable ReleaseCallback releaseCallback; private long pendingResetPositionUs; private long lastSeekPositionUs; /* package */ long decodeOnlyUntilPositionUs; @@ -306,20 +306,17 @@ public class ChunkSampleStream implements SampleStream, S *

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 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 diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/Loader.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/Loader.java index 9e495f42bf..a118f10784 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/Loader.java +++ b/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(); } diff --git a/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DashMediaPeriod.java b/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DashMediaPeriod.java index d6d6ca821c..00baf15228 100644 --- a/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DashMediaPeriod.java +++ b/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 stream) { + public synchronized void onSampleStreamReleased(ChunkSampleStream 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; } diff --git a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java index 508f2f0f2f..f027ba5b05 100644 --- a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java +++ b/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; }