From 69b4aa9e36b854ee34a2b104d9569ab7d0767ea7 Mon Sep 17 00:00:00 2001 From: olly Date: Thu, 31 May 2018 09:52:18 -0700 Subject: [PATCH] Explicitly null out LoadTask.callback on release As highlighted by the ref'd issue, we can end up with memory leaks if Loadable.load implementations take a long time to return upon cancelation. This change cuts off one of the two problematic reference chains. This doesn't do much about the ref'd issue, since there's a second reference chain that's much harder to deal with: Thread->LoadTask->loadable. But since it's easy just to cut this one off, I figure it makes sense to do so. Issue: #4249 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=198735386 --- .../com/google/android/exoplayer2/upstream/Loader.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) 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 074fc095ea..0f3198d06c 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 @@ -250,11 +250,12 @@ public final class Loader implements LoaderErrorThrower { private static final int MSG_IO_EXCEPTION = 3; private static final int MSG_FATAL_ERROR = 4; - private final T loadable; - private final Loader.Callback callback; public final int defaultMinRetryCount; + + private final T loadable; private final long startTimeMs; + private @Nullable Loader.Callback callback; private IOException currentError; private int errorCount; @@ -304,6 +305,11 @@ public final class Loader implements LoaderErrorThrower { finish(); long nowMs = SystemClock.elapsedRealtime(); callback.onLoadCanceled(loadable, nowMs, nowMs - startTimeMs, true); + // If loading, this task will be referenced from a GC root (the loading thread) until + // cancellation completes. The time taken for cancellation to complete depends on the + // implementation of the Loadable that the task is loading. We null the callback reference + // here so that it doesn't prevent garbage collection whilst cancellation is ongoing. + callback = null; } }