Merge pull request #2547 from square/py/backport_leaks

Bring #2545 to main

config/detekt-config.yml

@@ -513,8 +513,7 @@ style:
   RedundantVisibilityModifierRule:
     active: false
   ReturnCount:
-    #LeakCanary - increased from 2 to 4
-    active: true
+    active: false
     max: 4
     excludedFunctions: "equals"
     excludeLabeled: false

shark/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,10 +93,12 @@ 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;
@@ -106,6 +110,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;
@@ -147,6 +152,7 @@ public abstract class shark/AndroidReferenceMatchers : java/lang/Enum {
 	public static final field SPEN_GESTURE_MANAGER Lshark/AndroidReferenceMatchers;
 	public static final field STATIC_MTARGET_VIEW Lshark/AndroidReferenceMatchers;
 	public static final field SYSTEM_SENSOR_MANAGER__MAPPCONTEXTIMPL Lshark/AndroidReferenceMatchers;
+	public static final field TES Ljava/lang/String;
 	public static final field TEXT_LINE__SCACHED Lshark/AndroidReferenceMatchers;
 	public static final field TEXT_TO_SPEECH Lshark/AndroidReferenceMatchers;
 	public static final field TEXT_VIEW__MLAST_HOVERED_VIEW Lshark/AndroidReferenceMatchers;

shark/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

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

@@ -72,18 +72,21 @@ enum class AndroidReferenceMatchers {
     }
   },
 
+  /**
+   * See AndroidReferenceReaders.ACTIVITY_THREAD__NEW_ACTIVITIES for more context
+   */
   ACTIVITY_THREAD__M_NEW_ACTIVITIES {
     override fun add(
       references: MutableList<ReferenceMatcher>
     ) {
       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
       }
     }
   },
@@ -245,7 +248,7 @@ enum class AndroidReferenceMatchers {
               InputMethodManager.mImeInsetsConsumer isn't set to null when the activity is destroyed.
             """.trimIndent()
       ) {
-        sdkInt == 31
+        sdkInt >= 31
       }
     }
   },
@@ -541,7 +544,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 +817,17 @@ enum class AndroidReferenceMatchers {
     override fun add(
       references: MutableList<ReferenceMatcher>
     ) {
-      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 +900,60 @@ enum class AndroidReferenceMatchers {
     }
   },
 
+  FLIPPER__APPLICATION_DESCRIPTOR {
+    override fun add(
+      references: MutableList<ReferenceMatcher>
+    ) {
+      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<ReferenceMatcher>) {
+      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<ReferenceMatcher>
+    ) {
+      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<ReferenceMatcher>) {
+      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 +1378,15 @@ enum class AndroidReferenceMatchers {
     override fun add(
       references: MutableList<ReferenceMatcher>
     ) {
-      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
       }
     }
   },
@@ -1390,7 +1449,7 @@ enum class AndroidReferenceMatchers {
           and that field ends up referencing lower contexts (e.g. service).
         """.trimIndent()
       ) {
-        listOf(HMD_GLOBAL, INFINIX, LENOVO, XIAOMI).contains(manufacturer) &&
+        listOf(HMD_GLOBAL, INFINIX, LENOVO, XIAOMI, TES).contains(manufacturer) &&
           sdkInt >= 30
       }
     }
@@ -1497,7 +1556,7 @@ enum class AndroidReferenceMatchers {
     const val XIAOMI = "Xiaomi"
     const val HMD_GLOBAL = "HMD Global"
     const val INFINIX = "INFINIX"
-
+    const val TES = "TES"
 
     /**
      * Returns a list of [ReferenceMatcher] that only contains [IgnoredReferenceMatcher] and no

shark/shark/api/shark.api

@@ -19,6 +19,7 @@ public final class shark/AndroidReferenceReaderFactory : shark/ReferenceReader$F
 }
 
 public abstract class shark/AndroidReferenceReaders : java/lang/Enum, shark/ChainingInstanceReferenceReader$VirtualInstanceReferenceReader$OptionalFactory {
+	public static final field ACTIVITY_THREAD__NEW_ACTIVITIES Lshark/AndroidReferenceReaders;
 	public static final field ANIMATOR_WEAK_REF_SUCKS Lshark/AndroidReferenceReaders;
 	public static final field ARRAY_SET Lshark/AndroidReferenceReaders;
 	public static final field Companion Lshark/AndroidReferenceReaders$Companion;

shark/shark/src/main/java/shark/AndroidReferenceReaders.kt

@@ -1,6 +1,9 @@
 package shark
 
 import shark.HeapObject.HeapInstance
+import shark.LibraryLeakReferenceMatcher
+import shark.ReferencePattern.InstanceFieldPattern
+import shark.ValueHolder
 import shark.ValueHolder.ReferenceHolder
 import shark.ChainingInstanceReferenceReader.VirtualInstanceReferenceReader
 import shark.ChainingInstanceReferenceReader.VirtualInstanceReferenceReader.OptionalFactory
@@ -10,6 +13,141 @@ import shark.ReferenceLocationType.INSTANCE_FIELD
 
 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<Reference> {
+          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<Long?>(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 =