Skip to content

Commit a6b2310

Browse files
pyricauclaude
pyricau
and
claude
committed
HprofPrimitiveArrayStripper: fix bugs, add StreamingSinkProvider
Fix two correctness bugs: - INSTANCE_DUMP: stackTraceSerialNumber is always u4 (4 bytes), not identifier-sized. This was silently corrupting output for heap dumps with 8-byte identifiers. - PRIMITIVE_ARRAY_DUMP: CopyingSource.transferUnsignedByte() already copies the type byte to the sink — the extra sink.writeByte(type) call was writing it twice, corrupting all primitive array records. Fix a third bug in CopyingSource.transferUtf8: it decoded bytes as UTF-8 into a Java String then re-encoded, which produces different byte counts for Modified UTF-8 sequences (used by HotSpot HPROF). The fix reads the raw bytes and copies them unchanged, only decoding to String for the return value. Other changes: - Add StreamingSinkProvider (mirrors StreamingSourceProvider) so the sink lifecycle is managed inside the function via .use {} - Replace BufferedSink parameter with StreamingSinkProvider in the streaming overload - Refactor parsing into a private stripPrimitiveArrays(CopyingSource) method - Add CopyingSource.indexOf delegate - Also updates all primitive wrapper instance values to 0 (in addition to primitive arrays), as an additional PII safety measure - Add deleteInputHprofFile parameter to delete the input file eagerly after the source is opened, using the Unix trick - Clean up test: replace raw-byte helpers with HprofWriter-based fixture, add JVM heap dump tests for string replacement and wrapper value zeroing Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent c2ddc5a commit a6b2310

File tree

6 files changed

+678
-142
lines changed

6 files changed

+678
-142
lines changed

‎leakcanary/leakcanary-android-release/src/main/java/leakcanary/internal/RealHeapAnalysisJob.kt‎

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import shark.HprofPrimitiveArrayStripper
2929
import shark.OnAnalysisProgressListener
3030
import shark.RandomAccessSource
3131
import shark.SharkLog
32-
import shark.StreamingSourceProvider
3332
import shark.ThrowingCancelableFileSourceProvider
3433

3534
internal class RealHeapAnalysisJob(
@@ -239,24 +238,19 @@ internal class RealHeapAnalysisJob(
239238
checkStopAnalysis("stripping heap dump")
240239
}
241240

242-
var openCalls = 0
243-
val deletingFileSourceProvider = StreamingSourceProvider {
244-
openCalls++
245-
sensitiveSourceProvider.openStreamingSource().apply {
246-
if (openCalls == 2) {
247-
// Using the Unix trick of deleting the file as soon as all readers have opened it.
248-
// No new readers/writers will be able to access the file, but all existing
249-
// ones will still have access until the last one closes the file.
250-
SharkLog.d { "Deleting $sourceHeapDumpFile eagerly" }
251-
sourceHeapDumpFile.delete()
252-
}
253-
}
254-
}
255-
256-
val strippedHprofSink = strippedHeapDumpFile.outputStream().sink().buffer()
257241
val stripper = HprofPrimitiveArrayStripper()
258242

259-
stripper.stripPrimitiveArrays(deletingFileSourceProvider, strippedHprofSink)
243+
stripper.stripPrimitiveArrays(
244+
hprofSourceProvider = sensitiveSourceProvider,
245+
hprofSinkProvider = { strippedHeapDumpFile.outputStream().sink().buffer() },
246+
onDoneOpeningNewSources = {
247+
// Using the Unix trick of deleting the file as soon as all readers have opened it.
248+
// No new readers/writers will be able to access the file, but all existing
249+
// ones will still have access until the last one closes the file.
250+
SharkLog.d { "Deleting $sourceHeapDumpFile eagerly" }
251+
sourceHeapDumpFile.delete()
252+
}
253+
)
260254
}
261255

