From ad720b1f631ce242e00b5f0284e13dc62d9dc1e7 Mon Sep 17 00:00:00 2001
From: Alexander Oprisnik <oprisnik@fb.com>
Date: Fri, 17 Jan 2020 12:28:44 -0800
Subject: [PATCH] Fix CloseableReference leaks in Flipper plugin

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
---
 .../plugins/fresco/FrescoFlipperPlugin.java   | 40 ++++++++++++-------
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/android/plugins/fresco/src/main/java/com/facebook/flipper/plugins/fresco/FrescoFlipperPlugin.java b/android/plugins/fresco/src/main/java/com/facebook/flipper/plugins/fresco/FrescoFlipperPlugin.java
index fdcf91ed3..66758b284 100644
--- a/android/plugins/fresco/src/main/java/com/facebook/flipper/plugins/fresco/FrescoFlipperPlugin.java
+++ b/android/plugins/fresco/src/main/java/com/facebook/flipper/plugins/fresco/FrescoFlipperPlugin.java
@@ -174,8 +174,13 @@ public class FrescoFlipperPlugin extends BufferingFlipperPlugin
                         imagePipelineFactory.getEncodedCountingMemoryCache())
                     .dumpCacheContent();
 
-            responder.success(getImageList(bitmapMemoryCache, encodedMemoryCache));
-            mPerfLogger.endMarker("Sonar.Fresco.listImages");
+            try {
+              responder.success(getImageList(bitmapMemoryCache, encodedMemoryCache));
+              mPerfLogger.endMarker("Sonar.Fresco.listImages");
+            } finally {
+              bitmapMemoryCache.release();
+              encodedMemoryCache.release();
+            }
           }
         });
 
@@ -202,20 +207,27 @@ public class FrescoFlipperPlugin extends BufferingFlipperPlugin
             // load from bitmap cache
             CloseableReference<CloseableImage> ref =
                 imagePipelineFactory.getBitmapCountingMemoryCache().get(cacheKey);
-            if (ref != null) {
-              loadFromBitmapCache(ref, imageId, cacheKey, responder);
-            } else {
-              // try to load from encoded cache
-              CloseableReference<PooledByteBuffer> encodedRef =
-                  imagePipelineFactory.getEncodedCountingMemoryCache().get(cacheKey);
-
-              if (encodedRef != null) {
-                loadFromEncodedCache(encodedRef, imageId, cacheKey, responder);
+            try {
+              if (ref != null) {
+                loadFromBitmapCache(ref, imageId, cacheKey, responder);
               } else {
-                respondError(responder, "no bitmap withId=" + imageId);
-                mPerfLogger.cancelMarker("Sonar.Fresco.getImage");
-                return;
+                // try to load from encoded cache
+                CloseableReference<PooledByteBuffer> encodedRef =
+                    imagePipelineFactory.getEncodedCountingMemoryCache().get(cacheKey);
+                try {
+                  if (encodedRef != null) {
+                    loadFromEncodedCache(encodedRef, imageId, cacheKey, responder);
+                  } else {
+                    respondError(responder, "no bitmap withId=" + imageId);
+                    mPerfLogger.cancelMarker("Sonar.Fresco.getImage");
+                    return;
+                  }
+                } finally {
+                  CloseableReference.closeSafely(encodedRef);
+                }
               }
+            } finally {
+              CloseableReference.closeSafely(ref);
             }
 
             mPerfLogger.endMarker("Sonar.Fresco.getImage");