Skip to content

Commit 58a5c1c

Browse files
blundellclaude
andcommitted
Fix COMPOSITION_IMPL inspector for new Compose state field structure
Fixes NullPointerException when analyzing heap dumps from newer Compose versions where CompositionImpl uses "state" int field instead of "disposed" boolean field. The inspector now gracefully handles both field structures: - Old Compose: disposed boolean field (backward compatibility) - New Compose: state int field with values RUNNING=0, DISPOSED=3, etc. - Edge case: neither field exists (graceful fallback) Before: NullPointerException at AndroidObjectInspectors$COMPOSITION_IMPL$inspect line 792 After: Analysis completes successfully with proper state detection Fixes: #2781 Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 6de01db commit 58a5c1c

File tree

3 files changed

+99
-5
lines changed

3 files changed

+99
-5
lines changed

docs/dev-env.md

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,28 @@
77
* Running the failing UI tests to confirm leak detection correctly fails UI tests: `./gradlew leakcanary-android-sample:connectedCheck`.
88
* Normal UI tests: `./gradlew leakcanary-android-core:connectedCheck`.
99

10-
## Static Code Analysis
10+
## Static Code Analysis
1111
* LeakCanary [uses](https://github.com/square/leakcanary/pull/1535) [Detekt](https://arturbosch.github.io/detekt/) for static Code analysis.
1212
* 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**.
1313
* 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!
14-
* You can bypass the git hook though; Travis CI will still run checks and will fail if any violations are found.
14+
* You can bypass the git hook though; Travis CI will still run checks and will fail if any violations are found.
1515
* Detekt report will be printed in the console and saved to `/moduleDir/build/reports/`.
1616

17+
## Unit Testing with Synthetic Heap Dumps
18+
19+
When testing heap analysis functionality, create synthetic heap dumps programmatically instead of committing binary `.hprof` files:
20+
21+
```kotlin
22+
val heapDump = dump {
23+
"com.example.MyClass" watchedInstance {
24+
field["fieldName"] = BooleanHolder(true)
25+
}
26+
}
27+
```
28+
29+
* Requires `testImplementation(projects.shark.sharkHprofTest)`.
30+
* See `LeakStatusTest` or `AndroidObjectInspectorsTest` for examples.
31+
1732
## Deploying locally
1833

1934
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

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -789,10 +789,30 @@ enum class AndroidObjectInspectors : ObjectInspector {
789789
COMPOSITION_IMPL {
790790
override fun inspect(reporter: ObjectReporter) {
791791
reporter.whenInstanceOf("androidx.compose.runtime.CompositionImpl") { instance ->
792-
if (instance["androidx.compose.runtime.CompositionImpl", "disposed"]!!.value.asBoolean!!) {
793-
leakingReasons += "Composition disposed"
792+
// Try the old "disposed" boolean field first (older Compose versions)
793+
val disposedField = instance["androidx.compose.runtime.CompositionImpl", "disposed"]
794+
if (disposedField != null) {
795+
if (disposedField.value.asBoolean!!) {
796+
leakingReasons += "Composition disposed"
797+
} else {
798+
notLeakingReasons += "Composition not disposed"
799+
}
794800
} else {
795-
notLeakingReasons += "Composition not disposed"
801+
// Try the new "state" field (newer Compose versions)
802+
// State is an int with constants: RUNNING=0, DEACTIVATED=1, INCONSISTENT=2, DISPOSED=3
803+
val stateField = instance["androidx.compose.runtime.CompositionImpl", "state"]
804+
if (stateField != null) {
805+
when (val stateValue = stateField.value.asInt!!) {
806+
3 -> leakingReasons += "Composition disposed" // DISPOSED state
807+
0 -> notLeakingReasons += "Composition running" // RUNNING state
808+
1 -> labels += "Composition deactivated" // DEACTIVATED state
809+
2 -> labels += "Composition inconsistent" // INCONSISTENT state
810+
else -> labels += "Composition state: $stateValue"
811+
}
812+
} else {
813+
// Neither field exists - can't determine composition state
814+
labels += "Composition disposal state unknown (no disposed or state field)"
815+
}
796816
}
797817
}
798818
}

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

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
package shark
22

3+
import java.io.File
34
import org.assertj.core.api.Assertions.assertThat
45
import org.junit.Test
6+
import shark.HprofHeapGraph.Companion.openHeapGraph
57
import shark.LeakTraceObject.LeakingStatus.LEAKING
68
import shark.LeakTraceObject.LeakingStatus.NOT_LEAKING
79
import shark.LeakTraceObject.ObjectType.INSTANCE
10+
import shark.ValueHolder.BooleanHolder
11+
import shark.ValueHolder.IntHolder
812

913
class AndroidObjectInspectorsTest {
1014

@@ -65,4 +69,59 @@ class AndroidObjectInspectorsTest {
6569
assertThat(recomposerNode.originObject.leakingStatus).isEqualTo(LEAKING)
6670
assertThat(recomposerNode.originObject.leakingStatusReason).isEqualTo("Composition disposed")
6771
}
72+
73+
@Test fun `COMPOSITION_IMPL with old disposed field true should be leaking`() {
74+
val analysis = analyzeCompositionImpl(mapOf("disposed" to BooleanHolder(true)))
75+
val unreachableObject = analysis.unreachableObjects.single()
76+
77+
assertThat(unreachableObject.leakingStatus).isEqualTo(LEAKING)
78+
assertThat(unreachableObject.leakingStatusReason)
79+
.contains("Composition disposed")
80+
}
81+
82+
@Test fun `COMPOSITION_IMPL with new state field DISPOSED should be leaking`() {
83+
val analysis = analyzeCompositionImpl(mapOf("state" to IntHolder(3))) // DISPOSED = 3
84+
val unreachableObject = analysis.unreachableObjects.single()
85+
86+
assertThat(unreachableObject.leakingStatus).isEqualTo(LEAKING)
87+
assertThat(unreachableObject.leakingStatusReason)
88+
.contains("Composition disposed")
89+
}
90+
91+
@Test fun `COMPOSITION_IMPL with new state field RUNNING should not be leaking`() {
92+
val analysis = analyzeCompositionImpl(mapOf("state" to IntHolder(0))) // RUNNING = 0
93+
val unreachableObject = analysis.unreachableObjects.single()
94+
95+
// Note: Status is still LEAKING because this is a watchedInstance (tracked by ObjectWatcher)
96+
// but the inspector provides the correct reason explaining why it's not actually leaking
97+
assertThat(unreachableObject.leakingStatus).isEqualTo(LEAKING)
98+
assertThat(unreachableObject.leakingStatusReason)
99+
.contains("Composition running")
100+
}
101+
102+
private fun analyzeCompositionImpl(fields: Map<String, ValueHolder>): HeapAnalysisSuccess {
103+
val heapDump = dump {
104+
"androidx.compose.runtime.CompositionImpl" watchedInstance {
105+
fields.forEach { (name, value) ->
106+
field[name] = value
107+
}
108+
}
109+
}
110+
111+
val heapAnalyzer = HeapAnalyzer(OnAnalysisProgressListener.NO_OP)
112+
113+
return heapDump.openHeapGraph().use { graph: HeapGraph ->
114+
heapAnalyzer.analyze(
115+
heapDumpFile = File("/no/file"),
116+
graph = graph,
117+
leakingObjectFinder = FilteringLeakingObjectFinder(
118+
ObjectInspectors.jdkLeakingObjectFilters
119+
),
120+
referenceMatchers = JdkReferenceMatchers.defaults,
121+
computeRetainedHeapSize = false,
122+
objectInspectors = listOf(ObjectInspectors.KEYED_WEAK_REFERENCE, AndroidObjectInspectors.COMPOSITION_IMPL),
123+
metadataExtractor = MetadataExtractor.NO_OP
124+
)
125+
} as HeapAnalysisSuccess
126+
}
68127
}

0 commit comments

Comments
 (0)