From 4f0b068f5d1fa18813211f6c1720ac7031be53fd Mon Sep 17 00:00:00 2001 From: PhysShell Date: Tue, 14 Jul 2026 23:15:04 +0500 Subject: [PATCH 1/4] corpus(wpf): a `-=` that exists is not a `-=` that runs (#278) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A regression guard for the release-reachability soundness gap: the subscription is to a STATIC publisher, a matching `-=` does exist in the class — so the extractor's "any matching `-=` in the class releases it" model falls silent — but it never runs. It sits behind a bool parameter every caller passes as `true`, in a method that is not a teardown, and that a whole subsystem never calls at all. before.cs is RED today: it is a real leak and own-check says nothing. That is the point of the case. after.cs must stay silent (the release is unconditional and in Dispose), so the fix cannot trade the false negative for a false positive. The .own reduction shows the core already handles this correctly — it reports OWN001 "not released before return (leaks on at least one path)" and points at the guarded branch. The bug is upstream, in the extractor's release-matching. Reduced from SectorTS GTD.cs:5192/:5259 and proven on the heap with a ClrMD root walk: 66.3% of the heap still reachable from the GC roots after 31 documents, held via [PinnedHandle] -> static KernelProperty -> GBProperty -> PropertyChangedEventHandler -> GTD. Detaching after each document makes the process memory-flat (peak RSS 2.71 GB -> 0.61 GB, byte-identical output), which confirms the diagnosis. Prior art: corpus/wpf/subscription-explicit-delegate-release/notes.md:28-41 already records the model as not flow-sensitive, but scoped it to a rebinding setter and deferred it. This case shows the surface is wider and gives it a real instance. CI: recall floor is unaffected (--min-recall is an absolute floor of caught cases, and this one is not caught yet); after.cs adds no false positive. The cfg/diag parity fixtures are regenerated because the corpus grew; tests/run_tests.py and cargo test are both green. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PjQnE1FDucd6vBVQbFswiE --- corpus/wpf/unsubscribe-behind-a-flag/after.cs | 60 +++++++++++++++ .../wpf/unsubscribe-behind-a-flag/before.cs | 62 +++++++++++++++ corpus/wpf/unsubscribe-behind-a-flag/case.own | 26 +++++++ .../expected-diagnostics.txt | 1 + corpus/wpf/unsubscribe-behind-a-flag/notes.md | 77 +++++++++++++++++++ tests/fixtures/cfg_parity.json | 6 ++ tests/fixtures/diag_parity.json | 10 +++ 7 files changed, 242 insertions(+) create mode 100644 corpus/wpf/unsubscribe-behind-a-flag/after.cs create mode 100644 corpus/wpf/unsubscribe-behind-a-flag/before.cs create mode 100644 corpus/wpf/unsubscribe-behind-a-flag/case.own create mode 100644 corpus/wpf/unsubscribe-behind-a-flag/expected-diagnostics.txt create mode 100644 corpus/wpf/unsubscribe-behind-a-flag/notes.md diff --git a/corpus/wpf/unsubscribe-behind-a-flag/after.cs b/corpus/wpf/unsubscribe-behind-a-flag/after.cs new file mode 100644 index 00000000..8266bb65 --- /dev/null +++ b/corpus/wpf/unsubscribe-behind-a-flag/after.cs @@ -0,0 +1,60 @@ +// FIXED. The release is unconditional and in a teardown. +// +// Two things changed, and both matter: +// +// 1. the `-=` moved into `Dispose()` — a teardown context, which is what P-001/P-004 require; +// 2. the flag no longer guards it. `UnregisterChildren` still exists for the callers that only +// wanted the child rows detached, but it can no longer be mistaken for a full teardown, and +// it cannot silently skip the static detach. +// +// own-check MUST treat this as released (silent). The point of the pair is that `before.cs` and +// `after.cs` differ ONLY in whether the release is provably reached — the `+=` and the `-=` name +// the same (receiver, handler) pair in both files. A model that keys on the mere existence of a +// matching `-=` cannot tell these two apart, which is exactly the soundness gap this case pins. +using System; +using System.ComponentModel; + +public static class AppSettings +{ + public static readonly NotifyingOptions Options = new NotifyingOptions(); +} + +public class NotifyingOptions : INotifyPropertyChanged +{ + public event PropertyChangedEventHandler PropertyChanged; +} + +public sealed class Document : IDisposable +{ + public Document() + { + AppSettings.Options.PropertyChanged += new PropertyChangedEventHandler(OnOptionsChanged); + } + + public void Dispose() + { + // Unconditional, in a teardown. This is the one the subscription is paired with. + AppSettings.Options.PropertyChanged -= OnOptionsChanged; + UnregisterChildren(); + } + + // Narrowed and honestly named: it detaches the child rows, and nothing else. It can no longer + // be handed a flag that quietly turns it into a no-op for the static subscription. + public void UnregisterChildren() + { + // ... detach the child rows only ... + } + + private void OnOptionsChanged(object sender, PropertyChangedEventArgs e) { /* ... */ } +} + +public sealed class ImportService +{ + public void Import() + { + using (var doc = new Document()) + { + // ... map / import ... + } // Dispose() detaches it from the static publisher + } +} diff --git a/corpus/wpf/unsubscribe-behind-a-flag/before.cs b/corpus/wpf/unsubscribe-behind-a-flag/before.cs new file mode 100644 index 00000000..93aabe2f --- /dev/null +++ b/corpus/wpf/unsubscribe-behind-a-flag/before.cs @@ -0,0 +1,62 @@ +// BUGGY. A `-=` that exists is not a `-=` that runs. +// +// The document subscribes to a STATIC event in its constructor. A matching `-=` does exist — +// so own-check's release model ("any matching `-=` in the class releases it") falls silent — +// but it is unreachable in practice on two independent counts: +// +// 1. it lives in `UnregisterEventHandlers`, which is not a teardown (`Dispose`/`OnClosed`/ +// `Unloaded`); P-001 and P-004 both specify the release must be *in* a teardown, and the +// extractor is looser than its own spec; +// 2. even when that method IS called, the `-=` sits behind `if (!unregOnlyChildren)`, and the +// calling code passes `true`. +// +// The publisher is static, so the handler pins the whole document graph for the life of the +// process. Reduced from SectorTS `BrokerDataClasses/GTD.cs:5192` (subscribe) / `:5259` +// (the flag-guarded release); heap-proven — 66% of the heap still reachable from the GC roots +// after 31 documents, retention path +// [PinnedHandle] -> static KernelProperty -> GBProperty -> PropertyChangedEventHandler -> GTD. +using System.ComponentModel; + +// The app-lifetime settings object. Static => lives for the whole process. +public static class AppSettings +{ + public static readonly NotifyingOptions Options = new NotifyingOptions(); +} + +public class NotifyingOptions : INotifyPropertyChanged +{ + public event PropertyChangedEventHandler PropertyChanged; +} + +public sealed class Document +{ + public Document() + { + // Subscribed to a STATIC publisher. Nothing detaches this unless somebody calls + // UnregisterEventHandlers(false) — and nobody does. -> OWN001 + AppSettings.Options.PropertyChanged += new PropertyChangedEventHandler(OnOptionsChanged); + } + + // NOT a teardown, and the release is guarded away by the parameter every caller passes. + public void UnregisterEventHandlers(bool unregOnlyChildren = false) + { + if (!unregOnlyChildren) + { + AppSettings.Options.PropertyChanged -= OnOptionsChanged; // the `-=` that never runs + } + + // ... detach the child rows only ... + } + + private void OnOptionsChanged(object sender, PropertyChangedEventArgs e) { /* ... */ } +} + +public sealed class ImportService +{ + public Document Import() + { + var doc = new Document(); + doc.UnregisterEventHandlers(true); // true => the static `-=` above is skipped + return doc; + } +} diff --git a/corpus/wpf/unsubscribe-behind-a-flag/case.own b/corpus/wpf/unsubscribe-behind-a-flag/case.own new file mode 100644 index 00000000..32e5d30c --- /dev/null +++ b/corpus/wpf/unsubscribe-behind-a-flag/case.own @@ -0,0 +1,26 @@ +// OwnLang model of SectorTS `BrokerDataClasses/GTD.cs` — a document that subscribes to a STATIC +// publisher in its constructor (`:5192`) and whose `-=` lives behind a parameter, in a method that +// is not a teardown (`UnregisterEventHandlers(bool UnregOnlyGoodys)`, `:5259`). Callers pass `true`; +// one whole subsystem (DocCloud) never calls it at all. +// +// `acquire` == `AppSettings.Options.PropertyChanged += h`, `release` == the matching `-=`. +// The flag is modelled as the branch that returns before the release — the path that skips cleanup — +// so the core flags the un-released subscription as OWN001. +// +// NOTE what this reduction proves: **the core already gets this right.** Give it a release that is +// not reached on every path and it says so. The bug is upstream, in the extractor's release-matching, +// which emits `released: true` from the mere *existence* of a matching `-=` in the class +// (`OwnSharp.Extractor/Program.cs:13`) and never asks whether it runs. See notes.md and #278. +module Corpus +resource Subscription { + acquire Subscribe + release Dispose + kind "subscription token" +} +fn Document(options: int, unregOnlyChildren: int) { + let sub = acquire Subscription(options); // ctor: `+=` on the process-wide static publisher + if (unregOnlyChildren) { + return; // caller passed `true` -> the static `-=` is skipped + } + release sub; // the `-=` only a caller passing `false` ever reaches +} diff --git a/corpus/wpf/unsubscribe-behind-a-flag/expected-diagnostics.txt b/corpus/wpf/unsubscribe-behind-a-flag/expected-diagnostics.txt new file mode 100644 index 00000000..ed2a1929 --- /dev/null +++ b/corpus/wpf/unsubscribe-behind-a-flag/expected-diagnostics.txt @@ -0,0 +1 @@ +OWN001 diff --git a/corpus/wpf/unsubscribe-behind-a-flag/notes.md b/corpus/wpf/unsubscribe-behind-a-flag/notes.md new file mode 100644 index 00000000..e7d88d00 --- /dev/null +++ b/corpus/wpf/unsubscribe-behind-a-flag/notes.md @@ -0,0 +1,77 @@ +# Unsubscribe behind a flag, in a method nobody calls (release-reachability FN) + +> **This case is RED today.** `before.cs` is a real, heap-proven leak and own-check is **silent** on it. +> It is a regression guard for #278, not a passing test. It goes green when release-matching stops +> concluding "released" from the mere *existence* of a `-=`. + +**Pattern.** A document subscribes to a **static** publisher in its constructor. A matching `-=` does +exist in the class — so the current model pairs them and says nothing — but it never runs: + +```csharp +public Document() +{ + AppSettings.Options.PropertyChanged += new PropertyChangedEventHandler(OnOptionsChanged); // static publisher +} + +public void UnregisterEventHandlers(bool unregOnlyChildren = false) // NOT a teardown +{ + if (!unregOnlyChildren) // callers pass true + AppSettings.Options.PropertyChanged -= OnOptionsChanged; // the `-=` that never runs +} +``` + +Three independent reasons the release is unreachable, any one of which is enough: + +1. `UnregisterEventHandlers` is **not a teardown** — not `Dispose`, `OnClosed` or `Unloaded`. + `docs/proposals/P-001-csharp-extractor.md:51` and `P-004-wpf-lifetime-profile.md:33` both specify the + release must be *in* one of those. The extractor is looser than its own spec + (`OwnSharp.Extractor/Program.cs:13`: *"released by a matching `-=` **in the class**"*). +2. The `-=` sits behind a **parameter guard**, and the calling code passes `true`. +3. Whole subsystems **never call the method at all**. + +**Why it matters.** The publisher is static, so the handler pins the subscriber for the life of the +process — the strongest leak tier P-004 defines, and the one the analyzer is supposed to call a *provable* +leak rather than a possible one. + +**Provenance.** Reduced from SectorTS `BrokerDataClasses/GTD.cs:5192` (subscribe to the static +`AppData.Properties.GBProperty`) and `:5259` (`UnregisterEventHandlers(bool UnregOnlyGoodys)`). +`Service/GTDService.cs` passes `true` at five sites; `BrokerDataClasses/DocCloud/**` — including eight +AutoMapper `.ConstructUsing(x => new GTD(null, null))` profiles that build a document per mapping — +never calls it at all. + +Proven at runtime with a ClrMD root walk, after **31 documents**: + +``` +on the heap : 1 685 951 objects 223 MB +REACHABLE from roots : 1 569 072 objects 148 MB +>>> 66.3% of the heap is genuinely RETAINED + +[PinnedHandle] System.Object[] + KernelProperty <- AppData.Properties (static) + GBProperty + PropertyChangedEventHandler + System.Object[] <- the delegate's invocation list + PropertyChangedEventHandler + GTD <- the whole document graph +``` + +Detaching after each document (`UnregisterEventHandlers(false)`) makes the process memory-flat — +peak RSS 2.71 GB → 0.61 GB on the same 389 documents, **byte-identical output** — which confirms the +diagnosis rather than merely being consistent with it. + +**Relation to the known gap.** `corpus/wpf/subscription-explicit-delegate-release/notes.md:28-41` +already records that the release model is not flow-sensitive ("*it treats any matching `-=` in the class +as releasing the subscription … that soundness gap is pre-existing*", Codex P2 on #163). That note scoped +the gap to a **rebinding setter** and deferred it. This case shows the surface is much wider — a +parameter guard, a non-teardown method, and an uncalled method are all ordinary code — and gives the gap +its first real, heap-proven instance. + +**Regression guard.** `scripts/benchmark.py` runs the real C# through the extractor + core: + +* `before.cs` must be **caught** (OWN001) — it is a genuine leak. *Currently it is not: this is the bug.* +* `after.cs` must be **silent** — the release is unconditional and in `Dispose`. It must stay silent + after the fix, or the fix has simply traded a false negative for a false positive. + +The pair differs **only** in whether the release is provably reached; the `+=` and the `-=` name the same +`(receiver, handler)` in both. That is deliberate: a model that keys on the existence of a matching `-=` +cannot tell these two files apart. diff --git a/tests/fixtures/cfg_parity.json b/tests/fixtures/cfg_parity.json index 2b07d228..0d322c6f 100644 --- a/tests/fixtures/cfg_parity.json +++ b/tests/fixtures/cfg_parity.json @@ -241,6 +241,12 @@ "cfg": "{\n \"functions\": [\n {\n \"blocks\": [\n {\n \"id\": 0,\n \"instrs\": [],\n \"label\": \"entry\",\n \"succ\": []\n }\n ],\n \"entry\": 0,\n \"has_return_type\": false,\n \"name\": \"GraphicsConfigurationDialog\",\n \"params\": [\n 0\n ],\n \"symbols\": [\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 20,\n \"is_param_borrow\": false,\n \"kind\": \"plain\",\n \"name\": \"systemEvents\",\n \"origin\": \"systemEvents#20\",\n \"resource_kind\": null,\n \"type_name\": \"SystemEvents\"\n }\n ]\n }\n ],\n \"ownlang_cfg_version\": 0\n}", "diags": [] }, + { + "name": "corpus/wpf/unsubscribe-behind-a-flag/case.own", + "source": "// OwnLang model of SectorTS `BrokerDataClasses/GTD.cs` — a document that subscribes to a STATIC\n// publisher in its constructor (`:5192`) and whose `-=` lives behind a parameter, in a method that\n// is not a teardown (`UnregisterEventHandlers(bool UnregOnlyGoodys)`, `:5259`). Callers pass `true`;\n// one whole subsystem (DocCloud) never calls it at all.\n//\n// `acquire` == `AppSettings.Options.PropertyChanged += h`, `release` == the matching `-=`.\n// The flag is modelled as the branch that returns before the release — the path that skips cleanup —\n// so the core flags the un-released subscription as OWN001.\n//\n// NOTE what this reduction proves: **the core already gets this right.** Give it a release that is\n// not reached on every path and it says so. The bug is upstream, in the extractor's release-matching,\n// which emits `released: true` from the mere *existence* of a matching `-=` in the class\n// (`OwnSharp.Extractor/Program.cs:13`) and never asks whether it runs. See notes.md and #278.\nmodule Corpus\nresource Subscription {\n acquire Subscribe\n release Dispose\n kind \"subscription token\"\n}\nfn Document(options: int, unregOnlyChildren: int) {\n let sub = acquire Subscription(options); // ctor: `+=` on the process-wide static publisher\n if (unregOnlyChildren) {\n return; // caller passed `true` -> the static `-=` is skipped\n }\n release sub; // the `-=` only a caller passing `false` ever reaches\n}\n", + "cfg": "{\n \"functions\": [\n {\n \"blocks\": [\n {\n \"id\": 0,\n \"instrs\": [\n {\n \"line\": 21,\n \"op\": \"acquire\",\n \"resource\": \"Subscription\",\n \"sym\": 2\n }\n ],\n \"label\": \"entry\",\n \"succ\": [\n 1,\n 2\n ]\n },\n {\n \"id\": 1,\n \"instrs\": [\n {\n \"line\": 23,\n \"op\": \"return\",\n \"sym\": null\n }\n ],\n \"label\": \"then\",\n \"succ\": []\n },\n {\n \"id\": 2,\n \"instrs\": [],\n \"label\": \"else\",\n \"succ\": [\n 3\n ]\n },\n {\n \"id\": 3,\n \"instrs\": [\n {\n \"line\": 25,\n \"op\": \"release\",\n \"sym\": 2\n }\n ],\n \"label\": \"merge\",\n \"succ\": []\n }\n ],\n \"entry\": 0,\n \"has_return_type\": false,\n \"name\": \"Document\",\n \"params\": [\n 0,\n 1\n ],\n \"symbols\": [\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 20,\n \"is_param_borrow\": false,\n \"kind\": \"plain\",\n \"name\": \"options\",\n \"origin\": \"options#20\",\n \"resource_kind\": null,\n \"type_name\": \"int\"\n },\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 20,\n \"is_param_borrow\": false,\n \"kind\": \"plain\",\n \"name\": \"unregOnlyChildren\",\n \"origin\": \"unregOnlyChildren#20\",\n \"resource_kind\": null,\n \"type_name\": \"int\"\n },\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 21,\n \"is_param_borrow\": false,\n \"kind\": \"owned\",\n \"name\": \"sub\",\n \"origin\": \"sub#21\",\n \"resource_kind\": \"subscription token\",\n \"type_name\": \"Subscription\"\n }\n ]\n }\n ],\n \"ownlang_cfg_version\": 0\n}", + "diags": [] + }, { "name": "corpus/wpf/viewmodel-escapes-to-app/case.own", "source": "module WpfRegionEscape\n\n// Lifetime regions: a Window-lived ViewModel must not outlive its window, and\n// the App-lived event bus outlives everything.\nlifetime App;\nlifetime Window < App;\nlifetime ViewModel < Window;\n\n// The ViewModel (ViewModel-lived) strongly subscribes itself to the App-lived\n// bus. Because App strictly outlives ViewModel, the subscription promotes the\n// VM to App lifetime -> it can never die while the app runs => OWN014. This is\n// the region-escape theorem: the *ordering* is what makes it a leak (subscribing\n// to a same/shorter-lived source would be fine).\nfn CustomerViewModel(bus: EventBus lifetime App) lifetime ViewModel {\n subscribe self to bus;\n}\n", diff --git a/tests/fixtures/diag_parity.json b/tests/fixtures/diag_parity.json index 01fc954c..faf40c9d 100644 --- a/tests/fixtures/diag_parity.json +++ b/tests/fixtures/diag_parity.json @@ -405,6 +405,16 @@ ] ] }, + { + "name": "corpus/wpf/unsubscribe-behind-a-flag/case.own", + "source": "// OwnLang model of SectorTS `BrokerDataClasses/GTD.cs` — a document that subscribes to a STATIC\n// publisher in its constructor (`:5192`) and whose `-=` lives behind a parameter, in a method that\n// is not a teardown (`UnregisterEventHandlers(bool UnregOnlyGoodys)`, `:5259`). Callers pass `true`;\n// one whole subsystem (DocCloud) never calls it at all.\n//\n// `acquire` == `AppSettings.Options.PropertyChanged += h`, `release` == the matching `-=`.\n// The flag is modelled as the branch that returns before the release — the path that skips cleanup —\n// so the core flags the un-released subscription as OWN001.\n//\n// NOTE what this reduction proves: **the core already gets this right.** Give it a release that is\n// not reached on every path and it says so. The bug is upstream, in the extractor's release-matching,\n// which emits `released: true` from the mere *existence* of a matching `-=` in the class\n// (`OwnSharp.Extractor/Program.cs:13`) and never asks whether it runs. See notes.md and #278.\nmodule Corpus\nresource Subscription {\n acquire Subscribe\n release Dispose\n kind \"subscription token\"\n}\nfn Document(options: int, unregOnlyChildren: int) {\n let sub = acquire Subscription(options); // ctor: `+=` on the process-wide static publisher\n if (unregOnlyChildren) {\n return; // caller passed `true` -> the static `-=` is skipped\n }\n release sub; // the `-=` only a caller passing `false` ever reaches\n}\n", + "diags": [ + [ + 23, + "OWN001" + ] + ] + }, { "name": "corpus/wpf/viewmodel-escapes-to-app/case.own", "source": "module WpfRegionEscape\n\n// Lifetime regions: a Window-lived ViewModel must not outlive its window, and\n// the App-lived event bus outlives everything.\nlifetime App;\nlifetime Window < App;\nlifetime ViewModel < Window;\n\n// The ViewModel (ViewModel-lived) strongly subscribes itself to the App-lived\n// bus. Because App strictly outlives ViewModel, the subscription promotes the\n// VM to App lifetime -> it can never die while the app runs => OWN014. This is\n// the region-escape theorem: the *ordering* is what makes it a leak (subscribing\n// to a same/shorter-lived source would be fine).\nfn CustomerViewModel(bus: EventBus lifetime App) lifetime ViewModel {\n subscribe self to bus;\n}\n", From 9f030ba5cd25c2179133313865a34a40ce5e6585 Mon Sep 17 00:00:00 2001 From: PhysShell Date: Tue, 14 Jul 2026 23:49:03 +0500 Subject: [PATCH 2/4] =?UTF-8?q?audit(runtime):=20retention=20paths=20?= =?UTF-8?q?=E2=80=94=20the=20half=20of=20the=20runtime=20arm=20HeapCounter?= =?UTF-8?q?=20leaves=20undone?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The README table has promised "Heap analysis (retained, duplicates, retention paths) — ClrMD" since Plan.md §4, and Plan.md:300 says retention paths are "considered ourselves via ClrMD". No tool did. OwnAudit/src/OwnAudit.Runtime/RuntimeArm.cs is an explicit placeholder for exactly this — it already declares `RetentionProof(TypeName, RetainedDelta, GcRootPath, Leaks)` and a TODO to "gcroot the survivors" — and points here: "the runtime arm is CANONICAL in Own.NET/audit/runtime". RetentionPath answers the two questions that decide a leak hunt, and that HeapCounter cannot: census — is any of it RETAINED, or is the heap just full of uncollected garbage? roots — if it is retained, WHO holds it? The first is not pedantry. ClrHeap.EnumerateObjects() walks the heap segments linearly and returns everything allocated, INCLUDING garbage the GC has not collected. A big heap is not evidence of a leak. HeapCounter mitigates that by forcing a GC in the target first; marking from the roots answers it directly and needs no cooperation from the target. If the retained share is low there is no reference to hunt and the next question is about GC timing, not about who holds what. Every hop of a root path names the field it traverses (ClrMD's EnumerateReferencesWithFields), which is what turns "this object is alive" into "this field is holding it" — the sentence a developer can act on. Attaches to a LIVE process (no procdump — which HeapCounter needs and which is not always installed); a dump still works via --dump. Output is the runtime.json contract (OwnAudit/docs/runtime-contract.md), so OwnAudit's runtime/correlate.py consumes it with no adapter — closing the loop its PR #13 designed: a `runtime-only` finding (retention with nothing static to explain it) is the analyzer's blind spot, i.e. a rule request. #278 is the first one. Validated end-to-end against a live net472 x64 target: 70.4% of a 573 MB heap retained, and the root walk names the holding fields. Limits are documented in the README rather than glossed: the path reported is the SHORTEST one and may not be the most explanatory; a [stack] root is not retention; and type matching compares the type, not its spelling (asking for GTDGoody must not match System.Func<...GTDGoody, Boolean> — it did, and confidently reported a path to the wrong object). Not CI-gated, like its three neighbours — CI runs only audit/runtime/ingest.py (--selftest 26/26, still green; tests/run_tests.py exit 0). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PjQnE1FDucd6vBVQbFswiE --- audit/runtime/README.md | 60 ++++ audit/runtime/RetentionPath/Heap.cs | 303 ++++++++++++++++++ audit/runtime/RetentionPath/Program.cs | 199 ++++++++++++ .../RetentionPath/RetentionPath.csproj | 33 ++ 4 files changed, 595 insertions(+) create mode 100644 audit/runtime/RetentionPath/Heap.cs create mode 100644 audit/runtime/RetentionPath/Program.cs create mode 100644 audit/runtime/RetentionPath/RetentionPath.csproj diff --git a/audit/runtime/README.md b/audit/runtime/README.md index cb4ebb9f..59c3fe6b 100644 --- a/audit/runtime/README.md +++ b/audit/runtime/README.md @@ -44,8 +44,68 @@ 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 paths — Windows/build-required, NOT CI-gated + RetentionPath.csproj # net472; Microsoft.Diagnostics.Runtime + Heap.cs # ClrMD: mark from the GC roots; root -> object path with field names + Program.cs # `census` (is it retained at all?) and `roots` (who holds it?) ``` +## Retention paths — is it retained, and by whom (Plan.md §4) + +`HeapCounter` answers *"how many instances of T are on the heap"*. That is **not** the same question as +*"how many are retained"*, and conflating them is how a leak hunt goes wrong: +`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 this by forcing a GC in the target first (SematixTrace), which works when you can drive the +target; `RetentionPath` does not need to, because marking from the roots answers it directly. + +```powershell +# 1. Is there anything to hunt? (attaches to a LIVE process — no procdump needed) +RetentionPath.exe 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` +``` + +If that share is low, stop: there is no reference to hunt, and the next question is about GC timing, not +about who holds what. + +```powershell +# 2. Who holds it? Every hop names the field it traverses. +RetentionPath.exe roots --pid 1234 --type GTDGoody + +=== BrokerDataClasses.GTDGoody — retained via [static-field], 4 hops === + [PinnedHandle] System.Object[] + BrokerDataClasses.GTD + BrokerDataClasses.CalcProcentGTD (.k__BackingField) + BrokerDataClasses.GTDGoody (.fMainObject) +``` + +A dump works too (`--dump target.dmp`) and is the right choice when the target must not be paused. +Output is the **`runtime.json` contract** (`OwnAudit/docs/runtime-contract.md`), so OwnAudit's +`runtime/correlate.py` consumes it with no adapter: a static leak finding whose type also shows up here +as retained is `confirmed`; retention with **nothing static to explain it** is `runtime-only` — the +analyzer's blind spot, and therefore a rule request. + +### What it does not do (read this before trusting it) + +* **The path it reports is the SHORTEST one, not necessarily the most explanatory one.** An object with + many retainers has many paths; breadth-first returns the nearest root. That may be a prototype, a + cache, or the stack — while the *interesting* retainer is eight hops away through a delegate's + invocation list. Pass `--max-paths 3` and read them all. "Which retainer holds *most* of the N + instances" is the obviously-next feature and is not built. +* **A `[stack]` root is not retention.** It means the object is live in a frame *right now*. The tool + labels it as such precisely so it is not mistaken for a leak; the same is true of `[finalizer]`. +* **It matches the TYPE, not the type's spelling.** Asking for `GTDGoody` will not match + `System.Func` — a cached lambda whose generic *argument* + mentions it. (It used to, and confidently reported a 2-hop path to the wrong object. A tool that + points at the wrong culprit is worse than no tool.) +* Attaching **suspends** the target for the duration of the walk. On a multi-GB heap that is minutes, + not seconds — take a dump instead. + ## How the leak-harness works (Plan.md §4.1) Deterministic loop, run on the local Windows machine against the target: diff --git a/audit/runtime/RetentionPath/Heap.cs b/audit/runtime/RetentionPath/Heap.cs new file mode 100644 index 00000000..8a26bf1e --- /dev/null +++ b/audit/runtime/RetentionPath/Heap.cs @@ -0,0 +1,303 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Microsoft.Diagnostics.Runtime; + +namespace OwnNet.Audit.Runtime +{ + /// + /// Mark-from-roots over a target's managed heap, and the root -> object path for a + /// suspect type. + /// + /// WHY THIS IS NOT HeapCounter. answers "how many instances + /// of T are on the heap". That is a different question from "how many are RETAINED", + /// because 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 this by forcing a GC in + /// the target first, which works when you can (SematixTrace); this type does not need + /// to, because marking from the roots answers the question directly: + /// + /// reachable ≈ heap -> genuinely retained; something holds it + /// reachable << heap -> not a leak; the GC simply has not collected yet + /// + /// That distinction is the difference between a leak hunt and a wild goose chase, and + /// it is cheap: one mark pass. + /// + internal sealed class RetentionWalker : IDisposable + { + private readonly DataTarget _target; + private readonly ClrRuntime _runtime; + + /// Attach to a LIVE process (suspends it for the read). No procdump needed. + public static RetentionWalker AttachToProcess(int pid) => + new RetentionWalker(DataTarget.AttachToProcess(pid, suspend: true)); + + /// Read a full dump — the right choice when the target must not be paused. + public static RetentionWalker LoadDump(string path) => + new RetentionWalker(DataTarget.LoadDump(path)); + + private RetentionWalker(DataTarget target) + { + _target = target; + var clr = _target.ClrVersions.FirstOrDefault() + ?? throw new InvalidOperationException( + "the target contains no CLR — is it a managed process / a full (-ma) dump?"); + _runtime = clr.CreateRuntime(); + } + + private ClrHeap Heap => _runtime.Heap; + + /// + /// One mark pass. Returns the retained set (by type) alongside the raw heap totals, + /// so the caller can state the retained SHARE rather than a bare object count. + /// + public HeapCensus Census() + { + long heapObjects = 0, heapBytes = 0; + foreach (var o in Heap.EnumerateObjects()) + { + if (!o.IsValid || o.Type == null) continue; + heapObjects++; + heapBytes += (long)o.Size; + } + + var seen = new HashSet(); + var stack = new Stack(); + foreach (var root in Heap.EnumerateRoots()) + { + var o = root.Object; + if (o.IsValid && seen.Add(o.Address)) stack.Push(o.Address); + } + int rootCount = seen.Count; + + var byType = new Dictionary(); + long liveObjects = 0, liveBytes = 0; + while (stack.Count > 0) + { + var obj = Heap.GetObject(stack.Pop()); + if (!obj.IsValid || obj.Type == null) continue; + + liveObjects++; + long size = (long)obj.Size; + liveBytes += size; + + string name = obj.Type.Name ?? ""; + if (!byType.TryGetValue(name, out var tally)) tally = new TypeTally(); + tally.Count++; + tally.Bytes += size; + byType[name] = tally; + + foreach (var child in obj.EnumerateReferences()) + if (child.IsValid && seen.Add(child.Address)) stack.Push(child.Address); + } + + return new HeapCensus(rootCount, heapObjects, heapBytes, liveObjects, liveBytes, byType); + } + + /// + /// The GC-root path to instances whose type name contains . + /// Breadth-first from the roots, so the path returned is the SHORTEST one — the most + /// legible explanation of who is holding the object, not merely a valid one. + /// + /// The hops name the FIELD they traverse (ClrMD's EnumerateReferencesWithFields), which + /// is what turns "a byte[] is reachable" into "AppData.Properties.GBProperty.PropertyChanged + /// holds it" — the sentence a developer can act on. + /// + public IReadOnlyList FindRootPaths(string typeName, int maxPaths, int maxHops) + { + // Match the TYPE, not the type's spelling. A naive substring match on the type name + // matches `System.Func` when you asked for + // `GTDGoody` — a cached lambda whose *generic argument* happens to mention it — and + // then confidently reports a 2-hop path to the wrong object. A tool that points at the + // wrong culprit is worse than no tool. So compare the simple name with the generic + // arguments stripped. + var targets = new Dictionary(); + foreach (var o in Heap.EnumerateObjects()) + { + if (!o.IsValid || o.Type?.Name == null) continue; + if (!IsType(o.Type.Name, typeName)) continue; + targets[o.Address] = o.Type.Name; + if (targets.Count >= maxPaths) break; + } + if (targets.Count == 0) return Array.Empty(); + + var found = new List(); + var seen = new HashSet(); + var queue = new Queue(); + + foreach (var root in Heap.EnumerateRoots()) + { + var o = root.Object; + if (!o.IsValid || !seen.Add(o.Address)) continue; + queue.Enqueue(new Node(o.Address, null, Describe(root, o), root.RootKind)); + } + + while (queue.Count > 0 && found.Count < maxPaths) + { + var node = queue.Dequeue(); + + if (targets.TryGetValue(node.Address, out var hitType)) + { + found.Add(new RetentionPathResult(hitType, Unwind(node), node.RootKind)); + continue; + } + + var obj = Heap.GetObject(node.Address); + if (!obj.IsValid || obj.Type == null) continue; + if (Depth(node) > maxHops) continue; + + // WithFields so a hop reads "GBProperty.PropertyChanged", not just "GBProperty". + foreach (var reference in obj.EnumerateReferencesWithFields()) + { + var child = reference.Object; + if (!child.IsValid || !seen.Add(child.Address)) continue; + string hop = child.Type?.Name ?? "?"; + if (reference.Field?.Name is string f && f.Length > 0) hop = hop + " (." + f + ")"; + queue.Enqueue(new Node(child.Address, node, hop, node.RootKind)); + } + } + return found; + } + + /// + /// Does name the type the caller asked for? Compares the SIMPLE + /// name with generic arguments stripped, so `GTDGoody` matches `BrokerDataClasses.GTDGoody` + /// but NOT `System.Func<BrokerDataClasses.GTDGoody, System.Boolean>`. A fully-qualified + /// request (`BrokerDataClasses.GTDGoody`) is matched exactly. + /// + internal static bool IsType(string heapType, string wanted) + { + if (string.Equals(heapType, wanted, StringComparison.Ordinal)) return true; + + // strip generic arguments: `Func` -> `Func`, `BindingList` -> `BindingList` + int lt = heapType.IndexOf('<'); + string bare = lt >= 0 ? heapType.Substring(0, lt) : heapType; + + if (string.Equals(bare, wanted, StringComparison.Ordinal)) return true; + + // allow the simple name: `GTDGoody` matches `BrokerDataClasses.GTDGoody` + int dot = bare.LastIndexOf('.'); + string simple = dot >= 0 ? bare.Substring(dot + 1) : bare; + return string.Equals(simple, wanted, StringComparison.Ordinal); + } + + private static string Describe(ClrRoot root, ClrObject o) => + "[" + root.RootKind + "] " + (o.Type?.Name ?? "?"); + + private static int Depth(Node n) + { + int d = 0; + for (var p = n.Parent; p != null; p = p.Parent) d++; + return d; + } + + private static IReadOnlyList Unwind(Node n) + { + var path = new List(); + for (var p = n; p != null; p = p.Parent) path.Add(p.Label); + path.Reverse(); + return path; + } + + public void Dispose() + { + _runtime.Dispose(); + _target.Dispose(); + } + + private sealed class Node + { + public readonly ulong Address; + public readonly Node? Parent; + public readonly string Label; + public readonly ClrRootKind RootKind; + + public Node(ulong address, Node? parent, string label, ClrRootKind rootKind) + { + Address = address; + Parent = parent; + Label = label; + RootKind = rootKind; + } + } + } + + internal struct TypeTally + { + public long Count; + public long Bytes; + } + + internal sealed class HeapCensus + { + public readonly int Roots; + public readonly long HeapObjects; + public readonly long HeapBytes; + public readonly long RetainedObjects; + public readonly long RetainedBytes; + public readonly IReadOnlyDictionary ByType; + + public HeapCensus(int roots, long heapObjects, long heapBytes, + long retainedObjects, long retainedBytes, + IReadOnlyDictionary byType) + { + Roots = roots; + HeapObjects = heapObjects; + HeapBytes = heapBytes; + RetainedObjects = retainedObjects; + RetainedBytes = retainedBytes; + ByType = byType; + } + + /// The number that decides whether this is a leak hunt at all. + public double RetainedShare => HeapBytes == 0 ? 0 : 100.0 * RetainedBytes / HeapBytes; + } + + internal sealed class RetentionPathResult + { + public readonly string TypeName; + public readonly IReadOnlyList Path; + public readonly ClrRootKind RootKind; + + public RetentionPathResult(string typeName, IReadOnlyList path, ClrRootKind rootKind) + { + TypeName = typeName; + Path = path; + RootKind = rootKind; + } + + /// + /// Map a ClrMD root kind onto the `runtime.json` kinds (OwnAudit/docs/runtime-contract.md: + /// static-field, static-event, gc-handle, thread-local, timer). + /// + /// Note there is no `StaticVar` root kind: on .NET Framework a class's statics live in a + /// pinned `System.Object[]` handed to the runtime as a **PinnedHandle**, which is why a + /// static-field leak surfaces as `[PinnedHandle] System.Object[] -> …`. So PinnedHandle is + /// where statics show up, and a **delegate hop** anywhere further down the path is what + /// makes it a static *event* rather than a plain static field — the distinction + /// correlate.py's `high` tier keys on. + /// + /// `Stack` and `FinalizerQueue` are reported as themselves. They are not contract kinds + /// and deliberately so: an object rooted only by the stack is merely *live right now*, not + /// retained, and reading it as a leak is how a leak hunt goes wrong. + /// + public string ContractKind() + { + bool viaDelegate = Path.Any(p => + p.IndexOf("EventHandler", StringComparison.Ordinal) >= 0 || + p.IndexOf("MulticastDelegate", StringComparison.Ordinal) >= 0); + + switch (RootKind) + { + case ClrRootKind.Stack: + return "stack"; // live in a frame right now — not retention + case ClrRootKind.FinalizerQueue: + return "finalizer"; // awaiting finalization — a stall, not a reference leak + case ClrRootKind.PinnedHandle: + return viaDelegate ? "static-event" : "static-field"; + default: + return viaDelegate ? "static-event" : "gc-handle"; + } + } + } +} diff --git a/audit/runtime/RetentionPath/Program.cs b/audit/runtime/RetentionPath/Program.cs new file mode 100644 index 00000000..ce8de203 --- /dev/null +++ b/audit/runtime/RetentionPath/Program.cs @@ -0,0 +1,199 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using Newtonsoft.Json; + +namespace OwnNet.Audit.Runtime +{ + /// + /// Retention paths (Plan.md §4): the half of the runtime arm that HeapCounter leaves + /// undone. HeapCounter counts instances of named types; this answers the two questions + /// that actually decide a leak hunt: + /// + /// 1. is any of it RETAINED, or is the heap just full of uncollected garbage? + /// 2. if it is retained — WHO is holding it? + /// + /// Emits the `runtime.json` contract (OwnAudit/docs/runtime-contract.md) so + /// OwnAudit's runtime/correlate.py consumes the output directly: a `confirmed` finding + /// is a static leak finding whose type also shows up here as retained, and a + /// `runtime-only` finding — retention with nothing static to explain it — is the + /// analyzer's blind spot, i.e. a rule request. + /// + /// Usage: + /// RetentionPath census --pid N | --dump D [--out runtime.json] [--top 25] + /// RetentionPath roots --pid N | --dump D --type Substring [--max-paths 3] [--max-hops 40] + /// + /// `census` prints the retained SHARE first, on purpose: if only 5% of the heap is + /// reachable, there is no leak to hunt and the next step is a GC question, not a + /// reference question. + /// + internal static class Program + { + private static int Main(string[] args) + { + if (args.Length == 0) return Usage(); + string verb = args[0].ToLowerInvariant(); + + int pid = ArgInt(args, "--pid", 0); + string? dump = Arg(args, "--dump"); + if (pid == 0 && dump == null) + { + Console.Error.WriteLine("retention-path: need --pid or --dump "); + return 2; + } + + try + { + using var walker = dump != null + ? RetentionWalker.LoadDump(dump) + : RetentionWalker.AttachToProcess(pid); + + switch (verb) + { + case "census": return Census(walker, args); + case "roots": return Roots(walker, args); + default: return Usage(); + } + } + catch (Exception ex) + { + // A failed read must not read as "clean" — exit 2, distinct from + // 0 (analysed, nothing retained) and 1 (analysed, retention found). + Console.Error.WriteLine($"retention-path: {ex.GetType().Name}: {ex.Message}"); + return 2; + } + } + + private static int Census(RetentionWalker walker, string[] args) + { + var c = walker.Census(); + int top = ArgInt(args, "--top", 25); + + Console.WriteLine($"roots : {c.Roots,12:N0} objects"); + Console.WriteLine($"on the heap : {c.HeapObjects,12:N0} objects {Mb(c.HeapBytes),10:N0} MB"); + Console.WriteLine($"REACHABLE from roots : {c.RetainedObjects,12:N0} objects {Mb(c.RetainedBytes),10:N0} MB"); + Console.WriteLine($"uncollected garbage : {c.HeapObjects - c.RetainedObjects,12:N0} objects {Mb(c.HeapBytes - c.RetainedBytes),10:N0} MB"); + Console.WriteLine(); + Console.WriteLine(c.RetainedShare > 50 + ? $">>> {c.RetainedShare:N1}% of the heap is genuinely RETAINED — something holds it; run `roots`" + : $">>> only {c.RetainedShare:N1}% of the heap is retained — the rest is garbage the GC has not collected"); + Console.WriteLine(); + Console.WriteLine($"{"type",-62}{"count",14}{"MB",12}"); + foreach (var kv in c.ByType.OrderByDescending(k => k.Value.Bytes).Take(top)) + Console.WriteLine($"{Short(kv.Key),-62}{kv.Value.Count,14:N0}{Mb(kv.Value.Bytes),12:N1}"); + + string? outPath = Arg(args, "--out"); + if (outPath != null) + { + // The runtime.json contract. `expected` is left at 0 — the collector does not + // know the budget; the scenario/config does, and correlate.py applies it. + var retained = c.ByType + .OrderByDescending(k => k.Value.Bytes) + .Take(top) + .Select(kv => new Dictionary + { + ["type"] = kv.Key, + ["count"] = kv.Value.Count, + ["expected"] = 0, + ["bytes"] = kv.Value.Bytes, + ["roots"] = new object[0], + }) + .ToList(); + + var doc = new Dictionary + { + ["schema"] = "own-runtime/1", + ["retained"] = retained, + }; + File.WriteAllText(outPath, JsonConvert.SerializeObject(doc, Formatting.Indented)); + Console.WriteLine(); + Console.WriteLine($"runtime.json written to {outPath}"); + } + + return c.RetainedShare > 50 ? 1 : 0; + } + + private static int Roots(RetentionWalker walker, string[] args) + { + string? type = Arg(args, "--type"); + if (type == null) + { + Console.Error.WriteLine("retention-path roots: need --type "); + return 2; + } + int maxPaths = ArgInt(args, "--max-paths", 3); + int maxHops = ArgInt(args, "--max-hops", 40); + + var paths = walker.FindRootPaths(type, maxPaths, maxHops); + if (paths.Count == 0) + { + Console.WriteLine($"no live instance of *{type}* is reachable from a GC root"); + return 0; + } + + foreach (var p in paths) + { + Console.WriteLine(); + Console.WriteLine($"=== {p.TypeName} — retained via [{p.ContractKind()}], {p.Path.Count} hops ==="); + foreach (var hop in p.Path) Console.WriteLine(" " + hop); + } + + string? outPath = Arg(args, "--out"); + if (outPath != null) + { + var doc = new Dictionary + { + ["schema"] = "own-runtime/1", + ["retained"] = paths.Select(p => new Dictionary + { + ["type"] = p.TypeName, + ["count"] = 1, + ["expected"] = 0, + ["bytes"] = 0, + ["roots"] = new object[] + { + new Dictionary + { + ["kind"] = p.ContractKind(), + ["holder"] = p.Path.Count > 1 ? p.Path[1] : p.Path[0], + ["via"] = "reference", + ["path"] = p.Path, + }, + }, + }).ToList(), + }; + File.WriteAllText(outPath, JsonConvert.SerializeObject(doc, Formatting.Indented)); + Console.WriteLine(); + Console.WriteLine($"runtime.json written to {outPath}"); + } + + return 1; // retention found + } + + private static double Mb(long bytes) => bytes / 1024.0 / 1024.0; + + private static string Short(string t) => + t.Length <= 60 ? t : t.Substring(0, 28) + "…" + t.Substring(t.Length - 30); + + private static string? Arg(string[] args, string name) + { + int i = Array.IndexOf(args, name); + return i >= 0 && i + 1 < args.Length ? args[i + 1] : null; + } + + private static int ArgInt(string[] args, string name, int fallback) + { + var v = Arg(args, name); + return v != null && int.TryParse(v, out int n) ? n : fallback; + } + + private static int Usage() + { + Console.Error.WriteLine("usage:"); + Console.Error.WriteLine(" RetentionPath census --pid | --dump [--out runtime.json] [--top 25]"); + Console.Error.WriteLine(" RetentionPath roots --pid | --dump --type [--max-paths 3] [--max-hops 40] [--out runtime.json]"); + return 2; + } + } +} diff --git a/audit/runtime/RetentionPath/RetentionPath.csproj b/audit/runtime/RetentionPath/RetentionPath.csproj new file mode 100644 index 00000000..bd032a1a --- /dev/null +++ b/audit/runtime/RetentionPath/RetentionPath.csproj @@ -0,0 +1,33 @@ + + + + Exe + net472 + latest + enable + RetentionPath + OwnNet.Audit.Runtime + x64 + x64 + + + + + + From 25ba5951a5084434582d1adc2977395715989dce Mon Sep 17 00:00:00 2001 From: PhysShell Date: Wed, 15 Jul 2026 00:03:51 +0500 Subject: [PATCH 3/4] audit(runtime): rank retainers by sample, not by shortest path to one object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "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. Reporting one of them was the tool's real weakness — the first version happily pointed at a prototype held by the stack while the leak was eight hops away through a delegate's invocation list. `roots` now samples the retained instances (--sample, default 200), computes every one's shortest path in a SINGLE BFS (breadth-first from the whole root set gives each node its shortest path for free), and reports the paths as a RANKED HISTOGRAM. On the live target this finds the leak on its own: #1 25/50 (50.0%) — via [static-event], 7 hops System.Object[] BrokerDataClasses.Property.KernelProperty BrokerDataClasses.Property.GBProperty (.fGBProperty) System.ComponentModel.PropertyChangedEventHandler (.PropertyChanged) System.Object[] (._invocationList) System.ComponentModel.PropertyChangedEventHandler BrokerDataClasses.GTD (._target) #4 1/50 (2.0%) — via [stack] — noise, and labelled as such Parent pointers are stored as addresses, not labels; type and field names are resolved only for the sampled paths. Carrying a label per node would cost hundreds of MB on a 4M-object heap. The run also exposes the remaining limit, and the README now says so plainly: the histogram partitions by SHORTEST path, so an object held by two references at once is attributed to the nearer one. Here 50% of the GTDs land under the static event and 44% under a static List — most likely many are held by BOTH, and detaching the event alone would not free them. "Which single reference, if cut, frees this object" is a different, well-posed question with a standard answer this tool does not implement: a dominator tree with retained sizes (Lengauer-Tarjan / Cooper-Harvey-Kennedy — what Eclipse MAT and dotMemory are built on). That is the honest next step. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PjQnE1FDucd6vBVQbFswiE --- audit/runtime/README.md | 53 +++-- audit/runtime/RetentionPath/Heap.cs | 308 ++++++++++++++++--------- audit/runtime/RetentionPath/Program.cs | 88 +++++-- 3 files changed, 300 insertions(+), 149 deletions(-) diff --git a/audit/runtime/README.md b/audit/runtime/README.md index 59c3fe6b..234d41fd 100644 --- a/audit/runtime/README.md +++ b/audit/runtime/README.md @@ -74,16 +74,38 @@ If that share is low, stop: there is no reference to hunt, and the next question about who holds what. ```powershell -# 2. Who holds it? Every hop names the field it traverses. -RetentionPath.exe roots --pid 1234 --type GTDGoody - -=== BrokerDataClasses.GTDGoody — retained via [static-field], 4 hops === - [PinnedHandle] System.Object[] - BrokerDataClasses.GTD - BrokerDataClasses.CalcProcentGTD (.k__BackingField) - BrokerDataClasses.GTDGoody (.fMainObject) +# 2. What holds the TYPICAL instance? Sampled, ranked, every hop naming its field. +RetentionPath.exe roots --pid 1234 --type GTD --sample 200 + +BrokerDataClasses.GTD: 50 on the heap, 50 of a 200-instance sample retained + +RETAINERS, ranked — what holds the TYPICAL instance, not merely one of them: + +#1 25/50 (50,0%) — via [static-event], 7 hops + System.Object[] + BrokerDataClasses.Property.KernelProperty + BrokerDataClasses.Property.GBProperty (.fGBProperty) + System.ComponentModel.PropertyChangedEventHandler (.PropertyChanged) + System.Object[] (._invocationList) + System.ComponentModel.PropertyChangedEventHandler + BrokerDataClasses.GTD (._target) + +#4 1/50 (2,0%) — via [stack], 2 hops + SerializerSim.TInfo + BrokerDataClasses.GTD (.Proto) + +>>> 50,0% of the retained instances hang off ONE reference: + System.ComponentModel.PropertyChangedEventHandler._target [static-event] ``` +**Why it samples.** *"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, not an +explanation. The question worth asking is *"what holds the **typical** instance"*. So the walk samples +the retained instances, computes each one's shortest path in a single BFS (breadth-first from the whole +root set gives every node its shortest path for free), and reports the paths as a **ranked histogram**. +The retainer that accounts for 129,900 of 130,000 instances is the leak; the one hanging off the stack +is noise — and reading *that* one as "the answer" is exactly how a leak hunt goes wrong. + A dump works too (`--dump target.dmp`) and is the right choice when the target must not be paused. Output is the **`runtime.json` contract** (`OwnAudit/docs/runtime-contract.md`), so OwnAudit's `runtime/correlate.py` consumes it with no adapter: a static leak finding whose type also shows up here @@ -92,11 +114,16 @@ analyzer's blind spot, and therefore a rule request. ### What it does not do (read this before trusting it) -* **The path it reports is the SHORTEST one, not necessarily the most explanatory one.** An object with - many retainers has many paths; breadth-first returns the nearest root. That may be a prototype, a - cache, or the stack — while the *interesting* retainer is eight hops away through a delegate's - invocation list. Pass `--max-paths 3` and read them all. "Which retainer holds *most* of the N - instances" is the obviously-next feature and is not built. +* **It cannot tell you that cutting the top retainer would free the object.** This is the important + one. The histogram partitions instances by their *shortest* path, so when an object is held by two + references at once, it is attributed to whichever is nearer — and the run above shows exactly that: + 50 % of the GTDs come out under the static event and 44 % under a static `List`, which most + likely means many are held by **both**. Detach the event and they still will not collect. + The question *"which single reference, if cut, frees this object — and how much memory does that + free"* is well-posed, and it has a standard answer this tool does not implement: a **dominator tree** + with retained sizes (Lengauer–Tarjan, or the iterative Cooper–Harvey–Kennedy formulation; it is what + Eclipse MAT and dotMemory are built on). That is the honest next step, and it is a feature, not a + tweak. * **A `[stack]` root is not retention.** It means the object is live in a frame *right now*. The tool labels it as such precisely so it is not mistaken for a leak; the same is true of `[finalizer]`. * **It matches the TYPE, not the type's spelling.** Asking for `GTDGoody` will not match diff --git a/audit/runtime/RetentionPath/Heap.cs b/audit/runtime/RetentionPath/Heap.cs index 8a26bf1e..56b46256 100644 --- a/audit/runtime/RetentionPath/Heap.cs +++ b/audit/runtime/RetentionPath/Heap.cs @@ -1,12 +1,13 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Text; using Microsoft.Diagnostics.Runtime; namespace OwnNet.Audit.Runtime { /// - /// Mark-from-roots over a target's managed heap, and the root -> object path for a + /// Mark-from-roots over a target's managed heap, and the root -> object paths for a /// suspect type. /// /// WHY THIS IS NOT HeapCounter. answers "how many instances @@ -14,14 +15,23 @@ namespace OwnNet.Audit.Runtime /// because 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 this by forcing a GC in - /// the target first, which works when you can (SematixTrace); this type does not need - /// to, because marking from the roots answers the question directly: + /// the target first (SematixTrace), which works when you can drive the target; this + /// type does not need to, because marking from the roots answers the question: /// /// reachable ≈ heap -> genuinely retained; something holds it /// reachable << heap -> not a leak; the GC simply has not collected yet /// - /// That distinction is the difference between a leak hunt and a wild goose chase, and - /// it is cheap: one mark pass. + /// WHY IT SAMPLES. "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 one is + /// an arbitrary pick, not an explanation. Ask instead: **what holds the typical + /// instance?** So the walk takes a SAMPLE of the retained instances, computes each + /// one's shortest path in a single BFS, and reports the paths as a HISTOGRAM. The + /// retainer that accounts for 129,900 of 130,000 instances is the leak; the three that + /// hang off the stack or a prototype are noise, and reading one of them as "the answer" + /// 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. /// internal sealed class RetentionWalker : IDisposable { @@ -95,68 +105,133 @@ public HeapCensus Census() } /// - /// The GC-root path to instances whose type name contains . - /// Breadth-first from the roots, so the path returned is the SHORTEST one — the most - /// legible explanation of who is holding the object, not merely a valid one. + /// Sample up to retained instances of , + /// compute every one's shortest root path in a SINGLE breadth-first pass (BFS from the whole + /// root set gives each node its shortest path for free), then group the paths by shape. /// - /// The hops name the FIELD they traverse (ClrMD's EnumerateReferencesWithFields), which - /// is what turns "a byte[] is reachable" into "AppData.Properties.GBProperty.PropertyChanged - /// holds it" — the sentence a developer can act on. + /// The result is ranked: the shape that retains the most instances comes first. That is the + /// answer to "what is holding all of this", as opposed to "here is a path to one of them". /// - public IReadOnlyList FindRootPaths(string typeName, int maxPaths, int maxHops) + public RetentionReport FindRetainers(string typeName, int sample, int maxHops) { + // ---- 1. the targets --------------------------------------------------------- // Match the TYPE, not the type's spelling. A naive substring match on the type name // matches `System.Func` when you asked for - // `GTDGoody` — a cached lambda whose *generic argument* happens to mention it — and - // then confidently reports a 2-hop path to the wrong object. A tool that points at the - // wrong culprit is worse than no tool. So compare the simple name with the generic - // arguments stripped. + // `GTDGoody` — a cached lambda whose *generic argument* happens to mention it — and then + // confidently reports a path to the wrong object. A tool that points at the wrong culprit + // is worse than no tool. var targets = new Dictionary(); + long totalOfType = 0; foreach (var o in Heap.EnumerateObjects()) { if (!o.IsValid || o.Type?.Name == null) continue; if (!IsType(o.Type.Name, typeName)) continue; - targets[o.Address] = o.Type.Name; - if (targets.Count >= maxPaths) break; + totalOfType++; + if (targets.Count < sample) targets[o.Address] = o.Type.Name; } - if (targets.Count == 0) return Array.Empty(); + if (targets.Count == 0) + return new RetentionReport(typeName, 0, 0, new List()); - var found = new List(); - var seen = new HashSet(); - var queue = new Queue(); + // ---- 2. one BFS from every root; parent pointers only (no strings) ------------ + // Storing a label per node would cost hundreds of MB on a 4M-object heap. Store the + // parent address, and resolve type/field names later, for the sampled paths only. + var parent = new Dictionary(); // child -> parent (0 = root) + var rootKind = new Dictionary(); + var queue = new Queue(); foreach (var root in Heap.EnumerateRoots()) { var o = root.Object; - if (!o.IsValid || !seen.Add(o.Address)) continue; - queue.Enqueue(new Node(o.Address, null, Describe(root, o), root.RootKind)); + if (!o.IsValid || parent.ContainsKey(o.Address)) continue; + parent[o.Address] = 0; + rootKind[o.Address] = root.RootKind; + queue.Enqueue(o.Address); } - while (queue.Count > 0 && found.Count < maxPaths) + int reachedTargets = 0; + while (queue.Count > 0 && reachedTargets < targets.Count) { - var node = queue.Dequeue(); + ulong addr = queue.Dequeue(); + if (targets.ContainsKey(addr)) reachedTargets++; + + var obj = Heap.GetObject(addr); + if (!obj.IsValid || obj.Type == null) continue; - if (targets.TryGetValue(node.Address, out var hitType)) + foreach (var child in obj.EnumerateReferences()) { - found.Add(new RetentionPathResult(hitType, Unwind(node), node.RootKind)); - continue; + if (!child.IsValid || parent.ContainsKey(child.Address)) continue; + parent[child.Address] = addr; + queue.Enqueue(child.Address); } + } - var obj = Heap.GetObject(node.Address); - if (!obj.IsValid || obj.Type == null) continue; - if (Depth(node) > maxHops) continue; + // ---- 3. unwind each sampled target, and group the paths by shape -------------- + var groups = new Dictionary(); + long retainedSampled = 0; + foreach (var kv in targets) + { + if (!parent.ContainsKey(kv.Key)) continue; // not reachable — genuinely garbage + retainedSampled++; + + var hops = Unwind(kv.Key, parent, rootKind, maxHops, out ClrRootKind kind); + string signature = string.Join(" -> ", hops.Select(h => h.Type)); + + if (!groups.TryGetValue(signature, out var retainer)) + { + retainer = new Retainer(hops, kind); + groups[signature] = retainer; + } + retainer.Instances++; + } + + var ranked = groups.Values.OrderByDescending(r => r.Instances).ToList(); + return new RetentionReport(targets.Values.First(), totalOfType, retainedSampled, ranked); + } + + /// + /// Walk the parent chain back to a root, naming the field traversed at every hop. The field + /// name is what turns "this object is alive" into "THIS FIELD is holding it" — the sentence a + /// developer can act on — so it is resolved here (by re-reading the parent's references), + /// rather than carried through the BFS at the cost of hundreds of megabytes. + /// + private List Unwind(ulong target, Dictionary parent, + Dictionary rootKind, int maxHops, + out ClrRootKind kind) + { + var chain = new List(); + ulong cur = target; + while (true) + { + chain.Add(cur); + if (!parent.TryGetValue(cur, out ulong p) || p == 0) break; + cur = p; + if (chain.Count > maxHops) break; + } + kind = rootKind.TryGetValue(cur, out var k) ? k : ClrRootKind.None; + chain.Reverse(); - // WithFields so a hop reads "GBProperty.PropertyChanged", not just "GBProperty". - foreach (var reference in obj.EnumerateReferencesWithFields()) + var hops = new List(chain.Count); + for (int i = 0; i < chain.Count; i++) + { + var obj = Heap.GetObject(chain[i]); + string type = obj.Type?.Name ?? "?"; + string? field = null; + if (i > 0) { - var child = reference.Object; - if (!child.IsValid || !seen.Add(child.Address)) continue; - string hop = child.Type?.Name ?? "?"; - if (reference.Field?.Name is string f && f.Length > 0) hop = hop + " (." + f + ")"; - queue.Enqueue(new Node(child.Address, node, hop, node.RootKind)); + var owner = Heap.GetObject(chain[i - 1]); + if (owner.IsValid && owner.Type != null) + { + foreach (var r in owner.EnumerateReferencesWithFields()) + { + if (r.Object.Address != chain[i]) continue; + field = r.Field?.Name; + break; + } + } } + hops.Add(new Hop(type, field)); } - return found; + return hops; } /// @@ -169,57 +244,20 @@ internal static bool IsType(string heapType, string wanted) { if (string.Equals(heapType, wanted, StringComparison.Ordinal)) return true; - // strip generic arguments: `Func` -> `Func`, `BindingList` -> `BindingList` - int lt = heapType.IndexOf('<'); + int lt = heapType.IndexOf('<'); // Func -> Func string bare = lt >= 0 ? heapType.Substring(0, lt) : heapType; - if (string.Equals(bare, wanted, StringComparison.Ordinal)) return true; - // allow the simple name: `GTDGoody` matches `BrokerDataClasses.GTDGoody` - int dot = bare.LastIndexOf('.'); + int dot = bare.LastIndexOf('.'); // Ns.GTDGoody -> GTDGoody string simple = dot >= 0 ? bare.Substring(dot + 1) : bare; return string.Equals(simple, wanted, StringComparison.Ordinal); } - private static string Describe(ClrRoot root, ClrObject o) => - "[" + root.RootKind + "] " + (o.Type?.Name ?? "?"); - - private static int Depth(Node n) - { - int d = 0; - for (var p = n.Parent; p != null; p = p.Parent) d++; - return d; - } - - private static IReadOnlyList Unwind(Node n) - { - var path = new List(); - for (var p = n; p != null; p = p.Parent) path.Add(p.Label); - path.Reverse(); - return path; - } - public void Dispose() { _runtime.Dispose(); _target.Dispose(); } - - private sealed class Node - { - public readonly ulong Address; - public readonly Node? Parent; - public readonly string Label; - public readonly ClrRootKind RootKind; - - public Node(ulong address, Node? parent, string label, ClrRootKind rootKind) - { - Address = address; - Parent = parent; - Label = label; - RootKind = rootKind; - } - } } internal struct TypeTally @@ -228,40 +266,30 @@ internal struct TypeTally public long Bytes; } - internal sealed class HeapCensus + internal sealed class Hop { - public readonly int Roots; - public readonly long HeapObjects; - public readonly long HeapBytes; - public readonly long RetainedObjects; - public readonly long RetainedBytes; - public readonly IReadOnlyDictionary ByType; + public readonly string Type; + public readonly string? Field; - public HeapCensus(int roots, long heapObjects, long heapBytes, - long retainedObjects, long retainedBytes, - IReadOnlyDictionary byType) + public Hop(string type, string? field) { - Roots = roots; - HeapObjects = heapObjects; - HeapBytes = heapBytes; - RetainedObjects = retainedObjects; - RetainedBytes = retainedBytes; - ByType = byType; + Type = type; + Field = field; } - /// The number that decides whether this is a leak hunt at all. - public double RetainedShare => HeapBytes == 0 ? 0 : 100.0 * RetainedBytes / HeapBytes; + public override string ToString() => + Field == null ? Type : Type + " (." + Field + ")"; } - internal sealed class RetentionPathResult + /// One distinct retention shape, and how many of the sampled instances it holds. + internal sealed class Retainer { - public readonly string TypeName; - public readonly IReadOnlyList Path; + public readonly IReadOnlyList Path; public readonly ClrRootKind RootKind; + public long Instances; - public RetentionPathResult(string typeName, IReadOnlyList path, ClrRootKind rootKind) + public Retainer(IReadOnlyList path, ClrRootKind rootKind) { - TypeName = typeName; Path = path; RootKind = rootKind; } @@ -272,20 +300,20 @@ public RetentionPathResult(string typeName, IReadOnlyList path, ClrRootK /// /// Note there is no `StaticVar` root kind: on .NET Framework a class's statics live in a /// pinned `System.Object[]` handed to the runtime as a **PinnedHandle**, which is why a - /// static-field leak surfaces as `[PinnedHandle] System.Object[] -> …`. So PinnedHandle is - /// where statics show up, and a **delegate hop** anywhere further down the path is what - /// makes it a static *event* rather than a plain static field — the distinction - /// correlate.py's `high` tier keys on. + /// static-field leak surfaces as `[PinnedHandle] System.Object[] -> …`. A **delegate hop** + /// further down the path is what makes it a static *event* rather than a plain static field — + /// the distinction correlate.py's `high` tier keys on. /// - /// `Stack` and `FinalizerQueue` are reported as themselves. They are not contract kinds - /// and deliberately so: an object rooted only by the stack is merely *live right now*, not - /// retained, and reading it as a leak is how a leak hunt goes wrong. + /// `Stack` and `FinalizerQueue` are reported as themselves, deliberately: an object rooted + /// only by the stack is merely *live right now*, not retained, and reading it as a leak is how + /// a leak hunt goes wrong. /// public string ContractKind() { - bool viaDelegate = Path.Any(p => - p.IndexOf("EventHandler", StringComparison.Ordinal) >= 0 || - p.IndexOf("MulticastDelegate", StringComparison.Ordinal) >= 0); + bool viaDelegate = Path.Any(h => + h.Type.IndexOf("EventHandler", StringComparison.Ordinal) >= 0 || + h.Type.IndexOf("MulticastDelegate", StringComparison.Ordinal) >= 0 || + (h.Field != null && h.Field.IndexOf("invocationList", StringComparison.OrdinalIgnoreCase) >= 0)); switch (RootKind) { @@ -299,5 +327,61 @@ public string ContractKind() return viaDelegate ? "static-event" : "gc-handle"; } } + + /// The object one hop above the target — the thing actually holding the reference. + public string Holder => Path.Count >= 2 ? Path[Path.Count - 2].Type : Path[0].Type; + + /// The field on that object, when the reference came from a named field. + public string? Member => Path.Count >= 1 ? Path[Path.Count - 1].Field : null; + + public string Render() + { + var sb = new StringBuilder(); + for (int i = 0; i < Path.Count; i++) + sb.Append(" ").Append(Path[i]).Append(Environment.NewLine); + return sb.ToString(); + } + } + + internal sealed class RetentionReport + { + public readonly string TypeName; + public readonly long TotalOnHeap; + public readonly long SampledRetained; + public readonly IReadOnlyList Retainers; + + public RetentionReport(string typeName, long totalOnHeap, long sampledRetained, + IReadOnlyList retainers) + { + TypeName = typeName; + TotalOnHeap = totalOnHeap; + SampledRetained = sampledRetained; + Retainers = retainers; + } + } + + internal sealed class HeapCensus + { + public readonly int Roots; + public readonly long HeapObjects; + public readonly long HeapBytes; + public readonly long RetainedObjects; + public readonly long RetainedBytes; + public readonly IReadOnlyDictionary ByType; + + public HeapCensus(int roots, long heapObjects, long heapBytes, + long retainedObjects, long retainedBytes, + IReadOnlyDictionary byType) + { + Roots = roots; + HeapObjects = heapObjects; + HeapBytes = heapBytes; + RetainedObjects = retainedObjects; + RetainedBytes = retainedBytes; + ByType = byType; + } + + /// The number that decides whether this is a leak hunt at all. + public double RetainedShare => HeapBytes == 0 ? 0 : 100.0 * RetainedBytes / HeapBytes; } } diff --git a/audit/runtime/RetentionPath/Program.cs b/audit/runtime/RetentionPath/Program.cs index ce8de203..aa6cbc34 100644 --- a/audit/runtime/RetentionPath/Program.cs +++ b/audit/runtime/RetentionPath/Program.cs @@ -21,12 +21,19 @@ namespace OwnNet.Audit.Runtime /// analyzer's blind spot, i.e. a rule request. /// /// Usage: - /// RetentionPath census --pid N | --dump D [--out runtime.json] [--top 25] - /// RetentionPath roots --pid N | --dump D --type Substring [--max-paths 3] [--max-hops 40] + /// RetentionPath census --pid N | --dump D [--out runtime.json] [--top 25] + /// RetentionPath roots --pid N | --dump D --type TypeName [--sample 200] [--max-hops 40] /// /// `census` prints the retained SHARE first, on purpose: if only 5% of the heap is /// reachable, there is no leak to hunt and the next step is a GC question, not a /// reference question. + /// + /// `roots` SAMPLES the instances and reports the paths as a ranked histogram, because + /// "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. The + /// question worth asking is "what holds the TYPICAL instance": the retainer that + /// accounts for 129,900 of 130,000 is the leak, and the three hanging off the stack or a + /// prototype are noise. /// internal static class Program { @@ -119,24 +126,55 @@ private static int Roots(RetentionWalker walker, string[] args) string? type = Arg(args, "--type"); if (type == null) { - Console.Error.WriteLine("retention-path roots: need --type "); + Console.Error.WriteLine("retention-path roots: need --type "); return 2; } - int maxPaths = ArgInt(args, "--max-paths", 3); + int sample = ArgInt(args, "--sample", 200); int maxHops = ArgInt(args, "--max-hops", 40); - var paths = walker.FindRootPaths(type, maxPaths, maxHops); - if (paths.Count == 0) + var report = walker.FindRetainers(type, sample, maxHops); + if (report.TotalOnHeap == 0) + { + Console.WriteLine($"no instance of {type} is on the heap"); + return 0; + } + if (report.SampledRetained == 0) { - Console.WriteLine($"no live instance of *{type}* is reachable from a GC root"); + Console.WriteLine($"{report.TotalOnHeap:N0} instance(s) of {type} on the heap, but NONE of the " + + "sample is reachable from a GC root — that is garbage, not a leak"); return 0; } - foreach (var p in paths) + Console.WriteLine($"{report.TypeName}: {report.TotalOnHeap:N0} on the heap, " + + $"{report.SampledRetained:N0} of a {sample:N0}-instance sample retained"); + Console.WriteLine(); + Console.WriteLine("RETAINERS, ranked — what holds the TYPICAL instance, not merely one of them:"); + + int rank = 0; + foreach (var r in report.Retainers) { + rank++; + double share = 100.0 * r.Instances / report.SampledRetained; Console.WriteLine(); - Console.WriteLine($"=== {p.TypeName} — retained via [{p.ContractKind()}], {p.Path.Count} hops ==="); - foreach (var hop in p.Path) Console.WriteLine(" " + hop); + Console.WriteLine($"#{rank} {r.Instances:N0}/{report.SampledRetained:N0} ({share:N1}%) " + + $"— via [{r.ContractKind()}], {r.Path.Count} hops"); + Console.Write(r.Render()); + if (rank >= 5) break; // the tail is noise; raise --sample for resolution + } + + Console.WriteLine(); + var dominant = report.Retainers[0]; + double dominantShare = 100.0 * dominant.Instances / report.SampledRetained; + if (dominantShare >= 50 && dominant.ContractKind() != "stack") + { + string member = dominant.Member != null ? "." + dominant.Member : ""; + Console.WriteLine($">>> {dominantShare:N1}% of the retained instances hang off ONE reference: " + + $"{dominant.Holder}{member} [{dominant.ContractKind()}]"); + } + else + { + Console.WriteLine(">>> no single dominant retainer in this sample — raise --sample, or the type " + + "really is held from many places"); } string? outPath = Arg(args, "--out"); @@ -145,23 +183,25 @@ private static int Roots(RetentionWalker walker, string[] args) var doc = new Dictionary { ["schema"] = "own-runtime/1", - ["retained"] = paths.Select(p => new Dictionary + ["retained"] = new object[] { - ["type"] = p.TypeName, - ["count"] = 1, - ["expected"] = 0, - ["bytes"] = 0, - ["roots"] = new object[] + new Dictionary { - new Dictionary + ["type"] = report.TypeName, + ["count"] = report.TotalOnHeap, + ["expected"] = 0, + ["bytes"] = 0, + ["roots"] = report.Retainers.Take(5).Select(r => new Dictionary { - ["kind"] = p.ContractKind(), - ["holder"] = p.Path.Count > 1 ? p.Path[1] : p.Path[0], - ["via"] = "reference", - ["path"] = p.Path, - }, + ["kind"] = r.ContractKind(), + ["holder"] = r.Holder, + ["member"] = r.Member ?? "", + ["via"] = r.ContractKind() == "static-event" ? "delegate" : "reference", + ["instances"] = r.Instances, + ["path"] = r.Path.Select(h => h.ToString()).ToList(), + }).ToList(), }, - }).ToList(), + }, }; File.WriteAllText(outPath, JsonConvert.SerializeObject(doc, Formatting.Indented)); Console.WriteLine(); @@ -192,7 +232,7 @@ private static int Usage() { Console.Error.WriteLine("usage:"); Console.Error.WriteLine(" RetentionPath census --pid | --dump [--out runtime.json] [--top 25]"); - Console.Error.WriteLine(" RetentionPath roots --pid | --dump --type [--max-paths 3] [--max-hops 40] [--out runtime.json]"); + Console.Error.WriteLine(" RetentionPath roots --pid | --dump --type [--sample 200] [--max-hops 40] [--out runtime.json]"); return 2; } } From 35368fbdb5c73f3925857062bb3beaf7671580c2 Mon Sep 17 00:00:00 2001 From: PhysShell Date: Wed, 15 Jul 2026 03:00:28 +0500 Subject: [PATCH 4/4] =?UTF-8?q?audit(runtime):=20dominator=20tree=20+=20re?= =?UTF-8?q?tained=20size=20=E2=80=94=20which=20ONE=20reference,=20if=20cut?= =?UTF-8?q?,=20frees=20the=20memory?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `roots` reports what holds the typical instance. It cannot tell you that CUTTING that reference would free anything, because an object held by two references at once is attributed to whichever is nearer. That is not an implementation shortcoming — "who holds it" is ill-posed. The well-posed question is "which single reference, if cut, makes this object collectable, and how much memory does that free", and dominance answers it: D dominates X when EVERY path from a root to X goes through D, so D's retained size is what you get back by dropping it. This is what Eclipse MAT and dotMemory are built on. It immediately paid for itself on the SectorTS leak. `roots` names the static PropertyChanged event, and it is not wrong — but `dominators` says: >>> NO single reference holds this memory — the biggest dominator accounts for only 6.7% (24 MB of 361 MB). The objects are reachable from SEVERAL roots at once, so cutting any one of them frees nothing. Which is correct, and is exactly what the real fix turned out to be: the working fix detaches SEVERAL references at once (UnregisterEventHandlers(false)). A shortest-path walk would have named one, confidently, and the fix would not have worked. Algorithm: Cooper-Harvey-Kennedy, "A Simple, Fast Dominance Algorithm" (2001) — the iterative formulation LLVM used for years. A page of code, converges in a couple of passes, no balanced forests. The paper is public; this is an implementation of it, not a copy of anyone's code. (PerfView, MIT, is the closest .NET reference — it computes a spanning tree with inclusive sizes, which approximates this.) The graph is CSR, not List>: ids are handed out in discovery order and BFS processes nodes in that same order, so each node's successors land contiguously in one int[]. ~150 bytes/object; 3.7M objects cost ~600 MB and 48 s attached live. Correctness is not taken on faith: * `RetentionPath selftest` (no target, no Windows, no ClrMD) checks the algorithm against graphs whose dominators are known by hand — a DIAMOND (an object reachable through both branches is dominated by NEITHER — the exact case a path walk gets wrong), a CHAIN (retained size accumulates), and a CYCLE (a gate dominates a reference cycle, which reference counting can never free). 13/13. * At run time the super-root's retained size must equal the total size of the reachable graph, or the walk REFUSES to report. A wrong dominator tree does not fail loudly — it quietly tells you to cut the wrong reference. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PjQnE1FDucd6vBVQbFswiE --- audit/runtime/README.md | 64 +++- audit/runtime/RetentionPath/Dominators.cs | 444 ++++++++++++++++++++++ audit/runtime/RetentionPath/Heap.cs | 6 + audit/runtime/RetentionPath/Program.cs | 82 +++- 4 files changed, 584 insertions(+), 12 deletions(-) create mode 100644 audit/runtime/RetentionPath/Dominators.cs diff --git a/audit/runtime/README.md b/audit/runtime/README.md index 234d41fd..73306d31 100644 --- a/audit/runtime/README.md +++ b/audit/runtime/README.md @@ -112,18 +112,62 @@ Output is the **`runtime.json` contract** (`OwnAudit/docs/runtime-contract.md`), as retained is `confirmed`; retention with **nothing static to explain it** is `runtime-only` — the analyzer's blind spot, and therefore a rule request. +### `dominators` — which ONE reference, if cut, frees the memory + +`roots` tells you what the *typical* path is. It cannot tell you that **cutting** that path would free +anything, because an object held by two references at once is attributed to whichever is nearer. That is +not a shortcoming of the implementation; the question "who holds it" is simply ill-posed. The well-posed +question is: + +> which single reference, if cut, makes this object collectable — and how much memory does that free? + +Dominance answers it. `D` dominates `X` when **every** path from a root to `X` goes through `D`, so `X`'s +immediate dominator is its one true retainer, and `D`'s **retained size** — everything it dominates — is +what you get back by dropping the reference to `D`. This is what Eclipse MAT and dotMemory are built on, +and it is why they can say *"detach this and you get 1.4 GB back"* while a path walk cannot. + +```powershell +RetentionPath.exe dominators --pid 1234 --top 8 + +3 727 278 reachable objects, 361 MB retained in total + +DOMINATORS — cut this ONE reference and the retained bytes go away: + retained MB own B type + 24,2 74 584 System.Object[] + 21,5 48 BaseDict.DictionaryList + 9,5 16 344 System.Object[] + +>>> NO single reference holds this memory — the biggest dominator accounts for only 6,7% + (24 MB of 361 MB). The objects are reachable from SEVERAL roots at once, so cutting any + one of them frees nothing. The fix must detach all of them. +``` + +**That verdict is the point of the verb.** On the SectorTS leak, `roots` names the static +`PropertyChanged` event and it is not wrong — but `dominators` shows that detaching it *alone* frees +nothing, because the same documents are also held by a static `List`. And that is exactly what +the real fix turned out to be: `UnregisterEventHandlers(false)` detaches **several** references at once. +A shortest-path walk would have named one of them, confidently, and the fix would not have worked. + +Algorithm: **Cooper–Harvey–Kennedy**, *"A Simple, Fast Dominance Algorithm"* (2001) — the iterative +formulation, a page of code, converging in a couple of passes on real graphs. (PerfView, MIT, is the +closest .NET reference; note it computes a *spanning tree* with inclusive sizes, which approximates +this.) The graph is held as CSR — a 4M-object heap will not fit in `List>`. + +Correctness is not taken on faith: + +* `RetentionPath selftest` — no target, no Windows, no ClrMD — checks the algorithm against graphs whose + dominators are known by hand: a **diamond** (an object reachable through both branches is dominated by + neither — the exact case a path walk gets wrong), a **chain** (retained size accumulates), and a + **cycle** (a gate dominates a reference cycle, which reference counting can never free). +* At run time the super-root's retained size must equal the total size of the reachable graph, or the + walk **refuses to report**. A wrong dominator tree does not fail loudly — it just tells you to cut the + wrong reference. + +Cost: ~48 s and ~600 MB of analyzer memory for a 3.7 M-object heap, attached live. A 40 M-object heap +will not fit; take a dump, or sample with `roots`. + ### What it does not do (read this before trusting it) -* **It cannot tell you that cutting the top retainer would free the object.** This is the important - one. The histogram partitions instances by their *shortest* path, so when an object is held by two - references at once, it is attributed to whichever is nearer — and the run above shows exactly that: - 50 % of the GTDs come out under the static event and 44 % under a static `List`, which most - likely means many are held by **both**. Detach the event and they still will not collect. - The question *"which single reference, if cut, frees this object — and how much memory does that - free"* is well-posed, and it has a standard answer this tool does not implement: a **dominator tree** - with retained sizes (Lengauer–Tarjan, or the iterative Cooper–Harvey–Kennedy formulation; it is what - Eclipse MAT and dotMemory are built on). That is the honest next step, and it is a feature, not a - tweak. * **A `[stack]` root is not retention.** It means the object is live in a frame *right now*. The tool labels it as such precisely so it is not mistaken for a leak; the same is true of `[finalizer]`. * **It matches the TYPE, not the type's spelling.** Asking for `GTDGoody` will not match diff --git a/audit/runtime/RetentionPath/Dominators.cs b/audit/runtime/RetentionPath/Dominators.cs new file mode 100644 index 00000000..6952af95 --- /dev/null +++ b/audit/runtime/RetentionPath/Dominators.cs @@ -0,0 +1,444 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Microsoft.Diagnostics.Runtime; + +namespace OwnNet.Audit.Runtime +{ + /// + /// The dominator tree of the object graph, and every object's RETAINED SIZE. + /// + /// WHY. "Who holds this object" is ill-posed when the object is reachable from several + /// roots — there are as many answers as there are paths, and a shortest-path walk just + /// picks one. The question that is well-posed, and the one an engineer actually needs, is: + /// + /// which single reference, if cut, makes this object collectable — + /// and how much memory does cutting it free? + /// + /// That is exactly what dominance answers. D dominates X when EVERY path from a root to X + /// goes through D. X's immediate dominator is therefore its one true retainer, and the + /// retained size of D — the total size of everything D dominates — is what you free by + /// dropping the reference to D. This is what Eclipse MAT and dotMemory are built on, and it + /// is why they can say "detach this and you get 1.4 GB back" while a path walk cannot. + /// + /// It also answers honestly in the awkward case. If the leaked objects are held by TWO + /// references at once, neither dominates them; their dominator sits higher up, at the point + /// the two paths meet — and the tool will say so, instead of confidently naming one of the + /// two and sending you off to cut a reference that frees nothing. + /// + /// ALGORITHM. Cooper–Harvey–Kennedy, "A Simple, Fast Dominance Algorithm" (2001) — the + /// iterative formulation LLVM used for years. Not Lengauer–Tarjan: CHK is a page of code, + /// converges in a couple of passes on real graphs, and needs no balanced forests. The paper + /// is public; this is an implementation of it, not a copy of anyone's code. (PerfView, MIT, + /// is the closest .NET reference — note it computes a *spanning tree* with inclusive sizes, + /// which is an approximation of this.) + /// + /// MEMORY. A 4M-object heap will not fit in `List<List<int>>`. The graph is built as CSR + /// (compressed sparse row): ids are handed out in discovery order and BFS processes nodes in + /// that same order, so each node's successors land contiguously and a single int[] holds every + /// edge. Budget roughly 150 bytes/object — a 4M-object heap costs ~600 MB in the analyzer, a + /// 40M-object heap will not fit and should be sampled instead. + /// + internal sealed class DominatorTree + { + // node 0 is a synthetic super-root whose successors are the GC roots. Every real object + // is therefore reachable from exactly one place, which is what makes dominance well-defined. + private readonly int _n; + private readonly ulong[] _address; + private readonly long[] _size; + private readonly int[] _succStart; // CSR: successors of u are _succ[_succStart[u] .. _succStart[u+1]) + private readonly int[] _succ; + private readonly int[] _idom; + private readonly int[] _rpoNum; // reverse-postorder index; -1 = unreachable + private readonly long[] _retained; + private readonly ClrHeap? _heap; // null in the self-test, where there is no target + + private DominatorTree(ClrHeap? heap, int n, ulong[] address, long[] size, + int[] succStart, int[] succ) + { + _heap = heap; + _n = n; + _address = address; + _size = size; + _succStart = succStart; + _succ = succ; + _idom = new int[n]; + _rpoNum = new int[n]; + _retained = new long[n]; + } + + /// + /// Dominate a graph given directly, with no heap behind it. Exists so the algorithm can be + /// tested without a target process — a dominator tree that is quietly wrong produces + /// confidently wrong advice ("cut this reference"), which is worse than no tool. + /// + internal static DominatorTree ForGraph(int n, int[] succStart, int[] succ, long[] size) + { + var t = new DominatorTree(null, n, new ulong[n], size, succStart, succ); + t.Dominate(); + t.ComputeRetained(); + return t; + } + + internal int IdomOf(int node) => _idom[node]; + internal long RetainedOf(int node) => _retained[node]; + + /// Walk the live graph once, then dominate it. + public static DominatorTree Build(ClrHeap heap) + { + // ---- 1. BFS the reachable graph into CSR --------------------------------------- + // ids are assigned in discovery order, and the queue hands nodes back in that same + // order, so a node's successors can be appended contiguously as it is processed. + var id = new Dictionary(); + var address = new List { 0 }; // node 0 = the synthetic super-root + var size = new List { 0 }; + var succStart = new List { 0 }; + var succ = new List(); + var queue = new Queue(); + + // the super-root's successors are the GC roots + foreach (var root in heap.EnumerateRoots()) + { + var o = root.Object; + if (!o.IsValid || o.Type == null) continue; + if (!id.TryGetValue(o.Address, out int rid)) + { + rid = address.Count; + id[o.Address] = rid; + address.Add(o.Address); + size.Add((long)o.Size); + queue.Enqueue(rid); + } + succ.Add(rid); + } + succStart.Add(succ.Count); // end of node 0's successor run + + while (queue.Count > 0) + { + int u = queue.Dequeue(); + // The invariant that makes CSR work: nodes are dequeued in ascending id order, so + // succStart is appended to in that order too. Assert it rather than trust it. + if (succStart.Count != u + 1) + throw new InvalidOperationException( + "BFS visited nodes out of id order — the CSR layout would be corrupt"); + + var obj = heap.GetObject(address[u]); + if (obj.IsValid && obj.Type != null) + { + foreach (var child in obj.EnumerateReferences()) + { + if (!child.IsValid || child.Type == null) continue; + if (!id.TryGetValue(child.Address, out int cid)) + { + cid = address.Count; + id[child.Address] = cid; + address.Add(child.Address); + size.Add((long)child.Size); + queue.Enqueue(cid); + } + succ.Add(cid); + } + } + succStart.Add(succ.Count); + } + + var t = new DominatorTree(heap, address.Count, address.ToArray(), size.ToArray(), + succStart.ToArray(), succ.ToArray()); + t.Dominate(); + t.ComputeRetained(); + return t; + } + + public int Count => _n; + public long TotalRetained => _retained.Length > 0 ? _retained[0] : 0; + + // ---- 2. reverse postorder over the successors ------------------------------------- + private int[] ReversePostorder() + { + var order = new List(_n); + var state = new byte[_n]; // 0 = unseen, 1 = on stack, 2 = done + var stack = new Stack<(int node, int next)>(); + + stack.Push((0, _succStart[0])); + state[0] = 1; + while (stack.Count > 0) + { + var (u, next) = stack.Pop(); + if (next < _succStart[u + 1]) + { + stack.Push((u, next + 1)); + int v = _succ[next]; + if (state[v] == 0) + { + state[v] = 1; + stack.Push((v, _succStart[v])); + } + } + else + { + state[u] = 2; + order.Add(u); // postorder + } + } + order.Reverse(); // reverse postorder + + for (int i = 0; i < _n; i++) _rpoNum[i] = -1; + for (int i = 0; i < order.Count; i++) _rpoNum[order[i]] = i; + return order.ToArray(); + } + + // ---- 3. predecessors (CHK needs them) --------------------------------------------- + private (int[] predStart, int[] pred) Predecessors() + { + var count = new int[_n + 1]; + for (int u = 0; u < _n; u++) + for (int e = _succStart[u]; e < _succStart[u + 1]; e++) + count[_succ[e] + 1]++; + + var predStart = new int[_n + 1]; + for (int i = 0; i < _n; i++) predStart[i + 1] = predStart[i] + count[i + 1]; + + var fill = new int[_n]; + var pred = new int[predStart[_n]]; + for (int u = 0; u < _n; u++) + for (int e = _succStart[u]; e < _succStart[u + 1]; e++) + { + int v = _succ[e]; + pred[predStart[v] + fill[v]++] = u; + } + return (predStart, pred); + } + + // ---- 4. Cooper–Harvey–Kennedy ------------------------------------------------------ + private void Dominate() + { + var rpo = ReversePostorder(); + var (predStart, pred) = Predecessors(); + + for (int i = 0; i < _n; i++) _idom[i] = -1; + _idom[0] = 0; + + bool changed = true; + while (changed) + { + changed = false; + foreach (int u in rpo) + { + if (u == 0) continue; + int newIdom = -1; + for (int e = predStart[u]; e < predStart[u + 1]; e++) + { + int p = pred[e]; + if (_idom[p] == -1) continue; // not processed yet this pass + newIdom = newIdom == -1 ? p : Intersect(p, newIdom); + } + if (newIdom != -1 && _idom[u] != newIdom) + { + _idom[u] = newIdom; + changed = true; + } + } + } + } + + /// Walk both fingers up the dominator chain until they meet. + private int Intersect(int a, int b) + { + while (a != b) + { + while (_rpoNum[a] > _rpoNum[b]) a = _idom[a]; + while (_rpoNum[b] > _rpoNum[a]) b = _idom[b]; + } + return a; + } + + // ---- 5. retained size --------------------------------------------------------------- + private void ComputeRetained() + { + for (int i = 0; i < _n; i++) _retained[i] = _size[i]; + + // idom[u] always precedes u in reverse postorder, so walking the RPO backwards means + // every node is finished before its dominator needs it. One pass, no recursion. + var rpo = new int[_n]; + for (int i = 0; i < _n; i++) if (_rpoNum[i] >= 0) rpo[_rpoNum[i]] = i; + for (int i = _n - 1; i >= 1; i--) + { + int u = rpo[i]; + if (u == 0) continue; + int d = _idom[u]; + if (d >= 0 && d != u) _retained[d] += _retained[u]; + } + + // The super-root dominates everything, so its retained size MUST be the total size of the + // reachable graph. If it is not, the dominator tree is wrong — and a wrong dominator tree + // does not fail loudly, it just tells you to cut the wrong reference. Check it. + long total = 0; + for (int i = 0; i < _n; i++) total += _size[i]; + if (_retained[0] != total) + throw new InvalidOperationException( + $"dominator tree is inconsistent: super-root retains {_retained[0]:N0} B but the " + + $"reachable graph is {total:N0} B — refusing to report a result that would be wrong"); + } + + /// + /// The algorithm, checked against graphs whose dominators are known by hand. Runs anywhere — + /// no target, no Windows. `RetentionPath selftest`. + /// + internal static bool SelfTest(Action log) + { + bool ok = true; + + void Check(string what, bool cond) + { + log((cond ? " ok " : " FAIL ") + what); + if (!cond) ok = false; + } + + // (a) a diamond — the case that matters. 3 is reachable through BOTH 1 and 2, so + // NEITHER dominates it: its immediate dominator is the root. This is precisely the + // shape a shortest-path walk gets wrong (it would name 1, or 2, and be confident). + // 0 -> 1 -> 3 -> 4 + // 0 -> 2 -> 3 + { + // 0 1 2 3 4 + var start = new[] { 0, 2, 3, 4, 5, 5 }; + var succ = new[] { 1, 2, 3, 3, 4 }; + var size = new long[] { 0, 10, 10, 10, 10 }; + var t = ForGraph(5, start, succ, size); + + Check("diamond: idom(1) = 0", t.IdomOf(1) == 0); + Check("diamond: idom(2) = 0", t.IdomOf(2) == 0); + Check("diamond: idom(3) = 0 (held by BOTH 1 and 2 — neither dominates)", t.IdomOf(3) == 0); + Check("diamond: idom(4) = 3", t.IdomOf(4) == 3); + Check("diamond: retained(3) = 20 (itself + 4)", t.RetainedOf(3) == 20); + Check("diamond: retained(1) = 10 (it does NOT retain 3)", t.RetainedOf(1) == 10); + Check("diamond: retained(root) = 40", t.RetainedOf(0) == 40); + } + + // (b) a chain — every link dominates the rest, so retained size accumulates. + // 0 -> 1 -> 2 -> 3 + { + var start = new[] { 0, 1, 2, 3, 3 }; + var succ = new[] { 1, 2, 3 }; + var size = new long[] { 0, 10, 20, 30 }; + var t = ForGraph(4, start, succ, size); + + Check("chain: idom(3) = 2", t.IdomOf(3) == 2); + Check("chain: retained(1) = 60 (the whole tail)", t.RetainedOf(1) == 60); + Check("chain: retained(2) = 50", t.RetainedOf(2) == 50); + } + + // (c) a cycle below a single gate. 1 gates the cycle 2<->3, so 1 dominates both even + // though they point at each other. A naive reference-count would never free them. + { + // 0 1 2 3 + var start = new[] { 0, 1, 2, 3, 4 }; + var succ = new[] { 1, 2, 3, 2 }; + var size = new long[] { 0, 10, 10, 10 }; + var t = ForGraph(4, start, succ, size); + + Check("cycle: idom(2) = 1", t.IdomOf(2) == 1); + Check("cycle: idom(3) = 2", t.IdomOf(3) == 2); + Check("cycle: retained(1) = 30 (cut 1 and the whole cycle collects)", t.RetainedOf(1) == 30); + } + + log(ok ? "dominator selftest: OK" : "dominator selftest: FAILED"); + return ok; + } + + /// + /// The objects whose removal frees the most memory — i.e. the answer to "what is holding + /// all of this". The super-root is skipped (it dominates everything by construction, which + /// is true and useless). + /// + public IReadOnlyList Top(int count, long minBytes) + { + var hits = new List(); + for (int u = 1; u < _n; u++) + { + if (_retained[u] < minBytes) continue; + hits.Add(new DominatorHit(u, _address[u], _retained[u], _size[u])); + } + hits.Sort((a, b) => b.RetainedBytes.CompareTo(a.RetainedBytes)); + + // A dominator chain reports the same bytes at every link (a -> b -> c each "retain" + // the subtree). Reporting all of them is noise; keep a link only if it retains + // meaningfully more than the child that follows it — i.e. the points where the graph + // actually branches. Otherwise the top-20 is one chain, twenty times. + var kept = new List(); + var claimed = new HashSet(); + foreach (var h in hits) + { + if (kept.Count >= count) break; + bool redundant = false; + for (int d = _idom[h.Node]; d > 0 && d != _idom[d]; d = _idom[d]) + { + if (claimed.Contains(d) && _retained[d] < h.RetainedBytes * 11 / 10) + { + redundant = true; // an ancestor already reported ~the same bytes + break; + } + } + if (redundant) continue; + claimed.Add(h.Node); + kept.Add(h); + } + return kept; + } + + /// The dominator chain from the super-root down to a node, naming each type. + public IReadOnlyList ChainTo(int node, int maxHops) + { + var chain = new List(); + for (int u = node; u > 0 && chain.Count < maxHops; u = _idom[u]) + { + chain.Add(u); + if (_idom[u] == u) break; + } + chain.Reverse(); + return chain.Select(TypeOf).ToList(); + } + + public string TypeOf(int node) + { + if (_heap == null) return "#" + node; + var o = _heap.GetObject(_address[node]); + return o.Type?.Name ?? "?"; + } + + /// Retained bytes grouped by the TYPE of the dominator — "what class of thing is holding memory". + public IReadOnlyList<(string Type, long Retained, long Count)> ByDominatorType(int top) + { + var acc = new Dictionary(); + for (int u = 1; u < _n; u++) + { + // count a node only where it is the immediate dominator of something — otherwise + // every leaf would "retain" itself and the table would just be a type histogram. + if (_idom[u] <= 0) continue; + string t = TypeOf(_idom[u]); + var cur = acc.TryGetValue(t, out var v) ? v : (0L, 0L); + acc[t] = (cur.Item1 + _retained[u], cur.Item2 + 1); + } + return acc.OrderByDescending(kv => kv.Value.bytes) + .Take(top) + .Select(kv => (kv.Key, kv.Value.bytes, kv.Value.n)) + .ToList(); + } + } + + internal sealed class DominatorHit + { + public readonly int Node; + public readonly ulong Address; + public readonly long RetainedBytes; + public readonly long OwnBytes; + + public DominatorHit(int node, ulong address, long retainedBytes, long ownBytes) + { + Node = node; + Address = address; + RetainedBytes = retainedBytes; + OwnBytes = ownBytes; + } + } +} diff --git a/audit/runtime/RetentionPath/Heap.cs b/audit/runtime/RetentionPath/Heap.cs index 56b46256..e11bc093 100644 --- a/audit/runtime/RetentionPath/Heap.cs +++ b/audit/runtime/RetentionPath/Heap.cs @@ -253,6 +253,12 @@ 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 DominatorTree Dominate() => DominatorTree.Build(Heap); + public void Dispose() { _runtime.Dispose(); diff --git a/audit/runtime/RetentionPath/Program.cs b/audit/runtime/RetentionPath/Program.cs index aa6cbc34..a8c1dade 100644 --- a/audit/runtime/RetentionPath/Program.cs +++ b/audit/runtime/RetentionPath/Program.cs @@ -42,6 +42,11 @@ private static int Main(string[] args) if (args.Length == 0) return Usage(); string verb = args[0].ToLowerInvariant(); + // The algorithm, checked against graphs whose dominators are known by hand. No target, + // no Windows, no ClrMD — so it can be run anywhere, including by a reviewer. + if (verb == "selftest") + return DominatorTree.SelfTest(Console.WriteLine) ? 0 : 1; + int pid = ArgInt(args, "--pid", 0); string? dump = Arg(args, "--dump"); if (pid == 0 && dump == null) @@ -60,6 +65,7 @@ private static int Main(string[] args) { case "census": return Census(walker, args); case "roots": return Roots(walker, args); + case "dominators": return Dominators(walker, args); default: return Usage(); } } @@ -211,6 +217,73 @@ private static int Roots(RetentionWalker walker, string[] args) return 1; // retention found } + /// + /// The question `roots` cannot answer: which single reference, if cut, frees the object — + /// and how much memory does cutting it free. Dominance answers it; a path walk cannot. + /// + private static int Dominators(RetentionWalker walker, string[] args) + { + int top = ArgInt(args, "--top", 15); + long minMb = ArgInt(args, "--min-mb", 1); + + Console.WriteLine("building the object graph and dominating it (Cooper-Harvey-Kennedy)..."); + var tree = walker.Dominate(); + Console.WriteLine($"{tree.Count - 1:N0} reachable objects, " + + $"{Mb(tree.TotalRetained):N0} MB retained in total"); + Console.WriteLine(); + + var hits = tree.Top(top, minMb * 1024 * 1024); + + Console.WriteLine("DOMINATORS — cut this ONE reference and the retained bytes go away:"); + Console.WriteLine(); + if (hits.Count == 0) + { + Console.WriteLine($" (none: no single object dominates as much as {minMb} MB)"); + } + else + { + Console.WriteLine($"{"retained MB",13}{"own B",10} {"type",-50}"); + foreach (var h in hits) + Console.WriteLine($"{Mb(h.RetainedBytes),13:N1}{h.OwnBytes,10:N0} {Short(tree.TypeOf(h.Node)),-50}"); + } + + // THE headline, and the reason this verb exists. If the biggest single dominator accounts + // for a sliver of the retained heap, then the memory is held from SEVERAL places at once — + // no one reference dominates it, and cutting any one of them frees nothing. A shortest-path + // walk cannot tell you that; it will happily name one path and send you off to cut it. + long biggest = hits.Count > 0 ? hits[0].RetainedBytes : 0; + double explained = tree.TotalRetained == 0 ? 0 : 100.0 * biggest / tree.TotalRetained; + Console.WriteLine(); + if (explained >= 25) + { + Console.WriteLine($">>> ONE reference holds {explained:N1}% of the retained heap " + + $"({Mb(biggest):N0} MB of {Mb(tree.TotalRetained):N0} MB). Cut it and that memory returns."); + Console.WriteLine(); + Console.WriteLine("its dominator chain (root -> … -> it):"); + foreach (var t in tree.ChainTo(hits[0].Node, 24)) + Console.WriteLine(" " + Short(t)); + } + else + { + Console.WriteLine($">>> NO single reference holds this memory — the biggest dominator accounts for " + + $"only {explained:N1}% ({Mb(biggest):N0} MB of {Mb(tree.TotalRetained):N0} MB)."); + Console.WriteLine(" The objects are reachable from SEVERAL roots at once, so cutting any one of"); + Console.WriteLine(" them frees nothing. The fix must detach all of them. (A shortest-path walk"); + Console.WriteLine(" would have named one and been confidently wrong.)"); + } + + Console.WriteLine(); + Console.WriteLine("RETAINED BY DOMINATOR TYPE — which class of object owns the subtrees."); + Console.WriteLine("NB this is STRUCTURE, not blame: a GTDGoody dominating its own fields is not a leak."); + Console.WriteLine("Read it as 'where the bytes live', then ask `roots` who keeps that alive."); + Console.WriteLine(); + Console.WriteLine($"{"retained MB",13}{"dominated",11} {"dominator type",-50}"); + foreach (var (type, retained, n) in tree.ByDominatorType(12)) + Console.WriteLine($"{Mb(retained),13:N1}{n,11:N0} {Short(type),-50}"); + + return hits.Count > 0 ? 1 : 0; + } + private static double Mb(long bytes) => bytes / 1024.0 / 1024.0; private static string Short(string t) => @@ -231,8 +304,13 @@ private static int ArgInt(string[] args, string name, int fallback) private static int Usage() { Console.Error.WriteLine("usage:"); - Console.Error.WriteLine(" RetentionPath census --pid | --dump [--out runtime.json] [--top 25]"); - Console.Error.WriteLine(" RetentionPath roots --pid | --dump --type [--sample 200] [--max-hops 40] [--out runtime.json]"); + Console.Error.WriteLine(" RetentionPath census --pid | --dump [--out runtime.json] [--top 25]"); + Console.Error.WriteLine(" RetentionPath roots --pid | --dump --type [--sample 200] [--max-hops 40] [--out runtime.json]"); + Console.Error.WriteLine(" RetentionPath dominators --pid | --dump [--top 15] [--min-mb 1]"); + Console.Error.WriteLine(); + Console.Error.WriteLine(" census is there anything retained at all, or is the heap just uncollected garbage?"); + Console.Error.WriteLine(" roots what holds the TYPICAL instance of a type (sampled, ranked)"); + Console.Error.WriteLine(" dominators which ONE reference, if cut, frees the memory — and how much"); return 2; } }