From 754444d79c926f9ad040e7c2108434d7aa149df8 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Aug 2026 10:55:35 +0000 Subject: [PATCH] docs(runtime): give the retention witness its section, and take back the comment that outlived its method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The remainder PR #280 left behind (#334). Three items, no code restored. **1. A doc comment outlived its method.** `0753a45` extracted the witness "minus the dominator tree" and removed the method but kept its ``, which C# then bound to the next member regardless of the blank line. `Dispose()` was documented as *"the dominator tree of the whole live graph, with retained sizes"* — the file advertising the one capability the extraction removed. Deleted. **2. `RetentionPath` was the only runtime tool with no README section**, while `README.md:27` kept advertising *"Heap analysis (retained, duplicates, retention paths) — ClrMD"* in the stack table. It now has one, covering what the two verbs answer and why the split matters: `census` first, because `ClrHeap.EnumerateObjects()` returns uncollected garbage too and a big heap is not evidence of a leak; then `roots`, with the field names that turn "this object is alive" into "this field is holding it". Including the parts that constrain trust rather than advertise: shares are shares of the RESOLVED SAMPLE and never of the population; the verdict is computed over every instance so no display budget can move it; a `[stack]` root is liveness, not retention; the type matcher deliberately refuses `System.Func<…GTDGoody…>` because it once confidently reported a 2-hop path to the wrong object. Plus the exit-code table and why exit 2 is a tier of its own. The Layout block and the Status list were both missing the tool as well; both now name it, and Status records the dominator tree under **Deferred** rather than leaving it unmentioned. **3. The dangling pointer.** `Heap.cs:34` told the reader "See the README" about the dominator tree, and the README had nothing to see. It now points at the section that exists, says plainly that dominance is NOT implemented here, and sends anyone who wants the reasoning to #334 — where the extracted implementation's argument is recorded. `Dominators.cs` is not restored, per the issue. The comment in the csproj that names the exclusion stays; it is the record of a decision, and the only thing wrong with it was that everything it pointed at was missing. One correction to #334 itself: it located the dangling pointer in `Program.cs`. It is in `Heap.cs:34`. `Program.cs` mentions the dominator tree too, but as an aside without a cross-reference, so it needed nothing. Verified: build 0 warnings / 0 errors, classifier selftest 16/16, ingest selftest 26/26, tests/run_tests.py wpf 28/28. Checked that all four runtime tools now have a README section, and that no dominator code came back. Closes #334. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016Lmcv3X9PoELp8CGDfNc9m --- audit/runtime/README.md | 111 +++++++++++++++++++++++++++- audit/runtime/RetentionPath/Heap.cs | 10 +-- 2 files changed, 113 insertions(+), 8 deletions(-) diff --git a/audit/runtime/README.md b/audit/runtime/README.md index cb4ebb9f..2a14dea5 100644 --- a/audit/runtime/README.md +++ b/audit/runtime/README.md @@ -44,6 +44,10 @@ audit/runtime/ PropertyChangedStorm/ # C# PropertyChanged-storm profiler — Windows/build-required, NOT CI-gated PropertyChangedStorm.csproj # net472; Microsoft.Diagnostics.Tracing.TraceEvent Program.cs # TraceEvent over an .etl: per-property raise frequency, storm findings + RetentionPath/ # C# retention witness — net8.0, CROSS-PLATFORM, build smoked in CI + RetentionPath.csproj # net8.0; Microsoft.Diagnostics.Runtime (ClrMD 3.x) + Newtonsoft.Json + Program.cs # census / roots verbs, exit-code tiers, runtime.json writer + Heap.cs # mark-from-roots, BFS root->object paths with field names ``` ## How the leak-harness works (Plan.md §4.1) @@ -121,6 +125,105 @@ python audit/runtime/ingest.py --propertychanged-storm \ # a located storm clusters with a static INPC0xx in the same file. ``` +## Retention paths — is it retained, and by whom (Plan.md §4) + +The stack table above has promised *"Heap analysis (retained, duplicates, **retention +paths**) — ClrMD"* since Plan.md §4. `RetentionPath/` is that half. `HeapCounter` +counts instances of named types; this answers the two questions that actually decide +a leak hunt — *is any of it retained at all*, and *who is holding it*. + +Unlike its three neighbours it is **net8.0 and cross-platform**, needs no procdump, +and can attach to a live PID. CI builds it and smokes its usage surface on every +push; the end-to-end demo runs on Linux and the WPF pair on Windows. + +### `census` — is there anything to hunt? + +```console +$ retention-path census --pid 1234 --out runtime.json + +roots : 308 objects +on the heap : 4 270 155 objects 573 MB +REACHABLE from roots : 4 144 653 objects 403 MB +uncollected garbage : 125 502 objects 170 MB +>>> 70,4% of the heap is genuinely RETAINED — something holds it; run `roots` +``` + +**This distinction is not pedantry.** `ClrHeap.EnumerateObjects()` walks the heap +segments linearly and returns *everything allocated, including garbage the GC has +not collected yet*. A big heap is not evidence of a leak. `HeapCounter` mitigates +that by forcing a GC in the target first, which works when you can drive the target; +marking from the roots answers it directly and needs no cooperation. If the retained +share is low, **stop** — there is no reference to hunt, and the next question is +about GC timing, not about who holds what. + +### `roots` — what holds the TYPICAL instance + +```console +$ retention-path roots --pid 1234 --type GTDGoody + +verdict: RETAINED — BrokerDataClasses.GTDGoody: 130 000 on the heap, 129 903 reachable … + +#1 25/50 resolved (50,0%) — via [static-event], 7 hops + [PinnedHandle] System.Object[] + BrokerDataClasses.GTD + BrokerDataClasses.CalcProcentGTD (.k__BackingField) + BrokerDataClasses.GTDGoody (.fMainObject) +``` + +The field names come from ClrMD's `EnumerateReferencesWithFields`; they are what turn +*"this object is alive"* into *"**this field** is holding it"* — the sentence a +developer can act on. A **delegate hop** in the path is what makes it a static +*event* rather than a plain static field, which is the distinction OwnAudit's +`correlate.py` keys its `high` tier on. + +**Why it samples, and what the percentages mean.** *"Who holds this object"* is +ill-posed for an object reachable from many roots: there are as many answers as +there are paths, and the shortest is an arbitrary pick rather than an explanation. +So the walk resolves a SAMPLE of paths and ranks them as a histogram — the retainer +accounting for 129 900 of 130 000 is the leak; the three hanging off the stack are +noise. The shares are shares **of the resolved sample**, never of the population. +The verdict is not sampled: reachability and the durable/transient census run over +every instance, so no display budget (`--sample`, `--max-hops`) can change the +diagnosis or the exit code. + +### What it does not do — read this before trusting it + +- **No dominator tree.** The principled form of "who holds it" is dominance: which + single reference, if cut, makes the object collectable, and how much that frees. + It also answers honestly when two references hold an object jointly, naming the + point where the paths meet instead of picking one. That is what Eclipse MAT and + dotMemory are built on. The A3 witness was extracted from PR #280 **without** it, + deliberately; Own.NET#334 records what the implementation argued. The sampled + histogram is a weaker instrument, and the ranking is how it stays honest about it. +- **A `[stack]` root is not retention** — the object is live in a frame right now, + and is labelled so it is not mistaken for a leak. Same for `[finalizer]`. A + verdict of `RETAINED` requires at least one *durable* retainer. +- **It matches the type, not the type's spelling.** `--type GTDGoody` must not match + `System.Func<…GTDGoody…>` — a cached lambda whose generic *argument* mentions it. + It did, during development, and confidently reported a 2-hop path to the wrong + object. +- **Attaching suspends the target.** On a multi-GB heap the mark pass is minutes, + not seconds — take a dump. + +### Exit codes, and why exit 2 exists + +| Exit | Meaning | +| --- | --- | +| 0 | The heap was read. `ABSENT` / `OBSERVED_ONLY` — nothing durably retains the type. | +| 1 | The heap was read. `RETAINED` — a durable path exists, and it is printed. | +| 2 | **The heap was not read.** Usage error, unreadable target, refused attach. | + +*Not looking* and *looking and finding nothing* are different outcomes, and +collapsing them is how a monitoring pipeline learns to report health it never +measured. Permissions, the Yama `ptrace_scope` cases and the CI rules are in +[`docs/runtime-witness-operations.md`](../../docs/runtime-witness-operations.md). + +Output is the `runtime.json` contract (`OwnAudit/docs/runtime-contract.md`), so +`OwnAudit/runtime/correlate.py` consumes it with no adapter — giving the three-way +split its missing input: **confirmed** (a static leak finding whose type also shows +up retained), **static-only** (probable FP), and **runtime-only** — retention with +nothing static to explain it, which is a rule request rather than a report. + ## Selftest `ingest.py` carries embedded-fixture selftests (no harness, no Windows needed) and @@ -138,8 +241,12 @@ python audit/runtime/ingest.py --selftest the duplicate detector and the PropertyChanged-storm profiler), the leak-harness scenario schema + one scenario, runtime rule mappings in the taxonomy (categories 2/3/4/6/11), the C# leak-harness skeleton, the C# duplicate-immutable detector - (strings), and the C# PropertyChanged-storm profiler (ETW). -- **Deferred:** duplicate detection for arbitrary immutable types (field-by-field + (strings), the C# PropertyChanged-storm profiler (ETW), and the **retention + witness** (`RetentionPath/`: census + ranked root paths, `runtime.json`, exit-code + tiers, CI-gated build and demo). +- **Deferred:** the **dominator tree / retained sizes** for the retention witness + (Own.NET#334 — the A3 extraction left it out on purpose; the sampled histogram is + what ships), duplicate detection for arbitrary immutable types (field-by-field content equality), the diagnostic-build INPC `EventSource` instrumentation in the target + PerfView/SematixTrace capture wiring, and a scenario corpus for the top-N screens. diff --git a/audit/runtime/RetentionPath/Heap.cs b/audit/runtime/RetentionPath/Heap.cs index 67f71c12..38b6a13b 100644 --- a/audit/runtime/RetentionPath/Heap.cs +++ b/audit/runtime/RetentionPath/Heap.cs @@ -31,7 +31,10 @@ namespace OwnNet.Audit.Runtime /// is how a leak hunt goes wrong. /// /// The principled version of this is a dominator tree (which single reference, if cut, - /// frees the object — and how much memory that frees). See the README. + /// frees the object — and how much memory that frees). It is NOT implemented here: the + /// A3 witness was extracted from PR #280 without it, deliberately. See + /// `audit/runtime/README.md` § "Retention paths" for what the sampled histogram does and + /// does not answer, and Own.NET#334 for what the dominator implementation argued. /// internal sealed class RetentionWalker : IDisposable { @@ -347,11 +350,6 @@ internal static bool IsType(string heapType, string wanted) return string.Equals(simple, wanted, StringComparison.Ordinal); } - /// - /// The dominator tree of the whole live graph, with retained sizes. This is the well-posed - /// version of "who holds it": not a path, but the one reference whose removal frees the object. - /// - public void Dispose() { _runtime.Dispose();