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
square · Nov 25, 2019 · 2860a3c · 2860a3c
1 parent 0911423
commit 2860a3c
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 3 deletions.
diff --git 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
@@ -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)
+  }
 }