Merge pull request #2797 from blundell/fix-compose-inspector-main

Fix COMPOSITION_IMPL inspector for new Compose state field structure

Author: pyricau

docs/dev-env.md

@@ -7,13 +7,28 @@
 * Running the failing UI tests to confirm leak detection correctly fails UI tests: `./gradlew leakcanary-android-sample:connectedCheck`.
 * Normal UI tests: `./gradlew leakcanary-android-core:connectedCheck`.
 
-## Static Code Analysis 
+## Static Code Analysis
 * LeakCanary [uses](https://github.com/square/leakcanary/pull/1535) [Detekt](https://arturbosch.github.io/detekt/) for static Code analysis.
 * Analyze the entire project with `./gradlew check` or particular modules with `./gradlew :module-name:check`. Detekt will fail the build if any ruleset violations are found. **You should fix all issues before pushing the branch to remote**.
   * There's also a **git pre-push** hook that will run analysis automatically before pushing a branch to the remote. If there are any violations - it will prevent the push. Fix the issues!
-  * You can bypass the git hook though; Travis CI will still run checks and will fail if any violations are found. 
+  * You can bypass the git hook though; Travis CI will still run checks and will fail if any violations are found.
 * Detekt report will be printed in the console and saved to `/moduleDir/build/reports/`.
 
+## Unit Testing with Synthetic Heap Dumps
+
+When testing heap analysis functionality, create synthetic heap dumps programmatically instead of committing binary `.hprof` files:
+
+```kotlin
+val heapDump = dump {
+  "com.example.MyClass" watchedInstance {
+    field["fieldName"] = BooleanHolder(true)
+  }
+}
+```
+
+* Requires `testImplementation(projects.shark.sharkHprofTest)`.
+* See `LeakStatusTest` or `AndroidObjectInspectorsTest` for examples.
+
 ## Deploying locally
 
 To deploy LeakCanary to your local maven repository, run the following command, changing the path to the path of your local repository:

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

@@ -789,10 +789,30 @@ enum class AndroidObjectInspectors : ObjectInspector {
   COMPOSITION_IMPL {
     override fun inspect(reporter: ObjectReporter) {
       reporter.whenInstanceOf("androidx.compose.runtime.CompositionImpl") { instance ->
-        if (instance["androidx.compose.runtime.CompositionImpl", "disposed"]!!.value.asBoolean!!) {
-          leakingReasons += "Composition disposed"
+        // Try the old "disposed" boolean field first (older Compose versions)
+        val disposedField = instance["androidx.compose.runtime.CompositionImpl", "disposed"]
+        if (disposedField != null) {
+          if (disposedField.value.asBoolean!!) {
+            leakingReasons += "Composition disposed"
+          } else {
+            notLeakingReasons += "Composition not disposed"
+          }
         } else {
-          notLeakingReasons += "Composition not disposed"
+          // Try the new "state" field (newer Compose versions)
+          // State is an int with constants: RUNNING=0, DEACTIVATED=1, INCONSISTENT=2, DISPOSED=3
+          val stateField = instance["androidx.compose.runtime.CompositionImpl", "state"]
+          if (stateField != null) {
+            when (val stateValue = stateField.value.asInt!!) {
+              3 -> leakingReasons += "Composition disposed" // DISPOSED state
+              0 -> notLeakingReasons += "Composition running" // RUNNING state
+              1 -> labels += "Composition deactivated" // DEACTIVATED state
+              2 -> labels += "Composition inconsistent" // INCONSISTENT state
+              else -> labels += "Composition state: $stateValue"
+            }
+          } else {
+            // Neither field exists - can't determine composition state
+            labels += "Composition disposal state unknown (no disposed or state field)"
+          }
         }
       }
     }

shark/shark-android/src/test/java/shark/AndroidObjectInspectorsTest.kt

@@ -1,10 +1,14 @@
 package shark
 
+import java.io.File
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.Test
+import shark.HprofHeapGraph.Companion.openHeapGraph
 import shark.LeakTraceObject.LeakingStatus.LEAKING
 import shark.LeakTraceObject.LeakingStatus.NOT_LEAKING
 import shark.LeakTraceObject.ObjectType.INSTANCE
+import shark.ValueHolder.BooleanHolder
+import shark.ValueHolder.IntHolder
 
 class AndroidObjectInspectorsTest {
 
@@ -65,4 +69,59 @@ class AndroidObjectInspectorsTest {
     assertThat(recomposerNode.originObject.leakingStatus).isEqualTo(LEAKING)
     assertThat(recomposerNode.originObject.leakingStatusReason).isEqualTo("Composition disposed")
   }
+
+  @Test fun `COMPOSITION_IMPL with old disposed field true should be leaking`() {
+    val analysis = analyzeCompositionImpl(mapOf("disposed" to BooleanHolder(true)))
+    val unreachableObject = analysis.unreachableObjects.single()
+
+    assertThat(unreachableObject.leakingStatus).isEqualTo(LEAKING)
+    assertThat(unreachableObject.leakingStatusReason)
+      .contains("Composition disposed")
+  }
+
+  @Test fun `COMPOSITION_IMPL with new state field DISPOSED should be leaking`() {
+    val analysis = analyzeCompositionImpl(mapOf("state" to IntHolder(3))) // DISPOSED = 3
+    val unreachableObject = analysis.unreachableObjects.single()
+
+    assertThat(unreachableObject.leakingStatus).isEqualTo(LEAKING)
+    assertThat(unreachableObject.leakingStatusReason)
+      .contains("Composition disposed")
+  }
+
+  @Test fun `COMPOSITION_IMPL with new state field RUNNING should not be leaking`() {
+    val analysis = analyzeCompositionImpl(mapOf("state" to IntHolder(0))) // RUNNING = 0
+    val unreachableObject = analysis.unreachableObjects.single()
+
+    // Note: Status is still LEAKING because this is a watchedInstance (tracked by ObjectWatcher)
+    // but the inspector provides the correct reason explaining why it's not actually leaking
+    assertThat(unreachableObject.leakingStatus).isEqualTo(LEAKING)
+    assertThat(unreachableObject.leakingStatusReason)
+      .contains("Composition running")
+  }
+
+  private fun analyzeCompositionImpl(fields: Map<String, ValueHolder>): HeapAnalysisSuccess {
+    val heapDump = dump {
+      "androidx.compose.runtime.CompositionImpl" watchedInstance {
+        fields.forEach { (name, value) ->
+          field[name] = value
+        }
+      }
+    }
+
+    val heapAnalyzer = HeapAnalyzer(OnAnalysisProgressListener.NO_OP)
+
+    return heapDump.openHeapGraph().use { graph: HeapGraph ->
+      heapAnalyzer.analyze(
+        heapDumpFile = File("/no/file"),
+        graph = graph,
+        leakingObjectFinder = FilteringLeakingObjectFinder(
+          ObjectInspectors.jdkLeakingObjectFilters
+        ),
+        referenceMatchers = JdkReferenceMatchers.defaults,
+        computeRetainedHeapSize = false,
+        objectInspectors = listOf(ObjectInspectors.KEYED_WEAK_REFERENCE, AndroidObjectInspectors.COMPOSITION_IMPL),
+        metadataExtractor = MetadataExtractor.NO_OP
+      )
+    } as HeapAnalysisSuccess
+  }
 }