diff --git a/shark-android/src/main/java/shark/AndroidReferenceMatchers.kt b/shark-android/src/main/java/shark/AndroidReferenceMatchers.kt index 46bb84f5dc..9ea0b94192 100644 --- a/shark-android/src/main/java/shark/AndroidReferenceMatchers.kt +++ b/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) { + 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) { + 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 ) { - 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, diff --git a/shark/src/main/java/shark/HeapAnalyzer.kt b/shark/src/main/java/shark/HeapAnalyzer.kt index 791af7e0e8..1ad4ca5bb9 100644 --- a/shark/src/main/java/shark/HeapAnalyzer.kt +++ b/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) { diff --git a/shark/src/main/java/shark/ReferencePattern.kt b/shark/src/main/java/shark/ReferencePattern.kt index 9de31795a1..4482412651 100644 --- a/shark/src/main/java/shark/ReferencePattern.kt +++ b/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" + } + } } \ No newline at end of file diff --git a/shark/src/main/java/shark/internal/PathFinder.kt b/shark/src/main/java/shark/internal/PathFinder.kt index 5ea4440420..30bb3bfe78 100644 --- a/shark/src/main/java/shark/internal/PathFinder.kt +++ b/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> private val staticFieldNameByClassName: Map> private val threadNameReferenceMatchers: Map + private val jniGlobalReferenceMatchers: Map init { val fieldNameByClassName = mutableMapOf>() val staticFieldNameByClassName = mutableMapOf>() val threadNames = mutableMapOf() + val jniGlobals = mutableMapOf() 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 { diff --git a/shark/src/main/java/shark/internal/ReferencePathNode.kt b/shark/src/main/java/shark/internal/ReferencePathNode.kt index 3e2c0e620e..bf83a23b52 100644 --- a/shark/src/main/java/shark/internal/ReferencePathNode.kt +++ b/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, diff --git a/shark/src/test/java/shark/ReferenceMatcherTest.kt b/shark/src/test/java/shark/ReferenceMatcherTest.kt index bf1c10076a..f9831986de 100644 --- a/shark/src/test/java/shark/ReferenceMatcherTest.kt +++ b/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( + 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( + 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") + } + } \ No newline at end of file