Ref patterns for native global variables

shark-android/src/main/java/shark/AndroidReferenceMatchers.kt

@@ -19,6 +19,7 @@ import shark.AndroidReferenceMatchers.Companion.appDefaults
 import shark.AndroidReferenceMatchers.Companion.buildKnownReferences
 import shark.ReferencePattern.InstanceFieldPattern
 import shark.ReferencePattern.JavaLocalPattern
+import shark.ReferencePattern.NativeGlobalVariablePattern
 import shark.ReferencePattern.StaticFieldPattern
 import java.lang.ref.PhantomReference
 import java.lang.ref.SoftReference
@@ -667,6 +668,28 @@ enum class AndroidReferenceMatchers {
     }
   },
 
+  CONTROLLED_INPUT_CONNECTION_WRAPPER {
+    override fun add(references: MutableList<ReferenceMatcher>) {
+      references += nativeGlobalVariableLeak(
+          "android.view.inputmethod.InputMethodManager\$ControlledInputConnectionWrapper",
+          description = """
+        ControlledInputConnectionWrapper is held by a global variable in native code. 
+      """.trimIndent()
+      )
+    }
+  },
+
+  TOAST_TN {
+    override fun add(references: MutableList<ReferenceMatcher>) {
+      references += nativeGlobalVariableLeak(
+          "android.widget.Toast\$TN",
+          description = """
+        Toast.TN is held by a global variable in native code due to an IPC call to show the toast.
+      """.trimIndent()
+      )
+    }
+  },
+
   // ######## Manufacturer specific known leaks ########
 
   // SAMSUNG
