From d882b741da0f516c80a1868711fbfa96ff98d20d Mon Sep 17 00:00:00 2001 From: Pierre-Yves Ricau Date: Thu, 1 Jun 2023 09:27:16 -0700 Subject: [PATCH 1/7] [LC2] Fix crash from LifecycleRegistry kotlin conversion Cherry picking 4f81e767def3c83ddb4dd0dfbc8d53976d81f476 onto the LeakCanary 2 branch --- .../src/main/java/shark/AndroidObjectInspectors.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/shark-android/src/main/java/shark/AndroidObjectInspectors.kt b/shark-android/src/main/java/shark/AndroidObjectInspectors.kt index cda50362cb..73e3e6cab1 100644 --- a/shark-android/src/main/java/shark/AndroidObjectInspectors.kt +++ b/shark-android/src/main/java/shark/AndroidObjectInspectors.kt @@ -854,7 +854,11 @@ enum class AndroidObjectInspectors : ObjectInspector { private val HeapInstance.lifecycleRegistryState: String get() { - val state = this["androidx.lifecycle.LifecycleRegistry", "mState"]!!.valueAsInstance!! + // LifecycleRegistry was converted to Kotlin + // https://cs.android.com/androidx/platform/frameworks/support/+/36833f9ab0c50bf449fc795e297a0e124df3356e + val stateField = this["androidx.lifecycle.LifecycleRegistry", "state"] + ?: this["androidx.lifecycle.LifecycleRegistry", "mState"]!! + val state = stateField.valueAsInstance!! return state["java.lang.Enum", "name"]!!.value.readAsJavaString()!! } }, From 50d05a6495492943862562cb93688b17a6a46f65 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Ricau Date: Wed, 28 Jun 2023 15:40:19 -0700 Subject: [PATCH 2/7] Bring in latest batch of known leaks --- shark-android/api/shark-android.api | 8 ++ .../java/shark/AndroidObjectInspectors.kt | 8 ++ .../java/shark/AndroidReferenceMatchers.kt | 98 ++++++++++++++++--- 3 files changed, 101 insertions(+), 13 deletions(-) diff --git a/shark-android/api/shark-android.api b/shark-android/api/shark-android.api index 75a7a05193..ece1532593 100644 --- a/shark-android/api/shark-android.api +++ b/shark-android/api/shark-android.api @@ -22,6 +22,7 @@ public final class shark/AndroidMetadataExtractor : shark/MetadataExtractor { public abstract class shark/AndroidObjectInspectors : java/lang/Enum, shark/ObjectInspector { public static final field ACTIVITY Lshark/AndroidObjectInspectors; + public static final field ACTIVITY_THREAD Lshark/AndroidObjectInspectors; public static final field ANDROIDX_FRAGMENT Lshark/AndroidObjectInspectors; public static final field ANIMATOR Lshark/AndroidObjectInspectors; public static final field APPLICATION Lshark/AndroidObjectInspectors; @@ -78,6 +79,7 @@ public abstract class shark/AndroidReferenceMatchers : java/lang/Enum { public static final field ASSIST_STRUCTURE Lshark/AndroidReferenceMatchers; public static final field AUDIO_MANAGER Lshark/AndroidReferenceMatchers; public static final field AUDIO_MANAGER__MCONTEXT_STATIC Lshark/AndroidReferenceMatchers; + public static final field AW_CONTENTS__A0 Lshark/AndroidReferenceMatchers; public static final field AW_RESOURCE__SRESOURCES Lshark/AndroidReferenceMatchers; public static final field BACKDROP_FRAME_RENDERER__MDECORVIEW Lshark/AndroidReferenceMatchers; public static final field BIOMETRIC_PROMPT Lshark/AndroidReferenceMatchers; @@ -91,11 +93,14 @@ public abstract class shark/AndroidReferenceMatchers : java/lang/Enum { public static final field CONTROLLED_INPUT_CONNECTION_WRAPPER Lshark/AndroidReferenceMatchers; public static final field Companion Lshark/AndroidReferenceMatchers$Companion; public static final field DEVICE_POLICY_MANAGER__SETTINGS_OBSERVER Lshark/AndroidReferenceMatchers; + public static final field DREAM_SERVICE Lshark/AndroidReferenceMatchers; public static final field EDITTEXT_BLINK_MESSAGEQUEUE Lshark/AndroidReferenceMatchers; public static final field EVENT_RECEIVER__MMESSAGE_QUEUE Lshark/AndroidReferenceMatchers; public static final field EXTENDED_STATUS_BAR_MANAGER Lshark/AndroidReferenceMatchers; public static final field FINALIZER_WATCHDOG_DAEMON Lshark/AndroidReferenceMatchers; + public static final field FLIPPER__APPLICATION_DESCRIPTOR Lshark/AndroidReferenceMatchers; public static final field GESTURE_BOOST_MANAGER Lshark/AndroidReferenceMatchers; + public static final field HMD_GLOBAL Ljava/lang/String; public static final field HOST_ADPU_SERVICE_MSG_HANDLER Lshark/AndroidReferenceMatchers; public static final field HUAWEI Ljava/lang/String; public static final field IMM_CURRENT_INPUT_CONNECTION Lshark/AndroidReferenceMatchers; @@ -104,6 +109,7 @@ public abstract class shark/AndroidReferenceMatchers : java/lang/Enum { public static final field INPUT_METHOD_MANAGER_IS_TERRIBLE Lshark/AndroidReferenceMatchers; public static final field INSTRUMENTATION_RECOMMEND_ACTIVITY Lshark/AndroidReferenceMatchers; public static final field IREQUEST_FINISH_CALLBACK Lshark/AndroidReferenceMatchers; + public static final field JOB_SERVICE Lshark/AndroidReferenceMatchers; public static final field LAYOUT_TRANSITION Lshark/AndroidReferenceMatchers; public static final field LEAK_CANARY_HEAP_DUMPER Lshark/AndroidReferenceMatchers; public static final field LEAK_CANARY_INTERNAL Lshark/AndroidReferenceMatchers; @@ -156,6 +162,8 @@ public abstract class shark/AndroidReferenceMatchers : java/lang/Enum { public static final field VIEW_GROUP__M_PRE_SORTED_CHILDREN Lshark/AndroidReferenceMatchers; public static final field VIVO Ljava/lang/String; public static final field WINDOW_ON_BACK_INVOKED_DISPATCHER__STUB Lshark/AndroidReferenceMatchers; + public static final field XIAMI__RESOURCES_IMPL Lshark/AndroidReferenceMatchers; + public static final field XIAOMI Ljava/lang/String; public synthetic fun (Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public static final fun buildKnownReferences (Ljava/util/Set;)Ljava/util/List; public static final fun getAppDefaults ()Ljava/util/List; diff --git a/shark-android/src/main/java/shark/AndroidObjectInspectors.kt b/shark-android/src/main/java/shark/AndroidObjectInspectors.kt index cda50362cb..a744c98e95 100644 --- a/shark-android/src/main/java/shark/AndroidObjectInspectors.kt +++ b/shark-android/src/main/java/shark/AndroidObjectInspectors.kt @@ -376,6 +376,14 @@ enum class AndroidObjectInspectors : ObjectInspector { } }, + ACTIVITY_THREAD { + override fun inspect(reporter: ObjectReporter) { + reporter.whenInstanceOf("android.app.ActivityThread") { + notLeakingReasons += "ActivityThread is a singleton" + } + } + }, + APPLICATION { override fun inspect( reporter: ObjectReporter diff --git a/shark-android/src/main/java/shark/AndroidReferenceMatchers.kt b/shark-android/src/main/java/shark/AndroidReferenceMatchers.kt index b2e1c8d57b..e47fbcda25 100644 --- a/shark-android/src/main/java/shark/AndroidReferenceMatchers.kt +++ b/shark-android/src/main/java/shark/AndroidReferenceMatchers.kt @@ -245,7 +245,7 @@ enum class AndroidReferenceMatchers { InputMethodManager.mImeInsetsConsumer isn't set to null when the activity is destroyed. """.trimIndent() ) { - sdkInt == 31 + sdkInt >= 31 } } }, @@ -541,7 +541,7 @@ enum class AndroidReferenceMatchers { " on the screen. TextView.ChangeWatcher and android.widget.Editor end up in spans and" + " typically hold on to the view hierarchy" ) { - sdkInt in 24..30 + sdkInt >= 24 } } }, @@ -814,15 +814,17 @@ enum class AndroidReferenceMatchers { override fun add( references: MutableList ) { - references += instanceFieldLeak( - "android.app.AppOpsManager\$3", "this\$0", + references += nativeGlobalVariableLeak( + "android.app.AppOpsManager\$3", description = """ - AppOpsManager\$3 implements IAppOpsActiveCallback.Stub and is held by a native ref and - holds on to am AppOpsManager which references an activity context. - Report: https://issuetracker.google.com/issues/210899127 + Fix: Update androidx.core:core to 1.10.0-alpha01 or greater as it includes an Android 12 + fix for this leak on Android 12, see https://github.com/androidx/androidx/pull/435 . + AppOpsManager\$3 implements IAppOpsActiveCallback.Stub and is held by a native ref long + until the calling side gets GCed, which can happen long after the stub is no longer of + use. """.trimIndent() ) { - sdkInt in 31..33 + sdkInt in 31..32 } } }, @@ -895,6 +897,60 @@ enum class AndroidReferenceMatchers { } }, + FLIPPER__APPLICATION_DESCRIPTOR { + override fun add( + references: MutableList + ) { + references += staticFieldLeak( + "com.facebook.flipper.plugins.inspector.descriptors.ApplicationDescriptor", "editedDelegates", + description = """ + Flipper's ApplicationDescriptor leaks root views after they've been detached. + https://github.com/facebook/flipper/issues/4270 + """.trimIndent() + ) + } + }, + + AW_CONTENTS__A0 { + override fun add(references: MutableList) { + staticFieldLeak( + "org.chromium.android_webview.AwContents", + "A0", + description = """ + WindowAndroidWrapper has a strong ref to the context key so this breaks the WeakHashMap + contracts and WeakHashMap is unable to perform its job of auto cleaning. + https://github.com/square/leakcanary/issues/2538 + """.trimIndent() + ) + } + }, + + JOB_SERVICE { + override fun add( + references: MutableList + ) { + AndroidReferenceMatchers.nativeGlobalVariableLeak( + className = "android.app.job.JobService\$1", + description = """ + JobService used to be leaked via a binder stub. + Fix: https://cs.android.com/android/_/android/platform/frameworks/base/+/0796e9fb3dc2dd03fa5ff2053c63f14861cffa2f + """.trimIndent() + ) { sdkInt < 24 } + } + }, + + DREAM_SERVICE { + override fun add(references: MutableList) { + AndroidReferenceMatchers.nativeGlobalVariableLeak( + className = "android.service.dreams.DreamService\$1", + description = """ + DreamService leaks a binder stub. + https://github.com/square/leakcanary/issues/2534 + """.trimIndent() + ) { sdkInt >= 33 } + } + }, + // ######## Manufacturer specific known leaks ######## // SAMSUNG @@ -1319,15 +1375,15 @@ enum class AndroidReferenceMatchers { override fun add( references: MutableList ) { - references += staticFieldLeak( - "android.app.ExtendedStatusBarManager", "sInstance", + references += instanceFieldLeak( + "android.app.ExtendedStatusBarManager", "mContext", description = """ - ExtendedStatusBarManager is held in a static sInstance field and has a mContext - field which references a decor context which references a destroyed activity. + ExtendedStatusBarManager has a mContext field which references a decor context which + references a destroyed activity. """.trimIndent() ) { - manufacturer == SHARP && sdkInt == 30 + manufacturer == SHARP && sdkInt >= 30 } } }, @@ -1381,6 +1437,20 @@ enum class AndroidReferenceMatchers { } }, + XIAMI__RESOURCES_IMPL { + override fun add(references: MutableList) { + references += staticFieldLeak( + "android.content.res.ResourcesImpl", "mAppContext", + description = """ + Both Nokia & Xiaomi added a static mAppContext field to the ResourcesImpl class and + that field ends up referencing lower contexts (e.g. service). + """.trimIndent() + ) { + (manufacturer == XIAOMI || manufacturer == HMD_GLOBAL) && sdkInt >= 30 + } + } + }, + // ######## Ignored references (not leaks) ######## REFERENCES { @@ -1479,6 +1549,8 @@ enum class AndroidReferenceMatchers { const val VIVO = "vivo" const val RAZER = "Razer" const val SHARP = "SHARP" + const val XIAOMI = "Xiaomi" + const val HMD_GLOBAL = "HMD Global" /** * Returns a list of [ReferenceMatcher] that only contains [IgnoredReferenceMatcher] and no From 8fb4380be0e4308623aac464f9ce0a0fadfc6041 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Ricau Date: Wed, 28 Jun 2023 22:37:38 -0700 Subject: [PATCH 3/7] Better support for ActivityThread.mNewActivities --- .../java/shark/AndroidReferenceMatchers.kt | 9 +- .../shark/internal/AndroidReferenceReaders.kt | 138 ++++++++++++++++++ 2 files changed, 145 insertions(+), 2 deletions(-) diff --git a/shark-android/src/main/java/shark/AndroidReferenceMatchers.kt b/shark-android/src/main/java/shark/AndroidReferenceMatchers.kt index e47fbcda25..2cde2aa06a 100644 --- a/shark-android/src/main/java/shark/AndroidReferenceMatchers.kt +++ b/shark-android/src/main/java/shark/AndroidReferenceMatchers.kt @@ -72,6 +72,9 @@ enum class AndroidReferenceMatchers { } }, + /** + * See AndroidReferenceReaders.ACTIVITY_THREAD__NEW_ACTIVITIES for more context + */ ACTIVITY_THREAD__M_NEW_ACTIVITIES { override fun add( references: MutableList @@ -79,11 +82,11 @@ enum class AndroidReferenceMatchers { references += instanceFieldLeak( "android.app.ActivityThread", "mNewActivities", description = """ - New activities are leaks by ActivityThread until the main thread becomes idle. + New activities are leaked by ActivityThread until the main thread becomes idle. Tracked here: https://issuetracker.google.com/issues/258390457 """.trimIndent() ) { - sdkInt in 19..33 + sdkInt >= 19 } } }, @@ -559,6 +562,8 @@ enum class AndroidReferenceMatchers { } }, + + BIOMETRIC_PROMPT { override fun add(references: MutableList) { references += instanceFieldLeak( diff --git a/shark/src/main/java/shark/internal/AndroidReferenceReaders.kt b/shark/src/main/java/shark/internal/AndroidReferenceReaders.kt index 9c07b384fc..15ab5bddf5 100644 --- a/shark/src/main/java/shark/internal/AndroidReferenceReaders.kt +++ b/shark/src/main/java/shark/internal/AndroidReferenceReaders.kt @@ -2,6 +2,9 @@ package shark.internal import shark.HeapGraph import shark.HeapObject.HeapInstance +import shark.LibraryLeakReferenceMatcher +import shark.ReferencePattern.InstanceFieldPattern +import shark.ValueHolder import shark.ValueHolder.ReferenceHolder import shark.internal.ChainingInstanceReferenceReader.VirtualInstanceReferenceReader import shark.internal.ChainingInstanceReferenceReader.VirtualInstanceReferenceReader.OptionalFactory @@ -11,6 +14,141 @@ import shark.internal.ReferenceLocationType.INSTANCE_FIELD internal enum class AndroidReferenceReaders : OptionalFactory { + /** + * ActivityThread.mNewActivity is a linked list of ActivityClientRecord that keeps track of + * activities after they were resumed, until the main thread is idle. This is used to report + * analytics to system_server about how long it took for the main thread to settle after + * resuming an activity. Unfortunately, if the main thread never becomes idle, all these + * new activities leak in memory. + * + * We'd normally catch these with a pattern in AndroidReferenceMatchers, and we do have + * AndroidReferenceMatchers.ACTIVITY_THREAD__M_NEW_ACTIVITIES to do that, however this matching + * only works if none of the activities alive are waiting for idle. If any activity alive is + * still waiting for idle (which all the alive activities would be if they main thread is never + * idle) then ActivityThread.mActivities will reference an ActivityClientRecord through an + * ArrayMap and because ActivityClientRecord are reused that instance will also have its nextIdle + * fields set, so we're effectively traversing the ActivityThread.mNewActivity from a completely + * different and unexpected entry point. + * + * To fix that problem of broken pattern matching, we emit the mNewActivities field when + * finding an ActivityThread instance, and because custom ref readers have priority over the + * default instance field reader, we're guaranteed that mNewActivities is enqueued before + * mActivities. Unfortunately, that also means we can't rely on AndroidReferenceMatchers as + * those aren't used here, so we recreate our own LibraryLeakReferenceMatcher. + * + * We want to traverse mNewActivities before mActivities so we can't set isLowPriority to true + * like we would for normal path tagged as source of leak. So we will prioritize going through all + * activities in mNewActivities, some of which aren't destroyed yet (and therefore not leaking). + * Going through those paths of non leaking activities, we might find other leaks though. + * This would result in us tagging unrelated leaks as part of the mNewActivities leak. To prevent + * this, we traverse ActivityThread.mNewActivities as a linked list through + * ActivityClientRecord.nextIdle as a linked list, but we emit only ActivityClientRecord.activity + * fields if such activities are destroyed, which means any live activity in + * ActivityThread.mNewActivities will be discovered through the normal field navigation process + * and should go through ActivityThread.mActivities. + */ + ACTIVITY_THREAD__NEW_ACTIVITIES { + override fun create(graph: HeapGraph): VirtualInstanceReferenceReader? { + val activityThreadClass = graph.findClassByName("android.app.ActivityThread") ?: return null + + if (activityThreadClass.readRecordFields().none { + activityThreadClass.instanceFieldName(it) == "mNewActivities" + } + ) { + return null + } + + val activityClientRecordClass = + graph.findClassByName("android.app.ActivityThread\$ActivityClientRecord") ?: return null + + val activityClientRecordFieldNames = activityClientRecordClass.readRecordFields() + .map { activityThreadClass.instanceFieldName(it) } + .toList() + + if ("nextIdle" !in activityClientRecordFieldNames || + "activity" !in activityClientRecordFieldNames) { + return null + } + + val activityThreadClassId = activityThreadClass.objectId + val activityClientRecordClassId = activityClientRecordClass.objectId + + return object : VirtualInstanceReferenceReader { + override fun matches(instance: HeapInstance) = + instance.instanceClassId == activityThreadClassId || + instance.instanceClassId == activityClientRecordClassId + + override fun read(source: HeapInstance): Sequence { + return if (source.instanceClassId == activityThreadClassId) { + val mNewActivities = + source["android.app.ActivityThread", "mNewActivities"]!!.value.asObjectId!! + if (mNewActivities == ValueHolder.NULL_REFERENCE) { + emptySequence() + } else { + source.graph.context[ACTIVITY_THREAD__NEW_ACTIVITIES.name] = mNewActivities + sequenceOf( + Reference( + valueObjectId = mNewActivities, + isLowPriority = false, + lazyDetailsResolver = { + LazyDetails( + name = "mNewActivities", + locationClassObjectId = activityThreadClassId, + locationType = INSTANCE_FIELD, + isVirtual = false, + matchedLibraryLeak = LibraryLeakReferenceMatcher( + pattern = InstanceFieldPattern( + "android.app.ActivityThread", "mNewActivities" + ), + description = """ + New activities are leaked by ActivityThread until the main thread becomes idle. + Tracked here: https://issuetracker.google.com/issues/258390457 + """.trimIndent() + ) + ) + }) + ) + } + } else { + val mNewActivities = + source.graph.context.get(ACTIVITY_THREAD__NEW_ACTIVITIES.name) + if (mNewActivities == null || source.objectId != mNewActivities) { + emptySequence() + } else { + generateSequence(source) { node -> + node["android.app.ActivityThread\$ActivityClientRecord", "nextIdle"]!!.valueAsInstance + }.withIndex().mapNotNull { (index, node) -> + + val activity = node["android.app.ActivityThread\$ActivityClientRecord", "activity"]!!.valueAsInstance + if (activity == null || + // Skip non destroyed activities. + // (!= true because we also skip if mDestroyed is missing) + activity["android.app.Activity", "mDestroyed"]?.value?.asBoolean != true + ) { + null + } else { + Reference( + valueObjectId = activity.objectId, + isLowPriority = false, + lazyDetailsResolver = { + LazyDetails( + name = "$index", + locationClassObjectId = activityClientRecordClassId, + locationType = ARRAY_ENTRY, + isVirtual = true, + matchedLibraryLeak = null + ) + }) + } + } + } + } + } + } + } + }, + + MESSAGE_QUEUE { override fun create(graph: HeapGraph): VirtualInstanceReferenceReader? { val messageQueueClass = From 455881c0341f4d91a59d7b142cb39c8231660ae3 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Ricau Date: Thu, 29 Jun 2023 07:02:46 -0700 Subject: [PATCH 4/7] fix build --- detekt-config.yml | 3 +-- shark-android/src/main/java/shark/AndroidReferenceMatchers.kt | 2 -- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/detekt-config.yml b/detekt-config.yml index 26e0ac82b8..14af4db9fa 100644 --- a/detekt-config.yml +++ b/detekt-config.yml @@ -512,8 +512,7 @@ style: RedundantVisibilityModifierRule: active: false ReturnCount: - #LeakCanary - increased from 2 to 4 - active: true + active: false max: 4 excludedFunctions: "equals" excludeLabeled: false diff --git a/shark-android/src/main/java/shark/AndroidReferenceMatchers.kt b/shark-android/src/main/java/shark/AndroidReferenceMatchers.kt index 2cde2aa06a..574f0fc08f 100644 --- a/shark-android/src/main/java/shark/AndroidReferenceMatchers.kt +++ b/shark-android/src/main/java/shark/AndroidReferenceMatchers.kt @@ -562,8 +562,6 @@ enum class AndroidReferenceMatchers { } }, - - BIOMETRIC_PROMPT { override fun add(references: MutableList) { references += instanceFieldLeak( From 38cfed89663c6fe0302c8792ad2ecade684a33e6 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Ricau Date: Thu, 29 Jun 2023 09:12:28 -0700 Subject: [PATCH 5/7] Prepare 2.12 release --- mkdocs.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mkdocs.yml b/mkdocs.yml index cd064ece60..5ef8bacc43 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -1,6 +1,6 @@ extra: leak_canary: - release: '2.11' + release: '2.12' next_release: '3.0' social: - icon: fontawesome/brands/github-alt From c227d839125f0d839f485acdbecf38003a0dac48 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Ricau Date: Thu, 29 Jun 2023 09:42:40 -0700 Subject: [PATCH 6/7] Prepare for next development iteration --- docs/changelog.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/docs/changelog.md b/docs/changelog.md index bcdea87988..d9cf8858af 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -3,6 +3,20 @@ Please thank our [contributors](https://github.com/square/leakcanary/graphs/contributors) 🙏 🙏 🙏. +I've started working on LeakCanary 3.0 so new 2.x releases only contain bug fixes and new known leak patterns. + +## Version 2.12 (2023-06-29) + +* 💥 [#2527](https://github.com/square/leakcanary/issues/2527) `LifecycleRegistry` in `androidx.lifecycle:lifecycle-runtime` was migrated to kotlin and its `mState` field name changed to `state` which broke LeakCanary expectations. +* 🐤 [#2545](https://github.com/square/leakcanary/pull/2545) Added several known manufacturer & framework leaks. + + +## Version 2.11 (2023-05-17) + +* 🐛 [#1764](https://github.com/square/leakcanary/issues/1764) Ignore phantom classes that were unloaded than reloaded (long time LeakCanary bug). +* 🐛 [#2471](https://github.com/square/leakcanary/issues/2471) Fix LeakCanary introducing a weird leak in Google's CI infra. +* 🐛 [#2496](https://github.com/square/leakcanary/issues/2496) Fix broken ViewModel leak detection + ## Version 2.10 (2022-11-10) ### Experimental Neo4j heap dump exploration @@ -12,7 +26,7 @@ Please thank our [contributors](https://github.com/square/leakcanary/graphs/cont ``` brew install leakcanary-shark -shark-cli --process com.example.app.debug neo4j +shark-cli --process com.example.app.debug neo4j ``` ![Neo4J heap dump](https://user-images.githubusercontent.com/557033/200693468-aa783bb4-9a5a-4a41-8b92-582d44b31b92.png) From f38e86a55e3d02ae321349afa5504194d732999e Mon Sep 17 00:00:00 2001 From: Eric Maxwell Date: Fri, 6 Oct 2023 15:32:47 -0400 Subject: [PATCH 7/7] Add NFINIX and LENOVO to list of manufacturers that we know of a static context leak --- shark-android/build.gradle | 1 + .../java/shark/AndroidReferenceMatchers.kt | 9 +- .../shark/AndroidReferenceMatchersTest.kt | 147 ++++++++++++++++++ 3 files changed, 154 insertions(+), 3 deletions(-) create mode 100644 shark-android/src/test/java/shark/AndroidReferenceMatchersTest.kt diff --git a/shark-android/build.gradle b/shark-android/build.gradle index c5052d2b23..9f0038f0fa 100644 --- a/shark-android/build.gradle +++ b/shark-android/build.gradle @@ -17,5 +17,6 @@ dependencies { testImplementation libs.mockito testImplementation libs.mockitoKotlin testImplementation libs.okio2 + testImplementation projects.sharkHprofTest testImplementation projects.sharkTest } diff --git a/shark-android/src/main/java/shark/AndroidReferenceMatchers.kt b/shark-android/src/main/java/shark/AndroidReferenceMatchers.kt index 574f0fc08f..1c80af5460 100644 --- a/shark-android/src/main/java/shark/AndroidReferenceMatchers.kt +++ b/shark-android/src/main/java/shark/AndroidReferenceMatchers.kt @@ -1445,11 +1445,12 @@ enum class AndroidReferenceMatchers { references += staticFieldLeak( "android.content.res.ResourcesImpl", "mAppContext", description = """ - Both Nokia & Xiaomi added a static mAppContext field to the ResourcesImpl class and - that field ends up referencing lower contexts (e.g. service). + A few device manufacturers added a static mAppContext field to the ResourcesImpl class + and that field ends up referencing lower contexts (e.g. service). """.trimIndent() ) { - (manufacturer == XIAOMI || manufacturer == HMD_GLOBAL) && sdkInt >= 30 + listOf(HMD_GLOBAL, INFINIX, LENOVO, XIAOMI).contains(manufacturer) && + sdkInt >= 30 } } }, @@ -1554,6 +1555,8 @@ enum class AndroidReferenceMatchers { const val SHARP = "SHARP" const val XIAOMI = "Xiaomi" const val HMD_GLOBAL = "HMD Global" + const val INFINIX = "INFINIX" + /** * Returns a list of [ReferenceMatcher] that only contains [IgnoredReferenceMatcher] and no diff --git a/shark-android/src/test/java/shark/AndroidReferenceMatchersTest.kt b/shark-android/src/test/java/shark/AndroidReferenceMatchersTest.kt new file mode 100644 index 0000000000..133e2782c7 --- /dev/null +++ b/shark-android/src/test/java/shark/AndroidReferenceMatchersTest.kt @@ -0,0 +1,147 @@ +package shark + +import org.assertj.core.api.Assertions.assertThat +import org.junit.Test +import java.io.File +import javax.print.attribute.standard.PrinterMoreInfoManufacturer +import shark.AndroidReferenceMatchers.Companion.HMD_GLOBAL +import shark.AndroidReferenceMatchers.Companion.HUAWEI +import shark.AndroidReferenceMatchers.Companion.INFINIX +import shark.AndroidReferenceMatchers.Companion.LENOVO +import shark.AndroidReferenceMatchers.Companion.LG +import shark.AndroidReferenceMatchers.Companion.MEIZU +import shark.AndroidReferenceMatchers.Companion.MOTOROLA +import shark.AndroidReferenceMatchers.Companion.NVIDIA +import shark.AndroidReferenceMatchers.Companion.ONE_PLUS +import shark.AndroidReferenceMatchers.Companion.RAZER +import shark.AndroidReferenceMatchers.Companion.SAMSUNG +import shark.AndroidReferenceMatchers.Companion.SHARP +import shark.AndroidReferenceMatchers.Companion.VIVO +import shark.AndroidReferenceMatchers.Companion.XIAOMI +import shark.HprofHeapGraph.Companion.openHeapGraph +import shark.ReferencePattern.StaticFieldPattern +import shark.ValueHolder.IntHolder + +class AndroidReferenceMatchersTest { + + @Test fun `Expect anything under SDK 30 unknown XIAMI__resources_impl`() { + expect(HMD_GLOBAL, 29, false) + } + + @Test fun `Expect NVIDIA unknown XIAMI__resources_impl`() { + expect(NVIDIA, 30, false) + } + + @Test fun `Expect XIAOMI known XIAMI__resources_impl`() { + expect(XIAOMI, 30, true) + } + + @Test fun `Expect LENOVO known XIAMI__resources_impl`() { + expect(LENOVO, 30, true) + } + + @Test fun `Expect INFINIX known XIAMI__resources_impl`() { + expect(INFINIX, 30, true) + } + + @Test fun `Expect HMD_GLOBAL known XIAMI__resources_impl`() { + expect(HMD_GLOBAL, 30, true) + } + + /** + * Validate whether or not we see a known library leak for the given device manufacturer + * and sdkInt combination, resulting from a call to + * AndroidReferenceMatchers.XIAMI__RESOURCES_IMPL.add(matchers). + */ + private fun expect(manufacturer: String, sdkInt: Int, expectKnown: Boolean) { + + val hprofBytes = dumpToBytes { + clazz("android.os.Build", + staticFields = listOf("MANUFACTURER" to string(manufacturer), "ID" to string("someId")) + ) + clazz("android.os.Build\$VERSION", + staticFields = listOf("SDK_INT" to IntHolder(sdkInt)) + ) + val leaking = instance(clazz("android.content.Context")) + keyedWeakReference(leaking) + clazz( + "android.content.res.ResourcesImpl", + staticFields = listOf( + "mAppContext" to leaking + ) + ) + } + + val matchers = mutableListOf() + AndroidReferenceMatchers.XIAMI__RESOURCES_IMPL.add(matchers) + val actualMatch = matchers.first() + val analysis = hprofBytes.checkForLeaks(referenceMatchers = matchers) + + if(expectKnown) { + // Expect no application leaks found + assertThat(analysis.applicationLeaks) + .describedAs(analysis.applicationLeaks.toString()).isEmpty() + + // Expect we see a known library leak instead matching the leak pattern we expect. + assertThat(analysis.libraryLeaks).hasSize(1) + val leak = analysis.libraryLeaks.first() + assertThat(leak.pattern).isEqualTo(actualMatch.pattern) + } else { + // Expect that we don't know about this leak + assertThat(analysis.libraryLeaks).isEmpty() + + // And hence it shows up as an application leak. + assertThat(analysis.applicationLeaks) + .describedAs(analysis.applicationLeaks.toString()).hasSize(1) + val leak = analysis.applicationLeaks.first() + + val (expectedClassName, expectedRefName) = leak.leakTraces.first().referencePath.first().let { + it.owningClassName to it.referenceName + } + + val (actualClassName, actualRefName) = (actualMatch.pattern as StaticFieldPattern).let { + it.className to it.fieldName + } + assertThat(expectedClassName).isEqualTo(actualClassName) + assertThat(expectedRefName).isEqualTo(actualRefName) + } + } + + fun ByteArray.checkForLeaks( + objectInspectors: List = emptyList(), + computeRetainedHeapSize: Boolean = false, + referenceMatchers: List = emptyList(), + metadataExtractor: MetadataExtractor = MetadataExtractor.NO_OP, + proguardMapping: ProguardMapping? = null, + leakingObjectFinder: LeakingObjectFinder = FilteringLeakingObjectFinder( + ObjectInspectors.jdkLeakingObjectFilters + ), + ): T { + + val basp = ByteArraySourceProvider(this) + + val inspectors = if (ObjectInspectors.KEYED_WEAK_REFERENCE !in objectInspectors) { + objectInspectors + ObjectInspectors.KEYED_WEAK_REFERENCE + } else { + objectInspectors + } + val heapAnalyzer = HeapAnalyzer(OnAnalysisProgressListener.NO_OP) + + val result = basp.openHeapGraph(proguardMapping).use { graph -> + heapAnalyzer.analyze( + heapDumpFile = File("ignored"), + graph = graph, + leakingObjectFinder = leakingObjectFinder, + referenceMatchers = referenceMatchers, + computeRetainedHeapSize = computeRetainedHeapSize, + objectInspectors = inspectors, + metadataExtractor = metadataExtractor, + ) + } + if (result is HeapAnalysisFailure) { + println(result) + } + @Suppress("UNCHECKED_CAST") + return result as T + } +}