262256
private fun analyzeHeapWithStats(heapDumpFile: File): Pair<HeapAnalysis, String> {

‎shark/shark-graph/src/test/java/shark/HprofPrimitiveArrayStripperTest.kt‎

Lines changed: 173 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -6,52 +6,50 @@ import org.assertj.core.api.Assertions.assertThat
66
import org.junit.Rule
77
import org.junit.Test
88
import org.junit.rules.TemporaryFolder
9-
import shark.HeapObject.HeapPrimitiveArray
109
import shark.HprofHeapGraph.Companion.openHeapGraph
11-
import shark.HprofRecord.HeapDumpRecord.ObjectRecord.PrimitiveArrayDumpRecord.BooleanArrayDump
10+
import shark.HprofRecord.HeapDumpRecord.ObjectRecord.ClassDumpRecord
11+
import shark.HprofRecord.HeapDumpRecord.ObjectRecord.ClassDumpRecord.FieldRecord
12+
import shark.HprofRecord.HeapDumpRecord.ObjectRecord.InstanceDumpRecord
1213
import shark.HprofRecord.HeapDumpRecord.ObjectRecord.PrimitiveArrayDumpRecord.CharArrayDump
13-
import shark.PrimitiveType.BOOLEAN
14-
import shark.PrimitiveType.CHAR
14+
import shark.HprofRecord.HeapDumpRecord.ObjectRecord.PrimitiveArrayDumpRecord.IntArrayDump
15+
import shark.HprofRecord.HeapDumpRecord.ObjectRecord.PrimitiveArrayDumpRecord.LongArrayDump
16+
import shark.HprofRecord.LoadClassRecord
17+
import shark.HprofRecord.StringRecord
18+
import shark.HprofVersion.ANDROID
1519

1620
class HprofPrimitiveArrayStripperTest {
1721

1822
@get:Rule
1923
var testFolder = TemporaryFolder()
2024

21-
private var lastId = 0L
22-
private val id: Long
23-
get() = ++lastId
24-
2525
@Test
2626
fun stripHprof() {
27-
val booleanArray = BooleanArrayDump(id, 1, booleanArrayOf(true, false, true, true))
28-
val charArray = CharArrayDump(id, 1, "Hello World!".toCharArray())
29-
val hprofBytes = listOf(booleanArray, charArray).asHprofBytes()
30-
31-
val stripper = HprofPrimitiveArrayStripper()
27+
val sourceByteArray =
28+
Buffer()
29+
.apply {
30+
writeRawTestHprof(
31+
secretLongArray = longArrayOf(0xCAFE, 0xDAD),
32+
secretCharArray = charArrayOf('P', 'Y'),
33+
secretWrappedLong = 42,
34+
)
35+
}
36+
.readByteArray()
3237

3338
val strippedBuffer = Buffer()
34-
stripper.stripPrimitiveArrays(hprofBytes, strippedBuffer)
35-
36-
val strippedSource = ByteArraySourceProvider(strippedBuffer.readByteArray())
37-
38-
strippedSource.openHeapGraph().use { graph ->
39-
val booleanArrays = graph.objects
40-
.filter { it is HeapPrimitiveArray && it.primitiveType == BOOLEAN }
41-
.map { it.readRecord() as BooleanArrayDump }
42-
.toList()
43-
assertThat(booleanArrays).hasSize(1)
44-
assertThat(booleanArrays[0].id).isEqualTo(booleanArray.id)
45-
assertThat(booleanArrays[0].array).isEqualTo(booleanArrayOf(false, false, false, false))
46-
47-
val charArrays = graph.objects
48-
.filter { it is HeapPrimitiveArray && it.primitiveType == CHAR }
49-
.map { it.readRecord() as CharArrayDump }
50-
.toList()
51-
assertThat(charArrays).hasSize(1)
52-
assertThat(charArrays[0].id).isEqualTo(charArray.id)
53-
assertThat(charArrays[0].array).isEqualTo("????????????".toCharArray())
54-
}
39+
val stripper = HprofPrimitiveArrayStripper()
40+
stripper.stripPrimitiveArrays(ByteArraySourceProvider(sourceByteArray), { strippedBuffer })
41+
42+
val expectedByteArray =
43+
Buffer()
44+
.apply {
45+
writeRawTestHprof(
46+
secretLongArray = longArrayOf(0, 0),
47+
secretCharArray = charArrayOf('?', '?'),
48+
secretWrappedLong = 0,
49+
)
50+
}
51+
.readByteArray()
52+
assertThat(strippedBuffer.readByteArray()).isEqualTo(expectedByteArray)
5553
}
5654

5755
@Test
@@ -71,15 +69,150 @@ class HprofPrimitiveArrayStripperTest {
7169
assertThat(strippedString).isEqualTo("?".repeat(stringSavedToDump.length))
7270
}
7371

74-
private class TestStringHolder(val string: String)
72+
@Test
73+
fun `input file deleted after stripping`() {
74+
val hprofFolder = testFolder.newFolder()
75+
val hprofFile = File(hprofFolder, "jvm_heap.hprof")
76+
val stringSavedToDump = "Yo!"
77+
hold(TestStringHolder(stringSavedToDump)) {
78+
JvmTestHeapDumper.dumpHeap(hprofFile.absolutePath)
79+
}
7580

76-
private fun File.readHolderString() = openHeapGraph().use { graph ->
77-
val className = "shark.HprofPrimitiveArrayStripperTest\$TestStringHolder"
78-
val holderClass = graph.findClassByName(className)!!
79-
val holderInstance = holderClass.instances.single()
80-
holderInstance[className, "string"]!!.value.readAsJavaString()!!
81+
val strippedFile =
82+
HprofPrimitiveArrayStripper().stripPrimitiveArrays(hprofFile, deleteInputHprofFile = true)
83+
assertThat(hprofFile.exists()).isFalse()
84+
// Ensures stripped file exists and can be read
85+
assertThat(strippedFile.readHolderString()).isEqualTo("?".repeat(stringSavedToDump.length))
8186
}
8287

88+
class Secret(val secretArray: IntArray) {
89+
val secretList: List<Int> = secretArray.toList()
90+
}
91+
92+
@Test
93+
fun `Primitive Wrapper Types wrap 0`() {
94+
val hprofFolder = testFolder.newFolder()
95+
val hprofFile = File(hprofFolder, "jvm_heap.hprof")
96+
val inMemorySecretArray = intArrayOf(0xCAFE, 0xDAD)
97+
val secret = Secret(inMemorySecretArray)
98+
hold(secret) { JvmTestHeapDumper.dumpHeap(hprofFile.absolutePath) }
99+
100+
val strippedFile = HprofPrimitiveArrayStripper().stripPrimitiveArrays(hprofFile)
101+
102+
val (secretArray, secretListArray) = hprofFile.readSecretInArrays()
103+
val (strippedSecretArray, strippedSecretListArray) = strippedFile.readSecretInArrays()
104+
105+
assertThat(secretArray).isEqualTo(inMemorySecretArray)
106+
assertThat(secretListArray).isEqualTo(inMemorySecretArray)
107+
108+
val arrayOfZeros = IntArray(inMemorySecretArray.size)
109+
assertThat(strippedSecretArray).isEqualTo(arrayOfZeros)
110+
assertThat(strippedSecretListArray).isEqualTo(arrayOfZeros)
111+
}
112+
113+
private fun Buffer.writeRawTestHprof(
114+
secretLongArray: LongArray,
115+
secretCharArray: CharArray,
116+
secretWrappedLong: Byte,
117+
) {
118+
HprofWriter.openWriterFor(
119+
this,
120+
hprofHeader = HprofHeader(heapDumpTimestamp = 42, version = ANDROID, identifierByteSize = 4),
121+
)
122+
.use { writer ->
123+
writer.write(StringRecord(id = 1, string = "java.lang.Object"))
124+
writer.write(StringRecord(id = 2, string = "java.lang.Long"))
125+
writer.write(StringRecord(id = 3, string = "value"))
126+
writer.write(
127+
LoadClassRecord(
128+
classSerialNumber = 0,
129+
id = 1,
130+
stackTraceSerialNumber = 0,
131+
classNameStringId = 1,
132+
)
133+
)
134+
writer.write(
135+
LoadClassRecord(
136+
classSerialNumber = 0,
137+
id = 2,
138+
stackTraceSerialNumber = 0,
139+
classNameStringId = 2,
140+
)
141+
)
142+
writer.write(
143+
ClassDumpRecord(
144+
id = 1,
145+
stackTraceSerialNumber = 0,
146+
superclassId = 0,
147+
classLoaderId = 0,
148+
signersId = 0,
149+
protectionDomainId = 0,
150+
instanceSize = 0,
151+
staticFields = emptyList(),
152+
fields = emptyList(),
153+
)
154+
)
155+
writer.write(
156+
ClassDumpRecord(
157+
id = 2,
158+
stackTraceSerialNumber = 0,
159+
superclassId = 1,
160+
classLoaderId = 0,
161+
signersId = 0,
162+
protectionDomainId = 0,
163+
instanceSize = 0,
164+
staticFields = emptyList(),
165+
fields = listOf(FieldRecord(3, PrimitiveType.LONG.hprofType)),
166+
)
167+
)
168+
writer.write(LongArrayDump(id = 4, stackTraceSerialNumber = 0, array = secretLongArray))
169+
writer.write(
170+
InstanceDumpRecord(
171+
id = 5,
172+
classId = 2,
173+
stackTraceSerialNumber = 0,
174+
fieldValues = byteArrayOf(0, 0, 0, 0, 0, 0, 0, secretWrappedLong),
175+
)
176+
)
177+
writer.write(CharArrayDump(id = 6, stackTraceSerialNumber = 0, array = secretCharArray))
178+
}
179+
}
180+
181+
private fun File.readSecretInArrays(): Pair<IntArray, IntArray> {
182+
return openHeapGraph().use { graph ->
183+
val className = Secret::class.java.name
184+
val secretInstance = graph.findClassByName(className)!!.instances.single()
185+
val secretArray =
186+
(secretInstance[className, "secretArray"]!!.valueAsPrimitiveArray!!.readRecord()
187+
as IntArrayDump)
188+
.array
189+
val secretListArray =
190+
secretInstance[className, "secretList"]!!
191+
.valueAsInstance!![ArrayList::class.java.name, "elementData"]!!
192+
.valueAsObjectArray!!
193+
.readElements()
194+
.map { arrayElement ->
195+
arrayElement.asObject!!
196+
.asInstance!![Int::class.javaObjectType.name, "value"]!!
197+
.value
198+
.asInt!!
199+
}
200+
.toList()
201+
.toIntArray()
202+
secretArray to secretListArray
203+
}
204+
}
205+
206+
private class TestStringHolder(val string: String)
207+
208+
private fun File.readHolderString() =
209+
openHeapGraph().use { graph ->
210+
val className = "shark.HprofPrimitiveArrayStripperTest\$TestStringHolder"
211+
val holderClass = graph.findClassByName(className)!!
212+
val holderInstance = holderClass.instances.single()
213+
holderInstance[className, "string"]!!.value.readAsJavaString()!!
214+
}
215+
83216
private fun hold(held: Any, block: () -> Unit) {
84217
try {
85218
block()

‎shark/shark-hprof/api/shark-hprof.api‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,24 @@ public final class shark/ConstantMemoryMetricsDualSourceProvider : shark/DualSou
1414
public fun openStreamingSource ()Lokio/BufferedSource;
1515
}
1616

17+
public final class shark/CopyingSource {
18+
public fun <init> (Lokio/BufferedSource;Lokio/BufferedSink;)V
19+
public final fun exhausted ()Z
20+
public final fun getBytesRead ()J
21+
public final fun indexOf (B)J
22+
public final fun overwrite ([B)V
23+
public final fun transfer (I)V
24+
public final fun transfer (J)V
25+
public final fun transferByte ()B
26+
public final fun transferInt ()I
27+
public final fun transferLong ()J
28+
public final fun transferShort ()I
29+
public final fun transferUnsignedByte ()I
30+
public final fun transferUnsignedInt ()J
31+
public final fun transferUnsignedShort ()I
32+
public final fun transferUtf8 (J)Ljava/lang/String;
33+
}
34+
1735
public abstract interface class shark/DualSourceProvider : shark/RandomAccessSourceProvider, shark/StreamingSourceProvider {
1836
}
1937

@@ -150,9 +168,10 @@ public final class shark/HprofHeader$Companion {
150168

151169
public final class shark/HprofPrimitiveArrayStripper {
152170
public fun <init> ()V
153-
public final fun stripPrimitiveArrays (Ljava/io/File;Ljava/io/File;)Ljava/io/File;
154-
public final fun stripPrimitiveArrays (Lshark/StreamingSourceProvider;Lokio/BufferedSink;)V
155-
public static synthetic fun stripPrimitiveArrays$default (Lshark/HprofPrimitiveArrayStripper;Ljava/io/File;Ljava/io/File;ILjava/lang/Object;)Ljava/io/File;
171+
public final fun stripPrimitiveArrays (Ljava/io/File;Ljava/io/File;Z)Ljava/io/File;
172+
public final fun stripPrimitiveArrays (Lshark/StreamingSourceProvider;Lshark/StreamingSinkProvider;Lkotlin/jvm/functions/Function0;)V
173+
public static synthetic fun stripPrimitiveArrays$default (Lshark/HprofPrimitiveArrayStripper;Ljava/io/File;Ljava/io/File;ZILjava/lang/Object;)Ljava/io/File;
174+
public static synthetic fun stripPrimitiveArrays$default (Lshark/HprofPrimitiveArrayStripper;Lshark/StreamingSourceProvider;Lshark/StreamingSinkProvider;Lkotlin/jvm/functions/Function0;ILjava/lang/Object;)V
156175
}
157176

158177
public abstract class shark/HprofRecord {
@@ -583,6 +602,10 @@ public final class shark/StreamingRecordReaderAdapter$Companion {
583602
public final fun asStreamingRecordReader (Lshark/StreamingHprofReader;)Lshark/StreamingRecordReaderAdapter;
584603
}
585604

605+
public abstract interface class shark/StreamingSinkProvider {
606+
public abstract fun openStreamingSink ()Lokio/BufferedSink;
607+
}
608+
586609
public abstract interface class shark/StreamingSourceProvider {
587610
public abstract fun openStreamingSource ()Lokio/BufferedSource;
588611
}

0 commit comments

Comments
 (0)