@@ -709,35 +732,18 @@ enum class AndroidReferenceMatchers {
     override fun add(
       references: MutableList<ReferenceMatcher>
     ) {
-      references += instanceFieldLeak(
-          "com.samsung.android.content.clipboard.SemClipboardManager",
-          "mContext"
-          ,
-          description =
-          "SemClipboardManager is held in memory by an anonymous inner class" +
-              " implementation of android.os.Binder, thereby leaking an activity context."
+      val description = """
+         SemClipboardManager inner classes are held by native references due to IPC calls 
+      """.trimIndent()
+      references += nativeGlobalVariableLeak(
+          "com.samsung.android.content.clipboard.SemClipboardManager$1", description
       ) {
-        manufacturer == SAMSUNG && sdkInt in 19..24
-      }
-      references += instanceFieldLeak(
-          "com.samsung.android.content.clipboard.SemClipboardManager$3",
-          "this$0"
-          ,
-          description =
-          "SemClipboardManager is held in memory by an anonymous inner class" +
-              " implementation of android.os.Binder, thereby leaking an activity context."
-      ) {
-        manufacturer == SAMSUNG && sdkInt in 22..28
+        manufacturer == SAMSUNG && sdkInt in 19..28
       }
-      references += instanceFieldLeak(
-          "com.samsung.android.content.clipboard.SemClipboardManager$1",
-          "this$0"
-          ,
-          description =
-          "SemClipboardManager is held in memory by an anonymous inner class" +
-              " implementation of android.os.Binder, thereby leaking an activity context."
+      references += nativeGlobalVariableLeak(
+          "com.samsung.android.content.clipboard.SemClipboardManager$3", description
       ) {
-        manufacturer == SAMSUNG && sdkInt == 24
+        manufacturer == SAMSUNG && sdkInt in 19..28
       }
     }
   },
@@ -1239,6 +1245,14 @@ enum class AndroidReferenceMatchers {
       return libraryLeak(InstanceFieldPattern(className, fieldName), description, patternApplies)
     }
 
+    fun nativeGlobalVariableLeak(
+      className: String,
+      description: String = "",
+      patternApplies: AndroidBuildMirror.() -> Boolean = ALWAYS
+    ): LibraryLeakReferenceMatcher {
+      return libraryLeak(NativeGlobalVariablePattern(className), description, patternApplies)
+    }
+
     private fun libraryLeak(
       referencePattern: ReferencePattern,
       description: String,

shark/src/main/java/shark/HeapAnalyzer.kt

@@ -48,7 +48,7 @@ import shark.internal.PathFinder
 import shark.internal.PathFinder.PathFindingResults
 import shark.internal.ReferencePathNode
 import shark.internal.ReferencePathNode.ChildNode
-import shark.internal.ReferencePathNode.ChildNode.LibraryLeakNode
+import shark.internal.ReferencePathNode.LibraryLeakNode
 import shark.internal.ReferencePathNode.RootNode
 import shark.internal.lastSegment
 import java.io.File
@@ -353,7 +353,7 @@ class HeapAnalyzer constructor(
       val className =
         recordClassName(graph.findObjectById(pathNode.objectId))
 
-      val firstLibraryLeakNode =
+      val firstLibraryLeakNode = if (rootNode is LibraryLeakNode) rootNode else
         shortestChildPath.firstOrNull { it is LibraryLeakNode } as LibraryLeakNode?
 
       if (firstLibraryLeakNode != null) {

shark/src/main/java/shark/ReferencePattern.kt

@@ -46,4 +46,10 @@ sealed class ReferencePattern : Serializable {
       return "instance field $className#$fieldName"
     }
   }
+
+  data class NativeGlobalVariablePattern(val className: String) : ReferencePattern() {
+    override fun toString(): String {
+      return "native global variable referencing $className"
+    }
+  }
 }

shark/src/main/java/shark/internal/PathFinder.kt

@@ -17,6 +17,7 @@ package shark.internal
 
 import shark.GcRoot
 import shark.GcRoot.JavaFrame
+import shark.GcRoot.JniGlobal
 import shark.GcRoot.ThreadObject
 import shark.HeapGraph
 import shark.HeapObject
@@ -38,12 +39,16 @@ import shark.PrimitiveType.INT
 import shark.ReferenceMatcher
 import shark.ReferencePattern
 import shark.ReferencePattern.InstanceFieldPattern
+import shark.ReferencePattern.NativeGlobalVariablePattern
 import shark.ReferencePattern.StaticFieldPattern
 import shark.SharkLog
 import shark.ValueHolder
-import shark.internal.ReferencePathNode.ChildNode.LibraryLeakNode
+import shark.internal.ReferencePathNode.ChildNode.LibraryLeakChildNode
 import shark.internal.ReferencePathNode.ChildNode.NormalNode
+import shark.internal.ReferencePathNode.LibraryLeakNode
 import shark.internal.ReferencePathNode.RootNode
+import shark.internal.ReferencePathNode.RootNode.LibraryLeakRootNode
+import shark.internal.ReferencePathNode.RootNode.NormalRootNode
 import shark.internal.hppc.LongLongScatterMap
 import shark.internal.hppc.LongScatterSet
 import java.util.ArrayDeque
@@ -106,11 +111,13 @@ internal class PathFinder(
   private val fieldNameByClassName: Map<String, Map<String, ReferenceMatcher>>
   private val staticFieldNameByClassName: Map<String, Map<String, ReferenceMatcher>>
   private val threadNameReferenceMatchers: Map<String, ReferenceMatcher>
+  private val jniGlobalReferenceMatchers: Map<String, ReferenceMatcher>
 
   init {
     val fieldNameByClassName = mutableMapOf<String, MutableMap<String, ReferenceMatcher>>()
     val staticFieldNameByClassName = mutableMapOf<String, MutableMap<String, ReferenceMatcher>>()
     val threadNames = mutableMapOf<String, ReferenceMatcher>()
+    val jniGlobals = mutableMapOf<String, ReferenceMatcher>()
 
     referenceMatchers.filter {
       (it is IgnoredReferenceMatcher || (it is LibraryLeakReferenceMatcher && it.patternApplies(
@@ -140,11 +147,15 @@ internal class PathFinder(
               }
               map[pattern.fieldName] = referenceMatcher
             }
+            is NativeGlobalVariablePattern -> {
+              jniGlobals[pattern.className] = referenceMatcher
+            }
           }
         }
     this.fieldNameByClassName = fieldNameByClassName
     this.staticFieldNameByClassName = staticFieldNameByClassName
     this.threadNameReferenceMatchers = threadNames
+    this.jniGlobalReferenceMatchers = jniGlobals
   }
 
   fun findPathsFromGcRoots(
@@ -243,35 +254,49 @@ internal class PathFinder(
       when (gcRoot) {
         is ThreadObject -> {
           threadsBySerialNumber[gcRoot.threadSerialNumber] = objectRecord.asInstance!! to gcRoot
-          enqueue(RootNode(gcRoot, gcRoot.id))
+          enqueue(NormalRootNode(gcRoot.id, gcRoot))
         }
         is JavaFrame -> {
           val (threadInstance, threadRoot) = threadsBySerialNumber.getValue(
               gcRoot.threadSerialNumber
           )
           val threadName = threadNames[threadInstance] ?: {
-            val name = threadInstance[Thread::class, "name"]?.value?.readAsJavaString()?:""
+            val name = threadInstance[Thread::class, "name"]?.value?.readAsJavaString() ?: ""
             threadNames[threadInstance] = name
             name
           }()
           val referenceMatcher = threadNameReferenceMatchers[threadName]
 
           if (referenceMatcher !is IgnoredReferenceMatcher) {
-            val rootNode = RootNode(gcRoot, threadRoot.id)
+            val rootNode = NormalRootNode(threadRoot.id, gcRoot)
             // Unfortunately Android heap dumps do not include stack trace data, so
             // JavaFrame.frameNumber is always -1 and we cannot know which method is causing the
             // reference to be held.
             val leakReference = LeakReference(LOCAL, "")
 
             val childNode = if (referenceMatcher is LibraryLeakReferenceMatcher) {
-              LibraryLeakNode(gcRoot.id, rootNode, leakReference, referenceMatcher)
+              LibraryLeakChildNode(gcRoot.id, rootNode, leakReference, referenceMatcher)
             } else {
               NormalNode(gcRoot.id, rootNode, leakReference)
             }
             enqueue(childNode)
           }
         }
-        else -> enqueue(RootNode(gcRoot, gcRoot.id))
+        is JniGlobal -> {
+          val referenceMatcher = when (objectRecord) {
+            is HeapClass -> jniGlobalReferenceMatchers[objectRecord.name]
+            is HeapInstance -> jniGlobalReferenceMatchers[objectRecord.instanceClassName]
+            is HeapObjectArray -> jniGlobalReferenceMatchers[objectRecord.arrayClassName]
+            is HeapPrimitiveArray -> jniGlobalReferenceMatchers[objectRecord.arrayClassName]
+          }
+          if (referenceMatcher !is IgnoredReferenceMatcher) {
+            if (referenceMatcher is LibraryLeakReferenceMatcher)
+              enqueue(LibraryLeakRootNode(gcRoot.id, gcRoot, referenceMatcher))
+          } else {
+            enqueue(NormalRootNode(gcRoot.id, gcRoot))
+          }
+        }
+        else -> enqueue(NormalRootNode(gcRoot.id, gcRoot))
       }
     }
   }
@@ -307,7 +332,7 @@ internal class PathFinder(
           val objectExists = graph.objectExists(gcRoot.id)
           if (!objectExists) {
             SharkLog.d {
-                "${gcRoot::class.java.simpleName} gc root ignored because it's pointing to unknown object @${gcRoot.id}"
+              "${gcRoot::class.java.simpleName} gc root ignored because it's pointing to unknown object @${gcRoot.id}"
             }
           }
           objectExists
@@ -348,7 +373,7 @@ internal class PathFinder(
 
       val node = when (val referenceMatcher = ignoredStaticFields[fieldName]) {
         null -> NormalNode(objectId, parent, LeakReference(STATIC_FIELD, fieldName))
-        is LibraryLeakReferenceMatcher -> LibraryLeakNode(
+        is LibraryLeakReferenceMatcher -> LibraryLeakChildNode(
             objectId, parent, LeakReference(STATIC_FIELD, fieldName), referenceMatcher
         )
         is IgnoredReferenceMatcher -> null
@@ -397,7 +422,7 @@ internal class PathFinder(
         if (threadLocalValuesMatcher != null && field.declaringClass.name == "java.lang.Thread" && field.name == "localValues") {
           // Earlier Android versions store local references in a Thread.localValues field.
           if (threadLocalValuesMatcher is LibraryLeakReferenceMatcher) {
-            LibraryLeakNode(
+            LibraryLeakChildNode(
                 objectId, parent, LeakReference(INSTANCE_FIELD, field.name),
                 threadLocalValuesMatcher
             )
@@ -407,7 +432,7 @@ internal class PathFinder(
         } else when (val referenceMatcher = fieldReferenceMatchers[field.name]) {
           null -> NormalNode(objectId, parent, LeakReference(INSTANCE_FIELD, field.name))
           is LibraryLeakReferenceMatcher ->
-            LibraryLeakNode(
+            LibraryLeakChildNode(
                 objectId, parent, LeakReference(INSTANCE_FIELD, field.name), referenceMatcher
             )
           is IgnoredReferenceMatcher -> null
@@ -572,7 +597,8 @@ internal class PathFinder(
       }
       return
     }
-    val nextDominator = if (parentIsRetainedObject) parent else dominatedObjectIds.getSlotValue(parentDominatorSlot)
+    val nextDominator =
+      if (parentIsRetainedObject) parent else dominatedObjectIds.getSlotValue(parentDominatorSlot)
     if (currentDominatorSlot == -1) {
       dominatedObjectIds[objectId] = nextDominator
     } else {

shark/src/main/java/shark/internal/ReferencePathNode.kt

@@ -7,10 +7,25 @@ import shark.LibraryLeakReferenceMatcher
 internal sealed class ReferencePathNode {
   abstract val objectId: Long
 
-  class RootNode(
-    val gcRoot: GcRoot,
-    override val objectId: Long
-  ) : ReferencePathNode()
+  interface LibraryLeakNode {
+    val matcher: LibraryLeakReferenceMatcher
+  }
+
+  sealed class RootNode : ReferencePathNode() {
+    abstract val gcRoot: GcRoot
+
+    class LibraryLeakRootNode(
+      override val objectId: Long,
+      override val gcRoot: GcRoot,
+      override val matcher: LibraryLeakReferenceMatcher
+    ) : RootNode(), LibraryLeakNode
+
+    class NormalRootNode(
+      override val objectId: Long,
+      override val gcRoot: GcRoot
+    ) : RootNode()
+
+  }
 
   sealed class ChildNode : ReferencePathNode() {
 
@@ -21,12 +36,12 @@ internal sealed class ReferencePathNode {
      */
     abstract val referenceFromParent: LeakReference
 
-    class LibraryLeakNode(
+    class LibraryLeakChildNode(
       override val objectId: Long,
       override val parent: ReferencePathNode,
       override val referenceFromParent: LeakReference,
-      val matcher: LibraryLeakReferenceMatcher
-    ) : ChildNode()
+      override val matcher: LibraryLeakReferenceMatcher
+    ) : ChildNode(), LibraryLeakNode
 
     class NormalNode(
       override val objectId: Long,

shark/src/test/java/shark/ReferenceMatcherTest.kt

@@ -5,9 +5,12 @@ import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
 import org.junit.rules.TemporaryFolder
+import shark.GcRoot.JniGlobal
 import shark.ReferencePattern.InstanceFieldPattern
 import shark.ReferencePattern.JavaLocalPattern
+import shark.ReferencePattern.NativeGlobalVariablePattern
 import shark.ReferencePattern.StaticFieldPattern
+import shark.ValueHolder.ReferenceHolder
 import java.io.File
 import java.lang.ref.WeakReference
 
@@ -116,4 +119,39 @@ class ReferenceMatcherTest {
     assertThat(analysis.libraryLeaks).isEmpty()
   }
 
+  @Test fun nativeGlobalVariableLibraryLeak() {
+    hprofFile.dump {
+      gcRoot(JniGlobal(id = "Leaking".watchedInstance {}.value, jniGlobalRefId = 42))
+    }
+
+    val matcher = LibraryLeakReferenceMatcher(NativeGlobalVariablePattern("Leaking"))
+    val analysis = hprofFile.checkForLeaks<HeapAnalysisSuccess>(
+        referenceMatchers = listOf(matcher)
+    )
+    val leak = analysis.libraryLeaks[0]
+    assertThat(leak.pattern).isEqualTo(matcher.pattern)
+  }
+
+  @Test fun nativeGlobalVariableShortestPathExcluded() {
+    hprofFile.dump {
+      val leaking = instance(clazz("Leaking"))
+      keyedWeakReference(leaking)
+      val hasLeaking = instance(
+          clazz("HasLeaking", fields = listOf("leaking" to ReferenceHolder::class)),
+          fields = listOf(leaking)
+      )
+      clazz("GcRoot", staticFields = listOf("longestPath" to hasLeaking))
+      gcRoot(JniGlobal(id = leaking.value, jniGlobalRefId = 42))
+    }
+
+    val matcher = LibraryLeakReferenceMatcher(NativeGlobalVariablePattern("Leaking"))
+    val analysis = hprofFile.checkForLeaks<HeapAnalysisSuccess>(
+        referenceMatchers = listOf(matcher)
+    )
+    val leak = analysis.applicationLeaks[0]
+    assertThat(leak.leakTrace.elements).hasSize(3)
+    assertThat(leak.leakTrace.elements[0].className).isEqualTo("GcRoot")
+    assertThat(leak.leakTrace.elements[0].reference!!.name).isEqualTo("longestPath")
+  }
+
 }