jens/flipper
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix CloseableReference leaks in Flipper plugin

Browse Source 
Summary:
When debugging closeable reference leaks, I found that the Flipper plugin doesn't properly close one:

```
2020-01-17 10:45:29.346 27038-27053/com.facebook.wakizashi D/YOLO: LEAK!!:
    java.lang.Throwable
        at com.facebook.common.references.CloseableReference.<init>(CloseableReference.java:158)
        at com.facebook.common.references.DefaultCloseableReference.<init>(DefaultCloseableReference.java:29)
        at com.facebook.common.references.CloseableReference.of(CloseableReference.java:237)
        at com.facebook.common.references.CloseableReference.of(CloseableReference.java:203)
        at com.facebook.common.references.CloseableReference.of(CloseableReference.java:176)
        at com.facebook.imagepipeline.cache.CountingMemoryCache.newClientReference(CountingMemoryCache.java:221)
        at com.facebook.imagepipeline.cache.CountingMemoryCache.get(CountingMemoryCache.java:209)
        at com.facebook.flipper.plugins.fresco.FrescoFlipperPlugin$4.onReceive(FrescoFlipperPlugin.java:204)
        at com.facebook.flipper.android.EventBase.loopForever(Native Method)
        at com.facebook.flipper.android.FlipperThread.run(FlipperThread.java:31)
```

Second leak:
```
2020-01-17 11:04:16.503 28855-28869/com.facebook.wakizashi D/YOLO: LEAK!!:
    java.lang.Throwable
        at com.facebook.common.references.CloseableReference.<init>(CloseableReference.java:147)
        at com.facebook.common.references.DefaultCloseableReference.<init>(DefaultCloseableReference.java:21)
        at com.facebook.common.references.DefaultCloseableReference.clone(DefaultCloseableReference.java:35)
        at com.facebook.common.references.CloseableReference.cloneOrNull(CloseableReference.java:258)
        at com.facebook.common.references.CloseableReference.cloneOrNull(CloseableReference.java:326)
        at com.facebook.imagepipeline.cache.CountingMemoryCacheInspector$DumpInfoEntry.<init>(CountingMemoryCacheInspector.java:32)
        at com.facebook.imagepipeline.cache.CountingMemoryCacheInspector.dumpCacheContent(CountingMemoryCacheInspector.java:101)
        at com.facebook.flipper.plugins.fresco.FrescoFlipperPlugin$3.onReceive(FrescoFlipperPlugin.java:171)
        at com.facebook.flipper.android.EventBase.loopForever(Native Method)
        at com.facebook.flipper.android.FlipperThread.run(FlipperThread.java:31)
```

Reviewed By: passy

Differential Revision: D19445902

fbshipit-source-id: 12a513d9e34fcac0d546a4eac55932956e4e4d5b
This commit is contained in:
Alexander Oprisnik
2020-01-17 12:28:44 -08:00
committed by Facebook Github Bot
parent 1b7a30ae6c
commit ad720b1f63
1 changed files with 26 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
android/plugins/fresco/src/main/java/com/facebook/flipper/plugins/fresco/FrescoFlipperPlugin.java
View File

@@ -174,8 +174,13 @@ public class FrescoFlipperPlugin extends BufferingFlipperPlugin
imagePipelineFactory.getEncodedCountingMemoryCache()) imagePipelineFactory.getEncodedCountingMemoryCache())
.dumpCacheContent(); .dumpCacheContent();
try {
responder.success(getImageList(bitmapMemoryCache, encodedMemoryCache)); responder.success(getImageList(bitmapMemoryCache, encodedMemoryCache));
mPerfLogger.endMarker("Sonar.Fresco.listImages"); mPerfLogger.endMarker("Sonar.Fresco.listImages");
} finally {
bitmapMemoryCache.release();
encodedMemoryCache.release();
}
} }
}); });
@@ -202,13 +207,14 @@ public class FrescoFlipperPlugin extends BufferingFlipperPlugin
// load from bitmap cache // load from bitmap cache
CloseableReference<CloseableImage> ref = CloseableReference<CloseableImage> ref =
imagePipelineFactory.getBitmapCountingMemoryCache().get(cacheKey); imagePipelineFactory.getBitmapCountingMemoryCache().get(cacheKey);
try {
if (ref != null) { if (ref != null) {
loadFromBitmapCache(ref, imageId, cacheKey, responder); loadFromBitmapCache(ref, imageId, cacheKey, responder);
} else { } else {
// try to load from encoded cache // try to load from encoded cache
CloseableReference<PooledByteBuffer> encodedRef = CloseableReference<PooledByteBuffer> encodedRef =
imagePipelineFactory.getEncodedCountingMemoryCache().get(cacheKey); imagePipelineFactory.getEncodedCountingMemoryCache().get(cacheKey);
try {
if (encodedRef != null) { if (encodedRef != null) {
loadFromEncodedCache(encodedRef, imageId, cacheKey, responder); loadFromEncodedCache(encodedRef, imageId, cacheKey, responder);
} else { } else {
@@ -216,6 +222,12 @@ public class FrescoFlipperPlugin extends BufferingFlipperPlugin
mPerfLogger.cancelMarker("Sonar.Fresco.getImage"); mPerfLogger.cancelMarker("Sonar.Fresco.getImage");
return; return;
} }
} finally {
CloseableReference.closeSafely(encodedRef);
}
}
} finally {
CloseableReference.closeSafely(ref);
} }
mPerfLogger.endMarker("Sonar.Fresco.getImage"); mPerfLogger.endMarker("Sonar.Fresco.getImage");