[pyricau]

Four commits for the change, plus a comment on a test:

1. Read 4 byte object ids unsigned, like every other heap dump tool. This is the root fix. Every other change to how ids are spelled had been working around it.

2. Write the remaining object ids in hex. This finishes what Read reasons and inspector labels the way a note is, link notes to source, write ids in hex #2994 started, including deleting ObjectDominators.renderDominatorTree(). It was Write the remaining object ids in hex #3012, which this PR replaces.

3. Print each object's id on its line of a leak trace, ├─ java.util.ArrayList instance 0x12d4cd48, so it can be handed straight to Shark Dive. This is the change this PR was opened for.

4. Name related leaking objects the way a leak trace names its objects. Also retains leaking object 0x12d36d08 (android.app.ContextImpl) is now Also retains android.app.ContextImpl instance 0x12d36d08, the same spelling as the object lines above it. Also retained by …, which has its own leak trace changes the same way. Both go through one internal function with the object line, so the two can't drift apart. From shark-cli analyze on leak_asynctask_o.hprof:

   ​     Also retains android.app.ContextImpl instance 0x12d36d08
   ​     Also retains com.android.internal.policy.PhoneWindow instance 0x12d36b48
   ​     Also retains 5 leaking android.view.ContextThemeWrapper objects
   

   The labels that group several objects of a class keep their wording.

5. Say why the WeakHashMap tests don't use System.runFinalization(). This is test-only. #3018 made clearWeakKeys() in OpenJdkInstanceRefReadersTest wait for the Reference Handler thread before dumping, because HotSpot dumps the reference pending list as a JNI global root. System.runFinalization() looks like it would do the same, and on ART it does: its finalization sentinel goes through the daemon that enqueues cleared references (FinalizerReference.java#118). On HotSpot, which these tests run on, it only runs the finalizers already queued (Finalizer.java#L127-L145). With the Reference Handler suspended for 300 ms around the dump, System.gc() plus System.runFinalization() passed 1 run in 10, the same as System.gc() alone, and waiting for the sentinels passed 10 in 10. The KDoc of clearWeakKeys() now says so.

Fixes #2866.

Reading 4 byte ids unsigned

An object id is the address the heap dump recorded the object at. Android heap dumps record it in 4 bytes, and Shark has read those as a signed Int widened to a Long since its first parser, 5b8ced493 in 2019. That code was copied from perflib's HprofParser.java#228, comment included: "As long as we don't interpret IDs, reading signed values here is fine." As a result, every object above the 2 GB mark got a negative id, 0xffffffff82000010 for the object at 0x82000010.

No other tool spells it that way:

