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
This commit is contained in:
olly 2018-05-31 09:52:18 -07:00 committed by Oliver Woodman
parent 27f009d239
commit 69b4aa9e36

View File

@ -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;
}
}