From 2860a3cf323a3cf2bea457c4ecead80432ba43c5 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Ricau <py@squareup.com>
Date: Mon, 25 Nov 2019 09:30:41 -0800
Subject: [PATCH] Fix bug in handling JNI roots

* When adding support for native reference leaks (879ab7c) there was a missing bracket in an if/else branch that led to all native gc roots being ignored.

Fixes #1634
---
 shark/src/main/java/shark/internal/PathFinder.kt | 7 ++++---
 shark/src/test/java/shark/HeapAnalyzerTest.kt    | 9 +++++++++
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/shark/src/main/java/shark/internal/PathFinder.kt b/shark/src/main/java/shark/internal/PathFinder.kt
index ff3a5b1e87..dd4c48252f 100644
--- a/shark/src/main/java/shark/internal/PathFinder.kt
+++ b/shark/src/main/java/shark/internal/PathFinder.kt
@@ -295,10 +295,11 @@ internal class PathFinder(
             is HeapPrimitiveArray -> jniGlobalReferenceMatchers[objectRecord.arrayClassName]
           }
           if (referenceMatcher !is IgnoredReferenceMatcher) {
-            if (referenceMatcher is LibraryLeakReferenceMatcher)
+            if (referenceMatcher is LibraryLeakReferenceMatcher) {
               enqueue(LibraryLeakRootNode(gcRoot.id, gcRoot, referenceMatcher))
-          } else {
-            enqueue(NormalRootNode(gcRoot.id, gcRoot))
+            } else {
+              enqueue(NormalRootNode(gcRoot.id, gcRoot))
+            }
           }
         }
         else -> enqueue(NormalRootNode(gcRoot.id, gcRoot))
diff --git a/shark/src/test/java/shark/HeapAnalyzerTest.kt b/shark/src/test/java/shark/HeapAnalyzerTest.kt
index f0b7a90163..027edeb6e8 100644
--- a/shark/src/test/java/shark/HeapAnalyzerTest.kt
+++ b/shark/src/test/java/shark/HeapAnalyzerTest.kt
@@ -5,6 +5,7 @@ import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
 import org.junit.rules.TemporaryFolder
+import shark.GcRoot.JniGlobal
 import shark.GcRoot.ThreadObject
 import shark.LeakTraceElement.Type.LOCAL
 import shark.LeakTraceElement.Type.STATIC_FIELD
@@ -146,4 +147,12 @@ class HeapAnalyzerTest {
     assertThat(leak.leakTrace.elements[1].className).isEqualTo("Leaking")
   }
 
+  @Test fun nativeGlobalVariableApplicationLeak() {
+    hprofFile.dump {
+      gcRoot(JniGlobal(id = "Leaking".watchedInstance {}.value, jniGlobalRefId = 42))
+    }
+
+    val leaks = hprofFile.checkForLeaks<HeapAnalysisSuccess>()
+    assertThat(leaks.applicationLeaks).hasSize(1)
+  }
 }
\ No newline at end of file