Skip to content

Commit

Permalink
Fix a rare LottieTask leak (#1986)
Browse files Browse the repository at this point in the history
Also did some routine code cleanup in surrounding code.

Fixes #1971
  • Loading branch information
gpeal authored Jan 10, 2022
1 parent b1b58d2 commit 0d1c96f
Showing 2 changed files with 50 additions and 85 deletions.
101 changes: 33 additions & 68 deletions lottie/src/main/java/com/airbnb/lottie/LottieCompositionFactory.java
Original file line number Diff line number Diff line change
@@ -29,6 +29,7 @@
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.Callable;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.zip.ZipEntry;
import java.util.zip.ZipInputStream;

@@ -92,15 +93,12 @@ public static LottieTask<LottieComposition> fromUrl(final Context context, final
* might need an animation in the future.
*/
public static LottieTask<LottieComposition> fromUrl(final Context context, final String url, @Nullable final String cacheKey) {
return cache(cacheKey, new Callable<LottieResult<LottieComposition>>() {
@Override
public LottieResult<LottieComposition> call() {
LottieResult<LottieComposition> result = L.networkFetcher(context).fetchSync(url, cacheKey);
if (cacheKey != null && result.getValue() != null) {
LottieCompositionCache.getInstance().put(cacheKey, result.getValue());
}
return result;
return cache(cacheKey, () -> {
LottieResult<LottieComposition> result = L.networkFetcher(context).fetchSync(url, cacheKey);
if (cacheKey != null && result.getValue() != null) {
LottieCompositionCache.getInstance().put(cacheKey, result.getValue());
}
return result;
});
}

@@ -155,12 +153,7 @@ public static LottieTask<LottieComposition> fromAsset(Context context, final Str
public static LottieTask<LottieComposition> fromAsset(Context context, final String fileName, @Nullable final String cacheKey) {
// Prevent accidentally leaking an Activity.
final Context appContext = context.getApplicationContext();
return cache(cacheKey, new Callable<LottieResult<LottieComposition>>() {
@Override
public LottieResult<LottieComposition> call() {
return fromAssetSync(appContext, fileName, cacheKey);
}
});
return cache(cacheKey, () -> fromAssetSync(appContext, fileName, cacheKey));
}

/**
@@ -226,13 +219,10 @@ public static LottieTask<LottieComposition> fromRawRes(Context context, @RawRes
// Prevent accidentally leaking an Activity.
final WeakReference<Context> contextRef = new WeakReference<>(context);
final Context appContext = context.getApplicationContext();
return cache(cacheKey, new Callable<LottieResult<LottieComposition>>() {
@Override
public LottieResult<LottieComposition> call() {
@Nullable Context originalContext = contextRef.get();
Context context = originalContext != null ? originalContext : appContext;
return fromRawResSync(context, rawRes, cacheKey);
}
return cache(cacheKey, () -> {
@Nullable Context originalContext = contextRef.get();
Context context1 = originalContext != null ? originalContext : appContext;
return fromRawResSync(context1, rawRes, cacheKey);
});
}

@@ -290,12 +280,7 @@ private static boolean isNightMode(Context context) {
* @see #fromJsonInputStreamSync(InputStream, String, boolean)
*/
public static LottieTask<LottieComposition> fromJsonInputStream(final InputStream stream, @Nullable final String cacheKey) {
return cache(cacheKey, new Callable<LottieResult<LottieComposition>>() {
@Override
public LottieResult<LottieComposition> call() {
return fromJsonInputStreamSync(stream, cacheKey);
}
});
return cache(cacheKey, () -> fromJsonInputStreamSync(stream, cacheKey));
}

/**
@@ -324,12 +309,9 @@ private static LottieResult<LottieComposition> fromJsonInputStreamSync(InputStre
*/
@Deprecated
public static LottieTask<LottieComposition> fromJson(final JSONObject json, @Nullable final String cacheKey) {
return cache(cacheKey, new Callable<LottieResult<LottieComposition>>() {
@Override
public LottieResult<LottieComposition> call() {
//noinspection deprecation
return fromJsonSync(json, cacheKey);
}
return cache(cacheKey, () -> {
//noinspection deprecation
return fromJsonSync(json, cacheKey);
});
}

@@ -348,12 +330,7 @@ public static LottieResult<LottieComposition> fromJsonSync(JSONObject json, @Nul
* @see #fromJsonStringSync(String, String)
*/
public static LottieTask<LottieComposition> fromJsonString(final String json, @Nullable final String cacheKey) {
return cache(cacheKey, new Callable<LottieResult<LottieComposition>>() {
@Override
public LottieResult<LottieComposition> call() {
return fromJsonStringSync(json, cacheKey);
}
});
return cache(cacheKey, () -> fromJsonStringSync(json, cacheKey));
}

/**
@@ -369,12 +346,7 @@ public static LottieResult<LottieComposition> fromJsonStringSync(String json, @N
}

public static LottieTask<LottieComposition> fromJsonReader(final JsonReader reader, @Nullable final String cacheKey) {
return cache(cacheKey, new Callable<LottieResult<LottieComposition>>() {
@Override
public LottieResult<LottieComposition> call() {
return fromJsonReaderSync(reader, cacheKey);
}
});
return cache(cacheKey, () -> fromJsonReaderSync(reader, cacheKey));
}


@@ -403,12 +375,7 @@ private static LottieResult<LottieComposition> fromJsonReaderSyncInternal(


public static LottieTask<LottieComposition> fromZipStream(final ZipInputStream inputStream, @Nullable final String cacheKey) {
return cache(cacheKey, new Callable<LottieResult<LottieComposition>>() {
@Override
public LottieResult<LottieComposition> call() {
return fromZipStreamSync(inputStream, cacheKey);
}
});
return cache(cacheKey, () -> fromZipStreamSync(inputStream, cacheKey));
}

/**
@@ -521,32 +488,30 @@ private static LottieTask<LottieComposition> cache(
@Nullable final String cacheKey, Callable<LottieResult<LottieComposition>> callable) {
final LottieComposition cachedComposition = cacheKey == null ? null : LottieCompositionCache.getInstance().get(cacheKey);
if (cachedComposition != null) {
return new LottieTask<>(new Callable<LottieResult<LottieComposition>>() {
@Override
public LottieResult<LottieComposition> call() {
return new LottieResult<>(cachedComposition);
}
});
return new LottieTask<>(() -> new LottieResult<>(cachedComposition));
}
if (cacheKey != null && taskCache.containsKey(cacheKey)) {
return taskCache.get(cacheKey);
}

LottieTask<LottieComposition> task = new LottieTask<>(callable);
if (cacheKey != null) {
task.addListener(new LottieListener<LottieComposition>() {
@Override
public void onResult(LottieComposition result) {
taskCache.remove(cacheKey);
}
AtomicBoolean resultAlreadyCalled = new AtomicBoolean(false);
task.addListener(result -> {
taskCache.remove(cacheKey);
resultAlreadyCalled.set(true);
});
task.addFailureListener(new LottieListener<Throwable>() {
@Override
public void onResult(Throwable result) {
taskCache.remove(cacheKey);
}
task.addFailureListener(result -> {
taskCache.remove(cacheKey);
resultAlreadyCalled.set(true);
});
taskCache.put(cacheKey, task);
// It is technically possible for the task to finish and for the listeners to get called
// before this code runs. If this happens, the task will be put in taskCache but never removed.
// This would require this thread to be sleeping at exactly this point in the code
// for long enough for the task to finish and call the listeners. Unlikely but not impossible.
if (!resultAlreadyCalled.get()) {
taskCache.put(cacheKey, task);
}
}
return task;
}
34 changes: 17 additions & 17 deletions lottie/src/main/java/com/airbnb/lottie/LottieTask.java
Original file line number Diff line number Diff line change
@@ -25,7 +25,7 @@
* <p>
* A task will produce a single result or a single failure.
*/
public class LottieTask<T> {
@SuppressWarnings("UnusedReturnValue") public class LottieTask<T> {

/**
* Set this to change the executor that LottieTasks are run on. This will be the executor that composition parsing and url
@@ -56,7 +56,7 @@ public LottieTask(Callable<LottieResult<T>> runnable) {
try {
setResult(runnable.call());
} catch (Throwable e) {
setResult(new LottieResult<T>(e));
setResult(new LottieResult<>(e));
}
} else {
EXECUTOR.execute(new LottieFutureTask(runnable));
@@ -77,6 +77,7 @@ private void setResult(@Nullable LottieResult<T> result) {
* @return the task for call chaining.
*/
public synchronized LottieTask<T> addListener(LottieListener<T> listener) {
LottieResult<T> result = this.result;
if (result != null && result.getValue() != null) {
listener.onResult(result.getValue());
}
@@ -87,7 +88,7 @@ public synchronized LottieTask<T> addListener(LottieListener<T> listener) {

/**
* Remove a given task listener. The task will continue to execute so you can re-add
* a listener if neccesary.
* a listener if necessary.
*
* @return the task for call chaining.
*/
@@ -103,6 +104,7 @@ public synchronized LottieTask<T> removeListener(LottieListener<T> listener) {
* @return the task for call chaining.
*/
public synchronized LottieTask<T> addFailureListener(LottieListener<Throwable> listener) {
LottieResult<T> result = this.result;
if (result != null && result.getException() != null) {
listener.onResult(result.getException());
}
@@ -113,7 +115,7 @@ public synchronized LottieTask<T> addFailureListener(LottieListener<Throwable> l

/**
* Remove a given task failure listener. The task will continue to execute so you can re-add
* a listener if neccesary.
* a listener if necessary.
*
* @return the task for call chaining.
*/
@@ -124,18 +126,16 @@ public synchronized LottieTask<T> removeFailureListener(LottieListener<Throwable

private void notifyListeners() {
// Listeners should be called on the main thread.
handler.post(new Runnable() {
@Override public void run() {
if (result == null) {
return;
}
// Local reference in case it gets set on a background thread.
LottieResult<T> result = LottieTask.this.result;
if (result.getValue() != null) {
notifySuccessListeners(result.getValue());
} else {
notifyFailureListeners(result.getException());
}
handler.post(() -> {
// Local reference in case it gets set on a background thread.
LottieResult<T> result = LottieTask.this.result;
if (result == null) {
return;
}
if (result.getValue() != null) {
notifySuccessListeners(result.getValue());
} else {
notifyFailureListeners(result.getException());
}
});
}
@@ -178,7 +178,7 @@ protected void done() {
try {
setResult(get());
} catch (InterruptedException | ExecutionException e) {
setResult(new LottieResult<T>(e));
setResult(new LottieResult<>(e));
}
}
}

0 comments on commit 0d1c96f

Please sign in to comment.