Tool	Shows the object at 0x82000010 as	Given 0xffffffff82000010
Android Studio (Instance.kt#31, HeapDumpInstanceObject.kt#72)	Foo@2181038096 (0x82000010). It reads signed like perflib but masks to 32 bits before displaying.	Has no address input
ahat (Parser.java#975)	Foo@82000010	Throws NumberFormatException
Eclipse MAT (DefaultPositionInputStream.java#L389)	Foo @ 0x82000010	Finds nothing
jhat (HprofReader.java#L519)	Foo@0x82000010	Finds it, because it truncates to an int
VisualVM (HprofByteBuffer.java#L97)	0x82000010 in its Object ID column	Finds nothing

Several workarounds had grown around the negative ids, and this PR removes all of them:

• HeapObject.positiveObjectId is deleted. #2365 added it in 2022 to mask the id the way Android Studio displays it, and it would now return the same value as objectId.
• The six copies of hexObjectId() used to mask the id down to its low 32 bits. Each is now a plain unsigned hex conversion.
• Shark Dive had two spellings of an id. Its files, links and agent surface used all 16 digits (exactHexObjectId), while labels used the 32 bit address. Now there is one spelling.
• Shark Dive read every address typed into a note two ways, because 0x82182c00 could have been either Long. It now reads it one way.
• Shark Dive needed a range check below Int.MIN_VALUE to tell one of its piles of small objects from a real object. It's now a sign check.

The fix is in every place Shark reads an id: HprofRecordReader.readId(), FieldValuesReader, ClassFieldsReader, ByteSubArray, FieldIdReader and HprofPrimitiveArrayStripper.

What changes for consumers:

• 🔀 For an object above the 2 GB mark of a 4 byte heap dump, HeapObject.objectId, ReferenceHolder.value and GC root ids are now the positive address. Objects below the mark keep the ids they had, and 8 byte heap dumps are unaffected.
• ⚠️ HeapObject.positiveObjectId is gone. Use objectId, which is now the same value.
• Leak signatures don't change, since they're built from class and reference names.
• HprofWriter now throws on an id that doesn't fit in the heap dump's id size, where it used to write the low bytes. Such an id would now read back as a different, positive id. One test relied on that truncation: ObjectArrayReferenceReaderTest wrote Long.MAX_VALUE as its dangling reference, and now writes 0xffffffff, which is the same bytes.
• The in-memory index stores ids as offsets from the smallest one, so its keys and the HprofRetainedHeapPerfTest / HprofIOPerfTest numbers are unchanged.
• In Shark Dive (unreleased, so no changelog entry), a verdicts or starred file written before this PR names an object above 2 GB with 16 digits, and that no longer matches anything. notes/decisions.md says so, along with the sed that fixes such a file. If an agent passes in one of those leftover ids, it's refused as "no object of this heap dump", not as a pile.

How often this matters: across the 115 ART heap dumps on my machine (API 22 to 36), 22 have objects above 2 GB, and some of those objects are classes. None of the 3,737 objects in their 556 leak traces was up there. One production dump did fail with Heap dump has an object of class 0x-61544530 …, which is the same bug showing up in an error message.

The remaining hex

#2994 already writes ids in hex in leak traces, HeapObject.toString(), errors, the Shark CLI and the heap explorer. What was left:

• ⚠️ ObjectDominators.renderDominatorTree() is deleted. It named objects by their position in Shark's index (#1234), which no other tool can look up. Nothing called it except HprofRetainedHeapPerfTest, which uses it to explain a failure, so the renderer moved into that test and now prints hex ids. buildDominatorTree() stays.
• Two HprofInMemoryIndex consistency checks now go through the shared helper.
• Two Shark Dive tree errors printed node ids in decimal.

The id on each line of a leak trace

A leak trace names an object by its class and the fields that lead to it. That's enough to read the leak, but not to find the same object again in the heap dump. Each object's line now ends with its id. It's in LeakTraceObject.toString(), so every leak trace Shark prints has it, and in the leak screens of the LeakCanary activity and leakcanary-app.

From the sample app on an Android 16 emulator, built from this branch:

┬───
│ GC Root: Thread object
│
├─ kotlin.concurrent.ThreadsKt$thread$thread$1 instance 0x20a2b50
│    Verdict: Unknown
│    Anonymous subclass of java.lang.Thread
│    Thread name: 'Leaking local variables'
│    ↓ ThreadsKt$thread$thread$1<Java Local>
│                               ~~~~~~~~~~~~
╰→ com.example.leakcanary.MainActivity instance 0x207b608
​     Verdict: Stuck (ObjectWatcher was watching this because com.example.leakcanary.MainActivity received
​     Activity#onDestroy() callback and Activity#mDestroyed is true)
​     Retaining 521.7 kB in 6579 objects
​     ...

The LeakCanary activity's leak screen shows the same line, com.example.leakcanary.MainActivity instance 0x207b608, with the id in the grey of instance.

An earlier version of this PR put the id in a label, Object id: 0x12d6c588. On the API 34 emulator I opened that heap dump in Shark Dive and ran shark-dive --cli describe_object object=<id> on each id it printed: 0x12d6c588 came back as the MainActivity (STUCK), 0x12d4cd98 as the TextView (STUCK), 0x12c823a8 as the LeakingThread (UNKNOWN) and 0x6f9541b0 as the FontsContract class (EXPECTED). The id printed now is the same HeapObject.objectId, just in a different place.

It's display only. LeakTraceObject carries the id in a new objectId property, and only the rendering reads it:

• The leak fingerprint is still built from class and reference names. The rendering tests print the same a49095c3373b957532aff14eb32987bb75ffe9d5 they did before.
• POS's Bugsnag uploader builds its fake stack trace from owningClassName and referenceDisplayName, so its grouping doesn't change. Only the leakTrace text it attaches as metadata gains the ids.
• ⚠️ LeakTraceObject's constructor takes objectId after className, which is a shark ABI change. The constructor already changed in this release when leakingStatus became verdict, so code that builds one has to change anyway. The serialVersionUID is bumped again so that an analysis serialized before this fails to deserialize, rather than reading back an id of 0.

Android Studio's on-device leak trace parser breaks on the new line, though it's broken already. NodeParser.isClassTypeExist expects the last word of ├─ java.util.ArrayList instance to be instance, class or array. That parser has failed on this unreleased version since Leaking: YES became Verdict: Stuck, because it requires a Leaking: line. So this adds one more line for Studio to update, not a new break.

Two smaller things in the same commit:

• The leakcanary-app leak screen drew the leaking object on every row of the path instead of that row's own object, which would have printed the leaking object's id all the way down. It now draws the row's object.
• invalidObjectIdErrorMessage named a missing object UnknownObject0x…. It's now UnknownObject instance 0x…, since the id is on the line anyway.

The docs' sample traces gained ids, set by hand to realistic ART addresses, the same way the Verdict rename edited them. fundamentals-how-leakcanary-works.md now says the id isn't part of the fingerprint.

Tests

• New HighAddressObjectIdsTest: a leak trace through a static field, an instance field and an array entry, all above 0x82000000, finds the leak with every id positive. Writing a sign-widened id throws.
• LeakTraceRenderingTest, HeapAnalysisStringRenderingTest and UnreachableObjectRenderingTest expect the id on every object line.
• The Shark Dive tests for objects above 2 GB (HeapDiveTest, DeepLinkTest, NoteTest, NodeIdsTest) now assert the 32 bit address. AgentToolsTest checks that a leftover 16-digit id is refused as no object.
• These pass locally on JDK 17: the unit tests of shark-hprof, shark-graph, shark, shark-android (including both perf tests), shark-cli, leakcanary-android-core and the four Shark Dive modules, plus detekt and checkKotlinAbi. The ABI dumps change by losing renderDominatorTree and getPositiveObjectId, and by LeakTraceObject gaining objectId. The leakcanary-app and leakcanary-android-core android test sources compile.

🤖 Generated with Claude Code