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 4ff58b108c..64cafb42b9 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 @@ -32,6 +32,7 @@ import java.lang.annotation.Documented; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.util.concurrent.ExecutorService; +import java.util.concurrent.atomic.AtomicBoolean; /** * Manages the background loading of {@link Loadable}s. @@ -56,6 +57,21 @@ public final class Loader implements LoaderErrorThrower { /** * Cancels the load. + * + *

Loadable implementations should ensure that a currently executing {@link #load()} call + * will exit reasonably quickly after this method is called. The {@link #load()} call may exit + * either by returning or by throwing an {@link IOException}. + * + *

If there is a currently executing {@link #load()} call, then the thread on which that call + * is being made will be interrupted immediately after the call to this method. Hence + * implementations do not need to (and should not attempt to) interrupt the loading thread + * themselves. + * + *

Although the loading thread will be interrupted, Loadable implementations should not use + * the interrupted status of the loading thread in {@link #load()} to determine whether the load + * has been canceled. This approach is not robust [Internal ref: b/79223737]. Instead, + * implementations should use their own flag to signal cancelation (for example, using {@link + * AtomicBoolean}). */ void cancelLoad(); @@ -307,10 +323,9 @@ public final class Loader implements LoaderErrorThrower { private static final String TAG = "LoadTask"; private static final int MSG_START = 0; - private static final int MSG_CANCEL = 1; - private static final int MSG_END_OF_SOURCE = 2; - private static final int MSG_IO_EXCEPTION = 3; - private static final int MSG_FATAL_ERROR = 4; + private static final int MSG_FINISH = 1; + private static final int MSG_IO_EXCEPTION = 2; + private static final int MSG_FATAL_ERROR = 3; public final int defaultMinRetryCount; @@ -321,8 +336,8 @@ public final class Loader implements LoaderErrorThrower { @Nullable private IOException currentError; private int errorCount; - @Nullable private volatile Thread executorThread; - private volatile boolean canceled; + @Nullable private Thread executorThread; + private boolean canceled; private volatile boolean released; public LoadTask(Looper looper, T loadable, Loader.Callback callback, @@ -354,16 +369,21 @@ public final class Loader implements LoaderErrorThrower { this.released = released; currentError = null; if (hasMessages(MSG_START)) { + // The task has not been given to the executor yet. + canceled = true; removeMessages(MSG_START); if (!released) { - sendEmptyMessage(MSG_CANCEL); + sendEmptyMessage(MSG_FINISH); } } else { - canceled = true; - loadable.cancelLoad(); - @Nullable Thread executorThread = this.executorThread; - if (executorThread != null) { - executorThread.interrupt(); + // The task has been given to the executor. + synchronized (this) { + canceled = true; + loadable.cancelLoad(); + @Nullable Thread executorThread = this.executorThread; + if (executorThread != null) { + executorThread.interrupt(); + } } } if (released) { @@ -382,8 +402,12 @@ public final class Loader implements LoaderErrorThrower { @Override public void run() { try { - executorThread = Thread.currentThread(); - if (!canceled) { + boolean shouldLoad; + synchronized (this) { + shouldLoad = !canceled; + executorThread = Thread.currentThread(); + } + if (shouldLoad) { TraceUtil.beginSection("load:" + loadable.getClass().getSimpleName()); try { loadable.load(); @@ -391,8 +415,13 @@ public final class Loader implements LoaderErrorThrower { TraceUtil.endSection(); } } + synchronized (this) { + executorThread = null; + // Clear the interrupted flag if set, to avoid it leaking into a subsequent task. + Thread.interrupted(); + } if (!released) { - sendEmptyMessage(MSG_END_OF_SOURCE); + sendEmptyMessage(MSG_FINISH); } } catch (IOException e) { if (!released) { @@ -445,10 +474,7 @@ public final class Loader implements LoaderErrorThrower { return; } switch (msg.what) { - case MSG_CANCEL: - callback.onLoadCanceled(loadable, nowMs, durationMs, false); - break; - case MSG_END_OF_SOURCE: + case MSG_FINISH: try { callback.onLoadCompleted(loadable, nowMs, durationMs); } catch (RuntimeException e) {