Commit 4ecce980 by olly Committed by Oliver Woodman

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
parent cd65cc85
......@@ -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<T> callback;
public final int defaultMinRetryCount;
private final T loadable;
private final long startTimeMs;
private @Nullable Loader.Callback<T> 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;
}
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment