Better support for ActivityThread.mNewActivities

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
       }
     }
   },
@@ -559,6 +562,8 @@ enum class AndroidReferenceMatchers {
     }
   },
 
+
+
   BIOMETRIC_PROMPT {
     override fun add(references: MutableList<ReferenceMatcher>) {
       references += instanceFieldLeak(

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<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 =