From a26a3f652664172de9398d7deddcaac75046fcb9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 13:33:07 +0000 Subject: [PATCH 1/7] =?UTF-8?q?test(corpus):=20red=20=E2=80=94=20#278=20a?= =?UTF-8?q?=20`-=3D`=20that=20exists=20is=20not=20a=20`-=3D`=20that=20runs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two corpus/wpf cases pin the OWN001 false negative from issue #278, both heap-motivated by the SectorTS GTD leak (66% retained heap, ClrMD-proven): * subscription-param-guarded-unregister — ctor `+=`, the only `-=` inside `UnregisterEventHandlers(bool UnregOnlyGoodys)` behind `if (!UnregOnlyGoodys)`; the leaking callers pass `true`. before.cs must be OWN001; after.cs releases unconditionally in Dispose and must stay silent. * subscription-nonteardown-release — ctor `+=`, an unconditional `-=` in an arbitrary non-teardown method nobody is proven to call. before.cs must be OWN001; after.cs detaches in a handler wired to the class's own Unloaded lifecycle event and must stay silent (the recognised-teardown control). Under the shipped "any matching `-=` in the class = released" model both before.cs are silent — the red half of this pair. The case.own reductions already fail honestly (guard modelled as an early return past the release; non-teardown `-=` not modelled as a release). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01MdYFKUaygHz1T9H1qJ7BqK --- .../subscription-nonteardown-release/after.cs | 32 ++++++++++ .../before.cs | 30 ++++++++++ .../subscription-nonteardown-release/case.own | 20 +++++++ .../expected-diagnostics.txt | 1 + .../subscription-nonteardown-release/notes.md | 44 ++++++++++++++ .../after.cs | 28 +++++++++ .../before.cs | 38 ++++++++++++ .../case.own | 24 ++++++++ .../expected-diagnostics.txt | 1 + .../notes.md | 58 +++++++++++++++++++ 10 files changed, 276 insertions(+) create mode 100644 corpus/wpf/subscription-nonteardown-release/after.cs create mode 100644 corpus/wpf/subscription-nonteardown-release/before.cs create mode 100644 corpus/wpf/subscription-nonteardown-release/case.own create mode 100644 corpus/wpf/subscription-nonteardown-release/expected-diagnostics.txt create mode 100644 corpus/wpf/subscription-nonteardown-release/notes.md create mode 100644 corpus/wpf/subscription-param-guarded-unregister/after.cs create mode 100644 corpus/wpf/subscription-param-guarded-unregister/before.cs create mode 100644 corpus/wpf/subscription-param-guarded-unregister/case.own create mode 100644 corpus/wpf/subscription-param-guarded-unregister/expected-diagnostics.txt create mode 100644 corpus/wpf/subscription-param-guarded-unregister/notes.md diff --git a/corpus/wpf/subscription-nonteardown-release/after.cs b/corpus/wpf/subscription-nonteardown-release/after.cs new file mode 100644 index 00000000..622e8cd9 --- /dev/null +++ b/corpus/wpf/subscription-nonteardown-release/after.cs @@ -0,0 +1,32 @@ +// FIXED. The detach moved into a recognised lifecycle teardown: a handler the +// class wires to its OWN `Unloaded` event in the ctor. The platform raises +// `Unloaded` when the view leaves the tree, so the `-=` provably runs at the +// subscriber's end-of-life — this is the P-004 teardown shape as written +// ("no matching `-=` in Dispose/OnClosed/Unloaded" is the finding; a `-=` IN +// one of those contexts is the fix). +// +// own-check MUST treat this as released (silent) — the recognised lifecycle +// teardown keeps its existing no-finding behaviour under #278. +using System; +using System.ComponentModel; + +public sealed class PriceListener +{ + private readonly INotifyPropertyChanged _prices; // injected, unknown lifetime + + public event EventHandler Unloaded; // raised by the host when the view is torn down + + public PriceListener(INotifyPropertyChanged prices) + { + _prices = prices; + _prices.PropertyChanged += OnPricesChanged; + Unloaded += OnViewUnloaded; + } + + private void OnViewUnloaded(object sender, EventArgs e) + { + _prices.PropertyChanged -= OnPricesChanged; // teardown context: the class's own Unloaded hook + } + + private void OnPricesChanged(object sender, PropertyChangedEventArgs e) { /* ... */ } +} diff --git a/corpus/wpf/subscription-nonteardown-release/before.cs b/corpus/wpf/subscription-nonteardown-release/before.cs new file mode 100644 index 00000000..54f79697 --- /dev/null +++ b/corpus/wpf/subscription-nonteardown-release/before.cs @@ -0,0 +1,30 @@ +// BUGGY (issue #278, rule 3; hand-reduced into case.own). +// +// A listener subscribes to an injected publisher in its ctor. The only matching +// `-=` is UNCONDITIONAL — but it sits in an arbitrary method (`StopListening`) +// that is not a teardown: nothing here proves any owner ever calls it (in the +// real SectorTS analog, an entire subsystem constructs these objects and never +// calls the unregister method). The mere EXISTENCE of a `-=` is not evidence +// that it RUNS. +// +// own-check MUST flag this OWN001. The old "any matching `-=` in the class = +// released" model silenced it — the false negative this case pins. +using System.ComponentModel; + +public sealed class PriceListener +{ + private readonly INotifyPropertyChanged _prices; // injected, unknown lifetime + + public PriceListener(INotifyPropertyChanged prices) + { + _prices = prices; + _prices.PropertyChanged += OnPricesChanged; + } + + public void StopListening() + { + _prices.PropertyChanged -= OnPricesChanged; // unconditional, but nobody has to call this + } + + private void OnPricesChanged(object sender, PropertyChangedEventArgs e) { /* ... */ } +} diff --git a/corpus/wpf/subscription-nonteardown-release/case.own b/corpus/wpf/subscription-nonteardown-release/case.own new file mode 100644 index 00000000..809aec34 --- /dev/null +++ b/corpus/wpf/subscription-nonteardown-release/case.own @@ -0,0 +1,20 @@ +module WpfSubscriptionNonTeardownRelease + +// A subscription token: `+= handler` acquires the source<->listener edge; the +// matching `-= handler` releases it. `kind` tags the resource so the generic +// ownership finding carries a [resource: ...] note. +resource Subscription { + acquire Subscribe + release Unsubscribe + kind "subscription token" +} + +// The ctor subscribes; the only `-=` lives in an arbitrary non-teardown method +// (`StopListening`) that no lifecycle path is proven to call. Modelled as the +// ctor scope alone: the token is acquired and never released within any +// teardown => OWN001. (The `-=`'s existence elsewhere is not modelled as a +// release — that is exactly the #278 rule: existence is not execution.) +fn PriceListener(prices: int) { + let sub = acquire Subscription(prices); + // no `release sub;` on any teardown path -> unreleased subscription (before.cs) +} diff --git a/corpus/wpf/subscription-nonteardown-release/expected-diagnostics.txt b/corpus/wpf/subscription-nonteardown-release/expected-diagnostics.txt new file mode 100644 index 00000000..ed2a1929 --- /dev/null +++ b/corpus/wpf/subscription-nonteardown-release/expected-diagnostics.txt @@ -0,0 +1 @@ +OWN001 diff --git a/corpus/wpf/subscription-nonteardown-release/notes.md b/corpus/wpf/subscription-nonteardown-release/notes.md new file mode 100644 index 00000000..5afc7181 --- /dev/null +++ b/corpus/wpf/subscription-nonteardown-release/notes.md @@ -0,0 +1,44 @@ +# Subscription whose only `-=` is in an arbitrary non-teardown method (soundness FN) + +**Pattern (issue #278, rule 3).** A listener subscribes in its ctor; the only +matching `-=` is unconditional but lives in an arbitrary method +(`StopListening()`) that is not a teardown. Nothing proves any owner calls it: +in the SectorTS analog the `DocCloud` subsystem constructs the objects through +AutoMapper profiles and never calls the unregister method — every instance stays +pinned to the publisher. + +**The bug (Own.NET extractor).** The shipped release model credited *any* +matching `-=` anywhere in the class, so this shape was silent. The design docs +specified the stricter rule all along (P-004: "no matching `-=` in +`Dispose`/`OnClosed`/`Unloaded`"; P-001 the same) — the implementation was looser +than its own spec, in the unsound direction. + +**The fix (#278).** A `-=` credits release only in a recognised teardown context: +`Dispose`/`DisposeAsync`/`OnClosed`/`OnUnloaded`-style methods, a finalizer, a +handler wired to the class's own `Closed`/`Closing`/`Unloaded`-style lifecycle +event (including the XAML `Window_Closing` naming convention), or a method the +teardown path calls intra-class. An arbitrary method grounds nothing, so +`before.cs` keeps the honest OWN001. At most a non-teardown `-=` is a +*mitigation candidate* — never silence. + +**Why `after.cs` uses an `Unloaded` hook (not `Dispose`).** The matching ok-case +for a `Dispose` release already exists +(`corpus/wpf/subscription-explicit-delegate-release`, +`subscription-param-guarded-unregister`). This case's `after.cs` pins the OTHER +acceptance half: a **recognised lifecycle teardown keeps its existing +no-finding behaviour** — the class wires `Unloaded += OnViewUnloaded` and +detaches there, and the extractor recognises the wired handler as a teardown +context. (The `Unloaded += OnViewUnloaded` wiring itself is a self-owned-source +subscription — `this`'s own event — and stays exempt as before.) + +**What the checker says (`.own` reduction).** The ctor scope acquires the token +and no teardown path releases it => **OWN001** with the subscription-token +resource tag. The non-teardown `-=` is deliberately NOT modelled as a release — +existence is not execution. + +**Regression guard.** `scripts/benchmark.py`: `before.cs` must be **caught**, +`after.cs` must be **silent**. Before the #278 fix, `before.cs` was silent. + +**Honesty / scope.** `case.own` carries the acquire/release logic only; the +teardown-context recognition lives in the extractor. The C# is representative, +not a verbatim SectorTS copy. diff --git a/corpus/wpf/subscription-param-guarded-unregister/after.cs b/corpus/wpf/subscription-param-guarded-unregister/after.cs new file mode 100644 index 00000000..b1b9faf7 --- /dev/null +++ b/corpus/wpf/subscription-param-guarded-unregister/after.cs @@ -0,0 +1,28 @@ +// FIXED. The one subscription the ctor creates is released UNCONDITIONALLY in +// Dispose — a recognised teardown context with no caller-controlled guard, so +// the release provably runs when the owner is torn down. +// +// own-check MUST treat this as released (silent): the `-=` is in `Dispose`, +// matches the `+=`'s (receiver, handler) pair (the explicit delegate-creation +// on the `+=` normalizes to the bare method group on the `-=`), and no +// parameter of the enclosing method can skip it. +using System; +using System.ComponentModel; + +public sealed class GoodsDocument : IDisposable +{ + private readonly INotifyPropertyChanged _properties; // injected, unknown lifetime + + public GoodsDocument(INotifyPropertyChanged properties) + { + _properties = properties; + _properties.PropertyChanged += new PropertyChangedEventHandler(OnPropertiesChanged); + } + + public void Dispose() + { + _properties.PropertyChanged -= OnPropertiesChanged; // unconditional, in a teardown + } + + private void OnPropertiesChanged(object sender, PropertyChangedEventArgs e) { /* ... */ } +} diff --git a/corpus/wpf/subscription-param-guarded-unregister/before.cs b/corpus/wpf/subscription-param-guarded-unregister/before.cs new file mode 100644 index 00000000..015343c4 --- /dev/null +++ b/corpus/wpf/subscription-param-guarded-unregister/before.cs @@ -0,0 +1,38 @@ +// BUGGY (SectorTS GTD shape, issue #278; hand-reduced into case.own). +// +// A data class subscribes to an injected publisher in its ctor and its ONLY +// matching `-=` sits inside a method that is NOT a teardown, behind a bool +// parameter of that method: `UnregisterEventHandlers(bool UnregOnlyGoodys)` +// with the detach under `if (!UnregOnlyGoodys)`. The leaking callers pass +// `true` (and one whole subsystem never calls it at all), so the `-=` provably +// does NOT run on those paths — the subscription pins the document graph to the +// publisher for the life of the process (heap-proven: 66% retained heap after +// 31 documents, GTD.cs:5192). +// +// own-check MUST flag this OWN001. The old "any matching `-=` in the class = +// released" model paired the ctor `+=` with this flag-skipped `-=` and stayed +// silent — the false negative this case pins. +using System.ComponentModel; + +public sealed class GoodsDocument +{ + private readonly INotifyPropertyChanged _properties; // injected, unknown lifetime + + public GoodsDocument(INotifyPropertyChanged properties) + { + _properties = properties; + // SectorTS idiom: explicit delegate-creation on the `+=`. + _properties.PropertyChanged += new PropertyChangedEventHandler(OnPropertiesChanged); + } + + public void UnregisterEventHandlers(bool UnregOnlyGoodys = false) + { + if (!UnregOnlyGoodys) // callers pass true; the block never runs + { + _properties.PropertyChanged -= OnPropertiesChanged; + } + // only the goods rows are detached here + } + + private void OnPropertiesChanged(object sender, PropertyChangedEventArgs e) { /* ... */ } +} diff --git a/corpus/wpf/subscription-param-guarded-unregister/case.own b/corpus/wpf/subscription-param-guarded-unregister/case.own new file mode 100644 index 00000000..5bc13988 --- /dev/null +++ b/corpus/wpf/subscription-param-guarded-unregister/case.own @@ -0,0 +1,24 @@ +module WpfSubscriptionParamGuardedUnregister + +// A subscription token: `+= handler` acquires the source<->listener edge; the +// matching `-= handler` releases it. `kind` tags the resource so the generic +// ownership finding carries a [resource: ...] note. +resource Subscription { + acquire Subscribe + release Unsubscribe + kind "subscription token" +} + +// The ctor subscribes; the only release sits behind a caller-controlled flag in +// a non-teardown method (`UnregisterEventHandlers(bool UnregOnlyGoodys)` under +// `if (!UnregOnlyGoodys)`). Modelled as one scope: on the flag=true path the +// release never runs, so the token is not released on all paths => OWN001. +// The real callers DO pass true (GTDService, DocCloud) — the leak path is the +// production path, not a corner. +fn GoodsDocument(properties: int, unregOnlyGoodys: int) { + let sub = acquire Subscription(properties); + if (unregOnlyGoodys) { + return; // caller passed true -> the `-=` block is skipped + } + release sub; // the guarded `-=`: runs only when the flag is false +} diff --git a/corpus/wpf/subscription-param-guarded-unregister/expected-diagnostics.txt b/corpus/wpf/subscription-param-guarded-unregister/expected-diagnostics.txt new file mode 100644 index 00000000..ed2a1929 --- /dev/null +++ b/corpus/wpf/subscription-param-guarded-unregister/expected-diagnostics.txt @@ -0,0 +1 @@ +OWN001 diff --git a/corpus/wpf/subscription-param-guarded-unregister/notes.md b/corpus/wpf/subscription-param-guarded-unregister/notes.md new file mode 100644 index 00000000..c0ba8a68 --- /dev/null +++ b/corpus/wpf/subscription-param-guarded-unregister/notes.md @@ -0,0 +1,58 @@ +# Subscription with a parameter-guarded `-=` in a non-teardown method (soundness FN) + +**Pattern (SectorTS `GTD`, issue #278).** A data class subscribes to a publisher +in its ctor. A matching `-=` exists — but it lives inside +`UnregisterEventHandlers(bool UnregOnlyGoodys = false)`, which is not a teardown, +and behind `if (!UnregOnlyGoodys)`: + +```csharp +AppData.Properties.GBProperty.PropertyChanged += GBProperty_PropertyChanged; // ctor + +public void UnregisterEventHandlers(bool UnregOnlyGoodys = false) +{ + if (!UnregOnlyGoodys) // <-- GTDService/DocCloud pass true; the block never runs + { + AppData.Properties.GBProperty.PropertyChanged -= GBProperty_PropertyChanged; + } +} +``` + +`Service/GTDService.cs` calls it with `true` at 5 sites, and the `DocCloud` +subsystem (8+ AutoMapper `.ConstructUsing(x => new GTD(null, null))` profiles) +never calls it at all — every mapped document pins itself to the static publisher +for the life of the process. Runtime proof (ClrMD retention-path walk, 31 +documents): 66.3% of the heap genuinely retained, with the path +`[PinnedHandle] -> KernelProperty -> PropertyChangedEventHandler -> GTD`. + +**The bug (Own.NET extractor).** The shipped release model treated *any* matching +`-=` anywhere in the class as releasing the subscription — no check that the +method holding it is a teardown, is ever called, or that the `-=` is not guarded +away by a parameter. So OWN001 paired the ctor `+=` with this flag-skipped `-=` +and stayed **silent** on the very codebase the heuristic was tuned against — a +false negative that silently swallows a leak class (the #238 doctrine violation: +the worst case of an exemption must be "keeps today's honest warning", never +"silently swallows a leak class"). + +**The fix (#278).** A matching `-=` credits release only when it sits in a +recognised teardown context (`Dispose`/`DisposeAsync`/`OnClosed`/`Unloaded`/…, a +handler wired to the class's own `Closed`/`Unloaded`-style lifecycle event, or a +method the teardown path calls intra-class) AND is not guarded by a parameter of +its enclosing method. `UnregisterEventHandlers(bool)` fails both rules, so +`before.cs` is flagged OWN001. `after.cs` releases unconditionally in `Dispose` +— a teardown with no caller-controlled guard — and stays silent. + +**What the checker says (`.own` reduction).** The guard is modelled as an early +`return` before the `release`: on the flag=true path the token is never +released, so the core reports **OWN001** ("not released on all paths") with the +subscription-token resource tag. + +**Regression guard.** `scripts/benchmark.py` runs the real C# through the +extractor + core: `before.cs` must be **caught** (the leak is real and +heap-proven) and `after.cs` must be **silent**. Before the #278 fix, `before.cs` +was silent — the false negative this case pins. + +**Honesty / scope.** `case.own` is a hand reduction carrying the acquire/release +logic; the teardown-context and parameter-guard reasoning lives in the extractor +(`frontend/roslyn/OwnSharp.Extractor/Program.cs`, the `unsub` gating). The C# +here is representative of the SectorTS idiom (explicit delegate-creation `+=`, +default-parameter unregister), not a verbatim copy of `GTD.cs`. From 229ece71b8c4b168aa4267b180204f294a83b310 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 13:33:24 +0000 Subject: [PATCH 2/7] =?UTF-8?q?fix(extractor):=20green=20=E2=80=94=20#278?= =?UTF-8?q?=20a=20`-=3D`=20releases=20only=20in=20an=20unguarded=20teardow?= =?UTF-8?q?n=20context?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Honour P-001/P-004 as written: a matching `target -= handler` credits the subscription's release ONLY when it is proven to run at the subscriber's end-of-life. The `unsub` collector now requires both: * a recognised TEARDOWN CONTEXT — Dispose/DisposeAsync/OnClosed/OnClosing/ OnUnloaded/OnFormClosed/OnFormClosing by name, a finalizer, a handler wired (`+=`, bare/`this.` receiver) to the class's OWN Closed/Closing/Unloaded/ FormClosed/FormClosing/Disposed lifecycle event (inline lambda handlers included), the XAML-wiring `*_Closed`/`*_Closing`/`*_Unloaded`/... naming convention, or any method such a context calls directly on `this` (intra-class fixpoint — deliberately NO whole-program call graph); * no parameter guard — a `-=` under a branch whose condition depends on a parameter of its enclosing method cannot be proven to run from the subscription site (SectorTS: `if (!UnregOnlyGoodys)`, callers pass true). The one canonical exception is a POSITIVE `if (disposing)` in `Dispose(bool)`; `if (!disposing)` still demotes. A `-=` in an arbitrary method, a ctor, or behind a caller-controlled flag now keeps the honest OWN001/OWN014 instead of silently swallowing the leak class — the #238 doctrine. Self-detaching handlers, old->new rotation and the timer `.Stop()` release are untouched. OwnIR schema, the Python core, and the S0/S2 fix pipeline are unchanged (the `--fix-candidates` teardown metadata keeps its own candidate scan by design). Evidence (docs/notes/own278-corpus-diff.md): corpus benchmark 40/44 -> 42/46 caught, 46/46 fixes clean, 0 FPs, every pre-existing row byte-identical; the samples diff flips exactly InpcAmbiguousTeardown + HandlerReassignedField (golden regenerated per tests/goldens/README.md, byte-parity gate passes); ScreenToGif sweep +4 findings, all one triaged shape (release only in a custom-named Destroy()); CsvHelper and the oracle push-target fixture byte-identical. The SectorTS reduction now flags GTD and PGC while KDT stays flagged and a Dispose-releasing sibling stays silent. Closes #278 acceptance rules 1-3; the call-graph reachability rule stays out of this slice by design. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01MdYFKUaygHz1T9H1qJ7BqK --- docs/notes/own278-corpus-diff.md | 85 ++++++ docs/notes/subscription-leaks-and-profiles.md | 13 + frontend/roslyn/OwnSharp.Extractor/Program.cs | 249 +++++++++++++++++- tests/goldens/fix_candidates_off.golden.json | 4 +- 4 files changed, 344 insertions(+), 7 deletions(-) create mode 100644 docs/notes/own278-corpus-diff.md diff --git a/docs/notes/own278-corpus-diff.md b/docs/notes/own278-corpus-diff.md new file mode 100644 index 00000000..97785de3 --- /dev/null +++ b/docs/notes/own278-corpus-diff.md @@ -0,0 +1,85 @@ +# #278 teardown-scoped release — before/after corpus + sweep evidence + +The soundness fix (a matching `-=` credits release only in a recognised teardown +context and never under a parameter guard) was measured against every surface +available on the dev runner. Baseline = frozen `366bbf93` (the G50 acceptance +commit), after = this branch. All scans used the same runner, same .NET 8 SDK; +the ScreenToGif rows additionally load the WindowsDesktop reference pack +(`OWN_EXTRA_REF_DIRS`, as `oracle.yml` does) so framework events resolve. + +## Corpus benchmark (`scripts/benchmark.py`) + +| | before (366bbf93) | after (#278) | +|---|---|---| +| bugs caught | 40/44 | **42/46** | +| fixes clean | 44/44 | **46/46** | +| false positives on fixes | 0 | **0** | + +Row-level diff: every pre-existing row byte-identical; the only delta is the two +new cases (`subscription-param-guarded-unregister`, +`subscription-nonteardown-release`), both `before[caught: OWN001] after[clean]`. +The 4 pre-existing MISSED rows (WPF-unresolvable on a Linux runner without the +ref pack) are unchanged. The delta is strictly additive — new true positives, +nothing silenced. + +## Extractor samples (`frontend/roslyn/samples`, whole-dir own-check diff) + +157 → 159 findings. The only two new rows, both intended semantics: + +- `FixCandidatesSample.cs:45` (`InpcAmbiguousTeardown`) — its two `-=` live in + `Detach1()`/`Detach2()`, arbitrary non-teardown methods → now OWN001. The + `--fix-candidates` teardown metadata (`ambiguous`, 2 candidates) is unchanged. +- `FixCandidatesSample.cs:254` (`HandlerReassignedField`) — its `-=` lives in + the ctor → now OWN001. + +Everything else — rotation silences, self-detach, `OrdersViewModel`/ +`CleanStaticEventViewModel` Dispose releases, `Window_Closing` (XAML-wired) +releases — byte-identical. `tests/goldens/fix_candidates_off.golden.json` was +regenerated for the two flipped `released` values (procedure per +`tests/goldens/README.md`); `check_fix_candidates_facts.py` and the byte-parity +gate pass. + +## Oracle push-target fixture (`corpus/fixtures/systemevents-console`) + +Byte-identical before/after (3 findings, 1 OWN050 advisory). + +## Real-repo sweep + +| target | before | after | delta | +|---|---|---|---| +| ScreenToGif (mine-target), no ref pack | 214 | 214 | none | +| ScreenToGif, WindowsDesktop ref pack | 61 (20 findings, 40 OWN050) | 65 (24 findings, 40 OWN050) | +4, classified below | +| CsvHelper `src/` | 0 | 0 | none | + +### Classification of the 4 new ScreenToGif findings + +All four are ONE shape: `ScreenToGif/Controls/ResizingAdorner.cs` subscribes its +injected `_adornedElement` to `PreviewMouseLeftButtonDown`/`MouseMove`/`MouseUp` +in the ctor (`:91-93`, plus the re-attach half of a suspend/resume `-=`/`+=` +inside the `MouseMove` handler, `:130`). The only unconditional detach lives in +a **custom-named** teardown, `public void Destroy()` (`:502-506`), which the +owning windows call when removing the adorner. + +Triage: the release is real but *not provable from the class alone* — `Destroy` +is an ordinary method the owner must remember to call (exactly the shape the +GTD leak had, minus the parameter guard; if an owner forgets `Destroy`, the +element pins the adorner). Under the #238/#278 doctrine this demotes to a kept +warning — a *mitigation candidate*, never silence. No baseline entry is added: +ScreenToGif is the miner's spot-check target (reviewed in run logs), not the +cross-tool oracle target, and the warning tier is the intended verdict for an +unproven custom teardown. If a future slice recognises "a `-=`-only method +called by all constructing owners" (needs the caller walk that is explicitly +out of this slice's budget), these four are the first candidates. + +## SectorTS acceptance (GTD / PGC / KDT shape) + +The real `STS_new/SectorTS` tree is not present on this runner; the faithful +reduction (static `AppData.Properties.GBProperty.PropertyChanged` chain, +`GTD` = flag-guarded `-=` in `UnregisterEventHandlers(bool)`, `PGC` = +unconditional `-=` in `UnregisterEventHandlers()`, `KDT` = no `-=`, +`CleanDoc` = `-=` in `Dispose`) was verified end-to-end: GTD, PGC and KDT are +flagged OWN001; CleanDoc is silent. Re-running +`OwnAudit/sts_audit` against the real tree (and the ClosedXML / 5-repo sweep) +remains a local, pre-merge step — together with the two merge gates from the +scope: the G50 Acceptance Run on frozen `366bbf93`, and the OwnAudit STS +baseline that classifies GTD as `runtime-only`. diff --git a/docs/notes/subscription-leaks-and-profiles.md b/docs/notes/subscription-leaks-and-profiles.md index ed941d1c..4bafbbc0 100644 --- a/docs/notes/subscription-leaks-and-profiles.md +++ b/docs/notes/subscription-leaks-and-profiles.md @@ -35,6 +35,19 @@ WPF005 strong capture by a longer-lived source -> OWN014 (region promotion) So the core stays neutral; "WPF" is *recognition + lifetime context*, not the error itself. The critique is correct, and we mostly already shipped it. +Since #278, "matching `-=`" is teardown-scoped, as the WPF001 row above always +said: the `-=` must sit in a recognised teardown context (`Dispose`/ +`DisposeAsync`/`OnClosed`/`Unloaded`-style methods, a finalizer, a handler wired +to the class's own `Closed`/`Closing`/`Unloaded`-style lifecycle event, or a +method the teardown path calls intra-class) and must not be guarded by a +parameter of its enclosing method (the canonical positive `if (disposing)` of +`Dispose(bool)` excepted). A `-=` in an arbitrary method, or behind a +caller-controlled flag, is not proven to run and keeps the honest OWN001/OWN014 +— the #238 doctrine: the worst case of an exemption must be "keeps today's +honest warning", never "silently swallows a leak class" (heap-proven on +SectorTS `GTD`, corpus: `subscription-param-guarded-unregister`, +`subscription-nonteardown-release`). + ## The naming debt the critique correctly smells The capability is general. The *same* `source.Event += h` without `-=` leaks in diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 83d4133e..b397b6b7 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -11,9 +11,14 @@ // When the left side's declaring type is an unresolved external reference we do // not guess: a handler-shaped RHS surfaces as an OWN050 "leakage analysis // skipped" note, never a leak. A subscription is "released" by a matching -// `target -= handler` in the class; a `Tick`/`Elapsed` handler is tagged -// resource=timer (WPF002) and is released if the timer's receiver also has a -// `.Stop()` call. The IDisposable/pool/local detectors remain syntactic for now +// `target -= handler` in a recognised TEARDOWN CONTEXT of the class (#278: +// Dispose/DisposeAsync/OnClosed/Unloaded-style methods, a finalizer, a handler +// wired to the class's own Closed/Unloaded-style lifecycle event, or a method +// the teardown path calls intra-class) that is not guarded by a parameter of +// its enclosing method — a `-=` in an arbitrary method, or behind a +// caller-controlled flag, is not proven to run and keeps the honest warning. +// A `Tick`/`Elapsed` handler is tagged resource=timer (WPF002) and is released +// if the timer's receiver also has a `.Stop()` call. The IDisposable/pool/local detectors remain syntactic for now // (P-014 rollout: the event fact goes type-aware first). // // Usage: ownsharp-extract [extract] [more ...] [-o|--out facts.json] @@ -810,6 +815,230 @@ static bool IsTimerEvent(ExpressionSyntax left) => left is MemberAccessExpressionSyntax m && (m.Name.Identifier.Text == "Tick" || m.Name.Identifier.Text == "Elapsed"); +// --- #278 soundness: a `-=` that exists is not a `-=` that runs. ----------------- +// +// A matching `-=` credits the subscription's release ONLY when it sits in a +// recognised TEARDOWN CONTEXT of the subscribing class (P-001/P-004 as written: +// `Dispose`/`OnClosed`/`Unloaded`), and is not guarded away by a parameter of its +// enclosing method. A `-=` in an arbitrary method — one nothing here proves is +// ever called, or whose guard the caller can flip — keeps today's honest OWN001 +// warning instead of silently swallowing the leak class (the #238 doctrine). +// Heap-proven on SectorTS GTD: ctor `+=` to a static publisher, the only `-=` +// inside `UnregisterEventHandlers(bool UnregOnlyGoodys)` behind `if (!UnregOnly- +// Goodys)`, and the leaking callers pass `true` — the old "any `-=` in the class" +// model paired them and stayed silent over a 66%-retained heap. +// +// Teardown contexts (deliberately NO whole-program call graph — intra-class only): +// * a method the platform itself runs at end-of-life, by exact name: +// Dispose / DisposeAsync / OnClosed / OnClosing / OnUnloaded / OnFormClosed / +// OnFormClosing, or a finalizer; +// * a handler this class wires (`+=`, bare/`this.` receiver) to its own +// lifecycle event: Closed / Closing / Unloaded / FormClosed / FormClosing / +// Disposed — including an inline lambda handler on those events; +// * the XAML-wiring naming convention `*_Closed` / `*_Closing` / `*_Unloaded` / +// `*_FormClosed` / `*_FormClosing` / `*_Disposed` (a XAML `Closing="Window_ +// Closing"` attach never reaches this extractor, but the generated handler +// name does — corpus: screentogif-loaded-subscription); +// * any method such a context calls DIRECTLY on `this` (`Cleanup()` from +// `Dispose()`), transitively within the class — the "method the type's own +// disposal path calls" rule, closed intra-class only. + +// A method name the platform (or the IDisposable contract) itself invokes at the +// end of the object's life, or the conventional XAML-wired lifecycle handler name. +static bool IsTeardownMethodName(string name) => + name is "Dispose" or "DisposeAsync" + or "OnClosed" or "OnClosing" or "OnUnloaded" or "OnFormClosed" or "OnFormClosing" + || name.EndsWith("_Closed", StringComparison.Ordinal) + || name.EndsWith("_Closing", StringComparison.Ordinal) + || name.EndsWith("_Unloaded", StringComparison.Ordinal) + || name.EndsWith("_FormClosed", StringComparison.Ordinal) + || name.EndsWith("_FormClosing", StringComparison.Ordinal) + || name.EndsWith("_Disposed", StringComparison.Ordinal); + +// An event MEMBER name that fires at the subscriber's own end-of-life, so a handler +// attached to it is a teardown context. +static bool IsTeardownEventName(string name) => + name is "Closed" or "Closing" or "Unloaded" or "FormClosed" or "FormClosing" or "Disposed"; + +// The event-member simple name of a `+=`/`-=` left side (`Closed`, `win.Closed`), +// null when the shape names no member. +static string? EventMemberName(ExpressionSyntax left) => left switch +{ + IdentifierNameSyntax id => id.Identifier.Text, + MemberAccessExpressionSyntax ma => ma.Name.Identifier.Text, + _ => null, +}; + +// Is the `+=` receiver THIS object (bare `Closed +=` / `this.Closed +=`)? A handler +// hooked to ANOTHER object's `Closed` runs at that object's teardown, not ours, so +// it may not ground OUR release. +static bool IsSelfLifecycleReceiver(ExpressionSyntax left) => + left is IdentifierNameSyntax + || (left is MemberAccessExpressionSyntax ma && ma.Expression is ThisExpressionSyntax); + +// The set of THIS class's method names that are teardown contexts: the named/wired +// roots above plus everything they call directly on `this`, to a fixpoint. Keyed by +// simple name (overloads conflate — the same conservative-toward-keeping-the-pair +// granularity as the text-keyed `unsub` set itself; a same-named helper is at worst +// credited like its sibling, never silently dropped). +static HashSet TeardownContextMethods(ClassDeclarationSyntax cls) +{ + var own = new Dictionary>(StringComparer.Ordinal); + foreach (var md in cls.Members.OfType()) + { + if (!own.TryGetValue(md.Identifier.Text, out var list)) + own[md.Identifier.Text] = list = new List(); + list.Add(md); + } + + var teardown = new HashSet(StringComparer.Ordinal); + var work = new Queue(); + void Root(string name) + { + if (own.ContainsKey(name) && teardown.Add(name)) + work.Enqueue(name); + } + foreach (var name in own.Keys) + if (IsTeardownMethodName(name)) + Root(name); + // handlers this class wires to its OWN lifecycle events (`Closed += OnDone;`). + foreach (var a in cls.DescendantNodes().OfType()) + if (a.IsKind(SyntaxKind.AddAssignmentExpression) + && EventMemberName(a.Left) is { } evName && IsTeardownEventName(evName) + && IsSelfLifecycleReceiver(a.Left)) + { + var h = NormalizeHandler(a.Right); + if (h is IdentifierNameSyntax hid) + Root(hid.Identifier.Text); + else if (h is MemberAccessExpressionSyntax { Expression: ThisExpressionSyntax } hma) + Root(hma.Name.Identifier.Text); + } + // intra-class closure: a method a teardown context calls on `this` is itself a + // teardown context ("a method the type's own disposal path calls"). + while (work.Count > 0) + foreach (var md in own[work.Dequeue()]) + { + SyntaxNode? body = (SyntaxNode?)md.Body ?? md.ExpressionBody; + if (body is null) + continue; + foreach (var inv in body.DescendantNodes().OfType()) + { + var callee = inv.Expression switch + { + IdentifierNameSyntax id => id.Identifier.Text, + MemberAccessExpressionSyntax { Expression: ThisExpressionSyntax } ma => ma.Name.Identifier.Text, + _ => null, + }; + if (callee is not null) + Root(callee); + } + } + return teardown; +} + +// Does this `-=` sit in a teardown context of `cls`? Walks the lexical ancestors: +// a finalizer or a teardown-set method of THIS class => yes; a ctor, an accessor +// (a rebinding setter is the #163 gap, not a proven teardown), or a non-teardown +// method => no. An anonymous function resolves to the event it is attached to when +// it IS a `+=` handler (only a lifecycle event counts); a plain inline lambda +// (`ForEach(x => x.E -= H)` inside Dispose) falls through to its lexical context. +static bool InTeardownContext(AssignmentExpressionSyntax sub, ClassDeclarationSyntax cls, + HashSet teardownMethods) +{ + for (SyntaxNode? cur = sub.Parent; cur is not null; cur = cur.Parent) + { + switch (cur) + { + case AnonymousFunctionExpressionSyntax lam: + // `Closed += (s, e) => { ... -= ... }` — the lambda runs at teardown. + // Unwrap `new EventHandler(...)`-style wrappers by walking parents to + // the assignment whose (normalized) RHS is this very lambda. + if (lam.FirstAncestorOrSelf() is { } attach + && attach.IsKind(SyntaxKind.AddAssignmentExpression) + && ReferenceEquals(NormalizeHandler(attach.Right), lam)) + return EventMemberName(attach.Left) is { } evName + && IsTeardownEventName(evName) + && IsSelfLifecycleReceiver(attach.Left); + continue; // not a handler: inherit the lexical context + case LocalFunctionStatementSyntax: + continue; // part of its declaring method's body + case DestructorDeclarationSyntax: + return true; + case MethodDeclarationSyntax md: + return ReferenceEquals(md.Parent, cls) + && teardownMethods.Contains(md.Identifier.Text); + case BaseMethodDeclarationSyntax: // ctor / operator: not a teardown + return false; + case AccessorDeclarationSyntax: + return false; + case TypeDeclarationSyntax: + return false; + } + } + return false; +} + +// #278 rule 2: a `-=` under a branch whose condition depends on a PARAMETER of the +// enclosing method cannot be proven to run from the subscription site — the caller +// chooses (SectorTS: `if (!UnregOnlyGoodys)`, callers pass `true`). Field/local +// guards (`if (_handler != null)`) stay credited: they are the class's own state, +// not a caller-controlled skip. The ONE canonical exception is the positive +// `disposing` guard of the IDisposable pattern — `Dispose(bool disposing)`'s +// `if (disposing) { ... }` runs on every `Dispose()` call, so a POSITIVE use of +// that single bool parameter does not demote; `if (!disposing)` (the finalizer +// branch) still does. +static bool IsParamGuardedRelease(AssignmentExpressionSyntax sub, SemanticModel model) +{ + for (SyntaxNode? cur = sub.Parent; cur is not null && cur is not MemberDeclarationSyntax; cur = cur.Parent) + { + ExpressionSyntax? cond = cur switch + { + IfStatementSyntax ifs => ifs.Condition, + ConditionalExpressionSyntax ce => ce.Condition, + SwitchStatementSyntax ss => ss.Expression, + SwitchExpressionSyntax se => se.GoverningExpression, + WhileStatementSyntax ws => ws.Condition, + ForStatementSyntax fs => fs.Condition, + _ => null, + }; + if (cond is null) + continue; + foreach (var id in cond.DescendantNodesAndSelf().OfType()) + if (model.GetSymbolInfo(id).Symbol is IParameterSymbol p + && !IsCanonicalDisposingGuardUse(id, p)) + return true; + } + return false; +} + +// A POSITIVE reference to the single bool parameter of `Dispose(bool)` — the +// canonical `if (disposing)` (also `if (disposing && !_disposed)`). A negated use +// (`!disposing`, `disposing == false`) selects the finalizer branch and does not +// prove the managed release runs. +static bool IsCanonicalDisposingGuardUse(IdentifierNameSyntax id, IParameterSymbol p) +{ + if (p.Type.SpecialType != SpecialType.System_Boolean + || p.ContainingSymbol is not IMethodSymbol m + || m.Name != "Dispose" || m.Parameters.Length != 1) + return false; + SyntaxNode use = id; + while (use.Parent is ParenthesizedExpressionSyntax pe) + use = pe; + if (use.Parent is PrefixUnaryExpressionSyntax pu && pu.IsKind(SyntaxKind.LogicalNotExpression)) + return false; + if (use.Parent is BinaryExpressionSyntax be + && (be.IsKind(SyntaxKind.EqualsExpression) || be.IsKind(SyntaxKind.NotEqualsExpression))) + { + var other = ReferenceEquals(be.Left, use) ? be.Right : be.Left; + var negated = be.IsKind(SyntaxKind.EqualsExpression) + ? other.IsKind(SyntaxKind.FalseLiteralExpression) + : other.IsKind(SyntaxKind.TrueLiteralExpression); + if (negated) + return false; + } + return true; +} + // Issue #218 — DP/property-changed old->new subscription ROTATION. A property-changed callback (a // `PropertyChangedCallback` reading `e.OldValue`/`e.NewValue`, or a virtual `OnXChanged(T old, T new)` // override) detaches the SAME handler from the OLD value and re-attaches it to the NEW one across a @@ -4878,11 +5107,21 @@ static bool IsPublicCtor(SyntaxTokenList modifiers) { var assigns = cls.DescendantNodes().OfType().ToList(); - // every `target -= handler` in this class, keyed by "left|right". + // every `target -= handler` in this class that is PROVEN to run at teardown, + // keyed by "left|right". #278 soundness: a `-=` in an arbitrary method (or + // behind a caller-controlled parameter guard) is not a release — it kept + // pairing SectorTS GTD's ctor `+=` with a flag-skipped `-=` and silently + // swallowed a heap-proven leak. Only a teardown-context, unguarded `-=` + // credits release now; everything else keeps the honest OWN001/OWN014. + // (Self-detaching handlers, old->new rotation and timer `.Stop()` have + // their own dedicated checks below, unchanged.) + var teardownMethods = TeardownContextMethods(cls); var unsub = new HashSet(); foreach (var a in assigns) if (a.IsKind(SyntaxKind.SubtractAssignmentExpression) - && IsHandler(NormalizeHandler(a.Right))) + && IsHandler(NormalizeHandler(a.Right)) + && InTeardownContext(a, cls, teardownMethods) + && !IsParamGuardedRelease(a, model)) unsub.Add($"{a.Left}|{NormalizeHandler(a.Right)}"); // every receiver with a `.Stop()` call: a timer detached this way counts diff --git a/tests/goldens/fix_candidates_off.golden.json b/tests/goldens/fix_candidates_off.golden.json index 39cfe1a1..01129f11 100644 --- a/tests/goldens/fix_candidates_off.golden.json +++ b/tests/goldens/fix_candidates_off.golden.json @@ -40,7 +40,7 @@ "event": "_pub.PropertyChanged", "handler": "OnChanged", "line": 45, - "released": true, + "released": false, "resource": "subscription", "source": "injected", "lambda": false @@ -310,7 +310,7 @@ "event": "_pub.PropertyChanged", "handler": "_handler", "line": 254, - "released": true, + "released": false, "resource": "subscription", "source": "injected", "lambda": false From dbbf759058a81143cf40f86c2bf805ec4e46ed07 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 13:59:22 +0000 Subject: [PATCH 3/7] =?UTF-8?q?test(corpus):=20red=20=E2=80=94=20#278=20fo?= =?UTF-8?q?llow-up:=20four=20teardown=20paths=20that=20credit=20an=20unpro?= =?UTF-8?q?ven=20release?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four corpus/wpf cases pin the silent-exemption holes the #278 review found in the first slice's teardown model. Each before.cs is SILENT under that slice (verified against its extractor) and must be OWN001: * subscription-finalizer-release — the only `-=` in the finalizer. Circularly unreachable: the publisher's delegate keeps the subscriber reachable, so the finalizer never runs while the subscription is live. * subscription-xaml-name-only-release — the only `-=` in a method NAMED `Window_Closing` that nothing in code wires. A name is not wiring; a bare handler-shaped name may be stale dead code. after.cs pins both wired good forms (method group + inline lambda on `this.Closing`). * subscription-overload-conflated-cleanup — Dispose calls `Cleanup()`; the `-=` lives only in the uncalled `Cleanup(bool)`. A name-keyed closure conflates the overloads. * subscription-uncalled-local-function — the `-=` in a local function (and a lambda) DECLARED inside Dispose but never invoked. Declaration is not execution. after.cs pins the called-local-function good form. The previously name-carried control screentogif-loaded-subscription/after.cs now wires `Closing += Window_Closing` in code — the honest, provable form of the same fix (the real ScreenToGif attaches it in XAML, which the extractor never sees); the name-only shape moves to the new bad case. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01MdYFKUaygHz1T9H1qJ7BqK --- .../screentogif-loaded-subscription/after.cs | 11 +++- .../screentogif-loaded-subscription/notes.md | 9 ++++ .../subscription-finalizer-release/after.cs | 26 ++++++++++ .../subscription-finalizer-release/before.cs | 32 ++++++++++++ .../subscription-finalizer-release/case.own | 20 +++++++ .../expected-diagnostics.txt | 1 + .../subscription-finalizer-release/notes.md | 30 +++++++++++ .../after.cs | 32 ++++++++++++ .../before.cs | 36 +++++++++++++ .../case.own | 20 +++++++ .../expected-diagnostics.txt | 1 + .../notes.md | 32 ++++++++++++ .../after.cs | 32 ++++++++++++ .../before.cs | 52 +++++++++++++++++++ .../case.own | 19 +++++++ .../expected-diagnostics.txt | 1 + .../notes.md | 37 +++++++++++++ .../after.cs | 48 +++++++++++++++++ .../before.cs | 32 ++++++++++++ .../case.own | 20 +++++++ .../expected-diagnostics.txt | 1 + .../notes.md | 37 +++++++++++++ 22 files changed, 527 insertions(+), 2 deletions(-) create mode 100644 corpus/wpf/subscription-finalizer-release/after.cs create mode 100644 corpus/wpf/subscription-finalizer-release/before.cs create mode 100644 corpus/wpf/subscription-finalizer-release/case.own create mode 100644 corpus/wpf/subscription-finalizer-release/expected-diagnostics.txt create mode 100644 corpus/wpf/subscription-finalizer-release/notes.md create mode 100644 corpus/wpf/subscription-overload-conflated-cleanup/after.cs create mode 100644 corpus/wpf/subscription-overload-conflated-cleanup/before.cs create mode 100644 corpus/wpf/subscription-overload-conflated-cleanup/case.own create mode 100644 corpus/wpf/subscription-overload-conflated-cleanup/expected-diagnostics.txt create mode 100644 corpus/wpf/subscription-overload-conflated-cleanup/notes.md create mode 100644 corpus/wpf/subscription-uncalled-local-function/after.cs create mode 100644 corpus/wpf/subscription-uncalled-local-function/before.cs create mode 100644 corpus/wpf/subscription-uncalled-local-function/case.own create mode 100644 corpus/wpf/subscription-uncalled-local-function/expected-diagnostics.txt create mode 100644 corpus/wpf/subscription-uncalled-local-function/notes.md create mode 100644 corpus/wpf/subscription-xaml-name-only-release/after.cs create mode 100644 corpus/wpf/subscription-xaml-name-only-release/before.cs create mode 100644 corpus/wpf/subscription-xaml-name-only-release/case.own create mode 100644 corpus/wpf/subscription-xaml-name-only-release/expected-diagnostics.txt create mode 100644 corpus/wpf/subscription-xaml-name-only-release/notes.md diff --git a/corpus/real-world/screentogif-loaded-subscription/after.cs b/corpus/real-world/screentogif-loaded-subscription/after.cs index d7389c1b..48fea235 100644 --- a/corpus/real-world/screentogif-loaded-subscription/after.cs +++ b/corpus/real-world/screentogif-loaded-subscription/after.cs @@ -1,6 +1,12 @@ // Fix: subscribe with named handlers (so they have a `-=` handle) and detach them -// in Window_Closing. The view-model no longer roots the window, and a repeated -// Loaded no longer stacks duplicate handlers. +// in Window_Closing, wired IN CODE (`Closing += Window_Closing`). The view-model +// no longer roots the window, and a repeated Loaded no longer stacks duplicate +// handlers. +// +// The code wiring is load-bearing (#278 follow-up): a `Window_Closing`-style NAME +// alone proves nothing to the extractor (the XAML attach never reaches it, and a +// bare name may be stale dead code), so the release is credited only because the +// ctor provably attaches the handler to the window's own Closing event. using System; using System.Windows; @@ -12,6 +18,7 @@ public VideoSource() { InitializeComponent(); _viewModel = DataContext as VideoSourceViewModel; + Closing += Window_Closing; } private void Window_Loaded(object sender, RoutedEventArgs e) diff --git a/corpus/real-world/screentogif-loaded-subscription/notes.md b/corpus/real-world/screentogif-loaded-subscription/notes.md index 1a38deb1..2571ce6d 100644 --- a/corpus/real-world/screentogif-loaded-subscription/notes.md +++ b/corpus/real-world/screentogif-loaded-subscription/notes.md @@ -38,3 +38,12 @@ produces the core's domain-neutral **OWN001** (the severity tiering lives in the extractor, above the core). As with the rest of the corpus, `case.own` is a hand reduction of the C# pattern, not verbatim extractor output; `before.cs` / `after.cs` are representative of the leak and its fix. + +**#278 follow-up.** `after.cs` wires `Closing += Window_Closing` in the ctor. The +real ScreenToGif attaches the handler in XAML, which the extractor never sees — and +since #278's follow-up a `Window_Closing`-style *name* alone is NOT a teardown +context (a bare name may be stale dead code; the name-suffix exemption was a +silent-FN hole). The corpus case therefore carries the wiring in code, which is the +honest, provable form of the same fix; the name-only shape is pinned as a BAD case +in `corpus/wpf/subscription-xaml-name-only-release`. A future XAML-aware slice can +credit the XAML attach with actual evidence. diff --git a/corpus/wpf/subscription-finalizer-release/after.cs b/corpus/wpf/subscription-finalizer-release/after.cs new file mode 100644 index 00000000..8299b26e --- /dev/null +++ b/corpus/wpf/subscription-finalizer-release/after.cs @@ -0,0 +1,26 @@ +// FIXED. The release moved to Dispose — a deterministic teardown the OWNER +// calls, which does not depend on the object first becoming unreachable. (A +// finalizer may still exist for unmanaged state; it just cannot be the +// subscription's release path.) +// +// own-check MUST treat this as released (silent). +using System; +using System.ComponentModel; + +public sealed class FinalizerDetachDocument : IDisposable +{ + private readonly INotifyPropertyChanged _properties; // injected, unknown lifetime + + public FinalizerDetachDocument(INotifyPropertyChanged properties) + { + _properties = properties; + _properties.PropertyChanged += OnPropertiesChanged; + } + + public void Dispose() + { + _properties.PropertyChanged -= OnPropertiesChanged; // deterministic teardown + } + + private void OnPropertiesChanged(object sender, PropertyChangedEventArgs e) { /* ... */ } +} diff --git a/corpus/wpf/subscription-finalizer-release/before.cs b/corpus/wpf/subscription-finalizer-release/before.cs new file mode 100644 index 00000000..3d03e2c9 --- /dev/null +++ b/corpus/wpf/subscription-finalizer-release/before.cs @@ -0,0 +1,32 @@ +// BUGGY (#278 follow-up, blocker 1; hand-reduced into case.own). +// +// The ctor subscribes to an injected publisher; the only matching `-=` sits in +// the FINALIZER. That release can never run while it matters: the publisher's +// delegate holds a strong reference to this object, so as long as the +// subscription is live the subscriber is REACHABLE and the GC never finalizes +// it. The `-=` exists precisely on the one path that the leak itself blocks. +// (For a static/process-lived publisher the finalizer is simply never reached +// for the life of the process — same argument, absolute.) +// +// own-check MUST flag this OWN001. Crediting the finalizer was a silent +// false-negative path in the first #278 slice. +using System.ComponentModel; + +public sealed class FinalizerDetachDocument +{ + private readonly INotifyPropertyChanged _properties; // injected, unknown lifetime + + public FinalizerDetachDocument(INotifyPropertyChanged properties) + { + _properties = properties; + _properties.PropertyChanged += OnPropertiesChanged; + } + + ~FinalizerDetachDocument() + { + // unreachable while subscribed: the delegate keeps `this` alive + _properties.PropertyChanged -= OnPropertiesChanged; + } + + private void OnPropertiesChanged(object sender, PropertyChangedEventArgs e) { /* ... */ } +} diff --git a/corpus/wpf/subscription-finalizer-release/case.own b/corpus/wpf/subscription-finalizer-release/case.own new file mode 100644 index 00000000..44a430c2 --- /dev/null +++ b/corpus/wpf/subscription-finalizer-release/case.own @@ -0,0 +1,20 @@ +module WpfSubscriptionFinalizerRelease + +// A subscription token: `+= handler` acquires the source<->listener edge; the +// matching `-= handler` releases it. `kind` tags the resource so the generic +// ownership finding carries a [resource: ...] note. +resource Subscription { + acquire Subscribe + release Unsubscribe + kind "subscription token" +} + +// The ctor subscribes; the only `-=` lives in the finalizer. A finalizer runs +// only after the object becomes unreachable — but the live subscription is +// exactly what keeps it reachable, so the release path is blocked by the leak +// it is supposed to fix. Modelled as the ctor scope with no release => OWN001. +fn FinalizerDetachDocument(properties: int) { + let sub = acquire Subscription(properties); + // the finalizer `-=` is not modelled as a release: it cannot run while + // the subscription holds `this` reachable (before.cs) +} diff --git a/corpus/wpf/subscription-finalizer-release/expected-diagnostics.txt b/corpus/wpf/subscription-finalizer-release/expected-diagnostics.txt new file mode 100644 index 00000000..ed2a1929 --- /dev/null +++ b/corpus/wpf/subscription-finalizer-release/expected-diagnostics.txt @@ -0,0 +1 @@ +OWN001 diff --git a/corpus/wpf/subscription-finalizer-release/notes.md b/corpus/wpf/subscription-finalizer-release/notes.md new file mode 100644 index 00000000..69c2375a --- /dev/null +++ b/corpus/wpf/subscription-finalizer-release/notes.md @@ -0,0 +1,30 @@ +# Subscription whose only `-=` is in the finalizer (soundness FN) + +**Pattern (#278 follow-up, blocker 1).** Ctor `+=` to an injected publisher; the +matching `-=` sits in `~FinalizerDetachDocument()`. The release is circularly +unreachable: a finalizer runs only after the object becomes unreachable, but the +publisher's delegate (the subscription) is precisely what keeps the subscriber +reachable. While the subscription is live the finalizer cannot run; once the +finalizer can run, there is nothing left to release. For a static/process-lived +publisher the same argument is absolute — the finalizer is never reached for the +life of the process. + +**The bug.** The first #278 slice treated `DestructorDeclarationSyntax` as a +teardown context, so this shape was silently credited as released — a +false-negative path of exactly the kind the slice existed to remove. + +**The fix.** A finalizer is explicitly NOT a teardown context for subscription +release. `before.cs` keeps the honest OWN001; `after.cs` releases in `Dispose` +(deterministic, owner-called, does not depend on unreachability) and is silent. + +**What the checker says (`.own` reduction).** The ctor scope acquires the token +and no reachable teardown path releases it => **OWN001** with the +subscription-token resource tag. + +**Regression guard.** `scripts/benchmark.py`: `before.cs` must be **caught**, +`after.cs` must be **silent**. + +**Honesty / scope.** `case.own` carries the acquire/release logic; the +finalizer-reachability reasoning lives in the extractor +(`InTeardownContext`: `DestructorDeclarationSyntax => false`). Timer `.Stop()` +release and the rest of the teardown model are unchanged by this case. diff --git a/corpus/wpf/subscription-overload-conflated-cleanup/after.cs b/corpus/wpf/subscription-overload-conflated-cleanup/after.cs new file mode 100644 index 00000000..934577f5 --- /dev/null +++ b/corpus/wpf/subscription-overload-conflated-cleanup/after.cs @@ -0,0 +1,32 @@ +// FIXED. The overload Dispose actually calls now holds the `-=`; the +// symbol-based teardown closure resolves `Dispose() -> Cleanup()` and credits +// exactly that overload. +// +// own-check MUST treat this as released (silent). +using System; +using System.ComponentModel; + +public sealed class ReportView : IDisposable +{ + private readonly INotifyPropertyChanged _report; // injected, unknown lifetime + + public ReportView(INotifyPropertyChanged report) + { + _report = report; + _report.PropertyChanged += OnReportChanged; + } + + public void Dispose() => Cleanup(); + + private void Cleanup() + { + _report.PropertyChanged -= OnReportChanged; // on the resolved teardown path + } + + private void Cleanup(bool detachHandlers) + { + // the uncalled overload no longer carries the only release + } + + private void OnReportChanged(object sender, PropertyChangedEventArgs e) { /* ... */ } +} diff --git a/corpus/wpf/subscription-overload-conflated-cleanup/before.cs b/corpus/wpf/subscription-overload-conflated-cleanup/before.cs new file mode 100644 index 00000000..1a0462eb --- /dev/null +++ b/corpus/wpf/subscription-overload-conflated-cleanup/before.cs @@ -0,0 +1,36 @@ +// BUGGY (#278 follow-up, blocker 3; hand-reduced into case.own). +// +// Dispose calls `Cleanup()` — the no-argument overload, which detaches +// nothing. The matching `-=` lives only in `Cleanup(bool)`, an overload that +// NOTHING on the teardown path calls. A name-keyed teardown closure conflates +// the two ("Dispose calls Cleanup, Cleanup has the -=") and silently credits +// the release; only symbol-resolved call targets keep the two apart. +// +// own-check MUST flag this OWN001. +using System; +using System.ComponentModel; + +public sealed class ReportView : IDisposable +{ + private readonly INotifyPropertyChanged _report; // injected, unknown lifetime + + public ReportView(INotifyPropertyChanged report) + { + _report = report; + _report.PropertyChanged += OnReportChanged; + } + + public void Dispose() => Cleanup(); // resolves to Cleanup(), not Cleanup(bool) + + private void Cleanup() + { + // releases buffers etc. — but detaches nothing + } + + private void Cleanup(bool detachHandlers) + { + _report.PropertyChanged -= OnReportChanged; // never called from any teardown + } + + private void OnReportChanged(object sender, PropertyChangedEventArgs e) { /* ... */ } +} diff --git a/corpus/wpf/subscription-overload-conflated-cleanup/case.own b/corpus/wpf/subscription-overload-conflated-cleanup/case.own new file mode 100644 index 00000000..e7d8fac9 --- /dev/null +++ b/corpus/wpf/subscription-overload-conflated-cleanup/case.own @@ -0,0 +1,20 @@ +module WpfSubscriptionOverloadConflatedCleanup + +// A subscription token: `+= handler` acquires the source<->listener edge; the +// matching `-= handler` releases it. `kind` tags the resource so the generic +// ownership finding carries a [resource: ...] note. +resource Subscription { + acquire Subscribe + release Unsubscribe + kind "subscription token" +} + +// The ctor subscribes; Dispose calls the Cleanup() overload that detaches +// nothing, while the `-=` lives only in the never-called Cleanup(bool). +// Modelled as the ctor+Dispose scope where the called path performs no +// release => OWN001. +fn ReportView(report: int) { + let sub = acquire Subscription(report); + // Dispose() -> Cleanup(): no release on the called overload; the + // Cleanup(bool) holding the `-=` is never called (before.cs) +} diff --git a/corpus/wpf/subscription-overload-conflated-cleanup/expected-diagnostics.txt b/corpus/wpf/subscription-overload-conflated-cleanup/expected-diagnostics.txt new file mode 100644 index 00000000..ed2a1929 --- /dev/null +++ b/corpus/wpf/subscription-overload-conflated-cleanup/expected-diagnostics.txt @@ -0,0 +1 @@ +OWN001 diff --git a/corpus/wpf/subscription-overload-conflated-cleanup/notes.md b/corpus/wpf/subscription-overload-conflated-cleanup/notes.md new file mode 100644 index 00000000..31b01e9a --- /dev/null +++ b/corpus/wpf/subscription-overload-conflated-cleanup/notes.md @@ -0,0 +1,32 @@ +# Subscription whose only `-=` is in an uncalled overload of a teardown helper (soundness FN) + +**Pattern (#278 follow-up, blocker 3).** `Dispose()` calls `Cleanup()` — the +no-argument overload, which detaches nothing. The matching `-=` lives only in +`Cleanup(bool)`, which no teardown path (and nothing else in the class) calls. + +**The bug.** The first #278 slice keyed its intra-class teardown closure by +SIMPLE METHOD NAME: "Dispose calls *Cleanup*" marked every method named +`Cleanup` as a teardown context, so the `-=` inside the uncalled `Cleanup(bool)` +was silently credited as released — overload conflation as a silent-exemption +path. + +**The fix.** The closure is SYMBOL-based (`IMethodSymbol` + +`SymbolEqualityComparer`): an invocation extends the teardown set only with the +specific method it RESOLVES to. `Dispose() => Cleanup();` credits exactly +`Cleanup()`; `Cleanup(bool)` stays outside the set and its `-=` grounds +nothing. An invocation that fails to resolve extends nothing — the worst case +stays "keeps today's honest warning". `before.cs` is OWN001; `after.cs` moves +the `-=` into the overload Dispose actually calls and is silent. + +(The method-GROUP name fallback used for wiring handlers to an unresolved +lifecycle event — see `subscription-xaml-name-only-release/notes.md` — is +distinct by construction: a method group carries no argument list, so its name +denotes the whole overload set; an invocation selects exactly one target and +must be resolved to it.) + +**What the checker says (`.own` reduction).** The scope acquires the token and +the called teardown path performs no release => **OWN001** with the +subscription-token resource tag. + +**Regression guard.** `scripts/benchmark.py`: `before.cs` must be **caught**, +`after.cs` must be **silent**. diff --git a/corpus/wpf/subscription-uncalled-local-function/after.cs b/corpus/wpf/subscription-uncalled-local-function/after.cs new file mode 100644 index 00000000..23bd620a --- /dev/null +++ b/corpus/wpf/subscription-uncalled-local-function/after.cs @@ -0,0 +1,32 @@ +// FIXED. Dispose now actually CALLS the local function holding the `-=`; the +// symbol-based teardown closure proves `Dispose() -> Detach()` and credits the +// release. (The wired-lambda good form lives in +// subscription-xaml-name-only-release/after.cs — a lambda counts only as the +// handler wired to a lifecycle event or when provably invoked.) +// +// own-check MUST treat this as released (silent). +using System; +using System.ComponentModel; + +public sealed class UncalledLocalFunctionView : IDisposable +{ + private readonly INotifyPropertyChanged _model; // injected, unknown lifetime + + public UncalledLocalFunctionView(INotifyPropertyChanged model) + { + _model = model; + _model.PropertyChanged += OnModelChanged; + } + + public void Dispose() + { + Detach(); // the call is the proof + + void Detach() + { + _model.PropertyChanged -= OnModelChanged; + } + } + + private void OnModelChanged(object sender, PropertyChangedEventArgs e) { /* ... */ } +} diff --git a/corpus/wpf/subscription-uncalled-local-function/before.cs b/corpus/wpf/subscription-uncalled-local-function/before.cs new file mode 100644 index 00000000..2e129b6b --- /dev/null +++ b/corpus/wpf/subscription-uncalled-local-function/before.cs @@ -0,0 +1,52 @@ +// BUGGY (#278 follow-up, blocker 4; hand-reduced into case.own). +// +// Both classes place the matching `-=` inside a callable DECLARED lexically +// inside Dispose — a local function in one, a lambda in the other — that +// Dispose never invokes. Declaration is not execution: a nested callable does +// not run just because its enclosing method does. Lexically inheriting the +// teardown context was a silent false-negative path in the first #278 slice. +// +// own-check MUST flag both OWN001. +using System; +using System.ComponentModel; + +public sealed class UncalledLocalFunctionView : IDisposable +{ + private readonly INotifyPropertyChanged _model; // injected, unknown lifetime + + public UncalledLocalFunctionView(INotifyPropertyChanged model) + { + _model = model; + _model.PropertyChanged += OnModelChanged; + } + + public void Dispose() + { + void Detach() + { + _model.PropertyChanged -= OnModelChanged; + } + // Detach() is never invoked — dead teardown code + } + + private void OnModelChanged(object sender, PropertyChangedEventArgs e) { /* ... */ } +} + +public sealed class UncalledLambdaView : IDisposable +{ + private readonly INotifyPropertyChanged _model; // injected, unknown lifetime + + public UncalledLambdaView(INotifyPropertyChanged model) + { + _model = model; + _model.PropertyChanged += OnModelChanged; + } + + public void Dispose() + { + Action detach = () => _model.PropertyChanged -= OnModelChanged; + // detach is never invoked — the delegate is created and dropped + } + + private void OnModelChanged(object sender, PropertyChangedEventArgs e) { /* ... */ } +} diff --git a/corpus/wpf/subscription-uncalled-local-function/case.own b/corpus/wpf/subscription-uncalled-local-function/case.own new file mode 100644 index 00000000..aad6e5ad --- /dev/null +++ b/corpus/wpf/subscription-uncalled-local-function/case.own @@ -0,0 +1,19 @@ +module WpfSubscriptionUncalledLocalFunction + +// A subscription token: `+= handler` acquires the source<->listener edge; the +// matching `-= handler` releases it. `kind` tags the resource so the generic +// ownership finding carries a [resource: ...] note. +resource Subscription { + acquire Subscribe + release Unsubscribe + kind "subscription token" +} + +// The ctor subscribes; the only `-=` sits in a nested callable (local +// function / lambda) that Dispose declares but never invokes. Declaration is +// not execution, so the teardown path performs no release => OWN001. +fn UncalledLocalFunctionView(model: int) { + let sub = acquire Subscription(model); + // Dispose declares Detach() but never calls it: no release on any + // executed path (before.cs) +} diff --git a/corpus/wpf/subscription-uncalled-local-function/expected-diagnostics.txt b/corpus/wpf/subscription-uncalled-local-function/expected-diagnostics.txt new file mode 100644 index 00000000..ed2a1929 --- /dev/null +++ b/corpus/wpf/subscription-uncalled-local-function/expected-diagnostics.txt @@ -0,0 +1 @@ +OWN001 diff --git a/corpus/wpf/subscription-uncalled-local-function/notes.md b/corpus/wpf/subscription-uncalled-local-function/notes.md new file mode 100644 index 00000000..7a90f476 --- /dev/null +++ b/corpus/wpf/subscription-uncalled-local-function/notes.md @@ -0,0 +1,37 @@ +# Subscription whose only `-=` is in an uncalled local function / lambda inside Dispose (soundness FN) + +**Pattern (#278 follow-up, blocker 4).** The matching `-=` sits inside a +callable declared lexically inside `Dispose` — a local function +(`UncalledLocalFunctionView`) or a lambda stored in a local +(`UncalledLambdaView`) — that Dispose never invokes. Declaration is not +execution: a nested callable does not run just because its enclosing method +does. + +**The bug.** The first #278 slice treated both as lexical pass-throughs — a +local function "part of its declaring method's body", a non-handler lambda +inheriting its lexical context — so a `-=` inside dead nested teardown code was +silently credited as released. + +**The fix.** +- A LOCAL FUNCTION is a teardown context only when the symbol-based intra-class + closure proves a teardown context CALLS it (`Dispose() { Detach(); void + Detach() { ... } }` — the `after.cs` shape). The closure walks each + callable's own body only (never descending into nested function bodies), so + an invocation inside an uncalled nested function cannot extend the set + either. +- A LAMBDA is a teardown context only as the handler provably wired to the + class's own lifecycle event (`this.Closing += (s, e) => ... -= ...`, pinned + in `subscription-xaml-name-only-release/after.cs`). Everything else — + including a lambda stored in a local, passed to a combinator + (`ForEach(x => x.E -= H)`), or simply declared and dropped — keeps the + honest warning: nothing intra-class proves the delegate is invoked. + +`before.cs` pins both bad forms (expected OWN001); `after.cs` pins the +called-local-function good form (silent). + +**What the checker says (`.own` reduction).** The scope acquires the token and +no executed path releases it => **OWN001** with the subscription-token resource +tag. + +**Regression guard.** `scripts/benchmark.py`: `before.cs` must be **caught**, +`after.cs` must be **silent**. diff --git a/corpus/wpf/subscription-xaml-name-only-release/after.cs b/corpus/wpf/subscription-xaml-name-only-release/after.cs new file mode 100644 index 00000000..292c674a --- /dev/null +++ b/corpus/wpf/subscription-xaml-name-only-release/after.cs @@ -0,0 +1,48 @@ +// FIXED. The SAME handler name — but now the ctor provably wires it to the +// view's own Closing lifecycle event (`this.Closing += Window_Closing`), so +// the `-=` inside it runs at teardown. A second class pins the inline-lambda +// form of the same wiring. +// +// own-check MUST treat both as released (silent): the release is credited by +// the code wiring, never by the name. +using System; +using System.ComponentModel; + +public sealed class SettingsView +{ + private readonly INotifyPropertyChanged _settings; // injected, unknown lifetime + + public event EventHandler Closing; // raised by the host at teardown + + public SettingsView(INotifyPropertyChanged settings) + { + _settings = settings; + _settings.PropertyChanged += OnSettingsChanged; + this.Closing += Window_Closing; // the wiring is the proof + } + + private void Window_Closing(object sender, EventArgs e) + { + _settings.PropertyChanged -= OnSettingsChanged; + } + + private void OnSettingsChanged(object sender, PropertyChangedEventArgs e) { /* ... */ } +} + +// The inline-lambda handler on the same lifecycle event — equally wired, +// equally silent. +public sealed class SettingsPane +{ + private readonly INotifyPropertyChanged _settings; // injected, unknown lifetime + + public event EventHandler Closing; // raised by the host at teardown + + public SettingsPane(INotifyPropertyChanged settings) + { + _settings = settings; + _settings.PropertyChanged += OnSettingsChanged; + this.Closing += (s, e) => _settings.PropertyChanged -= OnSettingsChanged; + } + + private void OnSettingsChanged(object sender, PropertyChangedEventArgs e) { /* ... */ } +} diff --git a/corpus/wpf/subscription-xaml-name-only-release/before.cs b/corpus/wpf/subscription-xaml-name-only-release/before.cs new file mode 100644 index 00000000..59c85488 --- /dev/null +++ b/corpus/wpf/subscription-xaml-name-only-release/before.cs @@ -0,0 +1,32 @@ +// BUGGY (#278 follow-up, blocker 2; hand-reduced into case.own). +// +// The ctor subscribes to an injected publisher; the matching `-=` sits in a +// method NAMED like a XAML-wired lifecycle handler (`Window_Closing`) — but +// NOTHING in code attaches it to any event. The name alone proves nothing: +// XAML attaches never reach the extractor, and a handler-shaped name with no +// wiring may equally be stale dead code left behind after the XAML attribute +// was removed. Crediting the naming convention was a silent false-negative +// path in the first #278 slice. +// +// own-check MUST flag this OWN001; a XAML-aware slice may later credit a REAL +// XAML attach with actual evidence. +using System; +using System.ComponentModel; + +public sealed class SettingsView +{ + private readonly INotifyPropertyChanged _settings; // injected, unknown lifetime + + public SettingsView(INotifyPropertyChanged settings) + { + _settings = settings; + _settings.PropertyChanged += OnSettingsChanged; + } + + private void Window_Closing(object sender, EventArgs e) + { + _settings.PropertyChanged -= OnSettingsChanged; // nothing wires this handler + } + + private void OnSettingsChanged(object sender, PropertyChangedEventArgs e) { /* ... */ } +} diff --git a/corpus/wpf/subscription-xaml-name-only-release/case.own b/corpus/wpf/subscription-xaml-name-only-release/case.own new file mode 100644 index 00000000..edb5c38c --- /dev/null +++ b/corpus/wpf/subscription-xaml-name-only-release/case.own @@ -0,0 +1,20 @@ +module WpfSubscriptionXamlNameOnlyRelease + +// A subscription token: `+= handler` acquires the source<->listener edge; the +// matching `-= handler` releases it. `kind` tags the resource so the generic +// ownership finding carries a [resource: ...] note. +resource Subscription { + acquire Subscribe + release Unsubscribe + kind "subscription token" +} + +// The ctor subscribes; the only `-=` lives in a method whose NAME matches the +// XAML handler convention but which nothing in code wires to any event. A name +// is not wiring — the method is unreachable as far as the code can prove. +// Modelled as the ctor scope with no release => OWN001. +fn SettingsView(settings: int) { + let sub = acquire Subscription(settings); + // the name-only `Window_Closing` is not modelled as a release: nothing + // proves the platform ever calls it (before.cs) +} diff --git a/corpus/wpf/subscription-xaml-name-only-release/expected-diagnostics.txt b/corpus/wpf/subscription-xaml-name-only-release/expected-diagnostics.txt new file mode 100644 index 00000000..ed2a1929 --- /dev/null +++ b/corpus/wpf/subscription-xaml-name-only-release/expected-diagnostics.txt @@ -0,0 +1 @@ +OWN001 diff --git a/corpus/wpf/subscription-xaml-name-only-release/notes.md b/corpus/wpf/subscription-xaml-name-only-release/notes.md new file mode 100644 index 00000000..e9625f0e --- /dev/null +++ b/corpus/wpf/subscription-xaml-name-only-release/notes.md @@ -0,0 +1,37 @@ +# Subscription whose only `-=` is in a XAML-convention-NAMED but unwired handler (soundness FN) + +**Pattern (#278 follow-up, blocker 2).** Ctor `+=` to an injected publisher; the +matching `-=` sits in `Window_Closing(object, EventArgs)` — a method named +exactly like a XAML-wired lifecycle handler — but nothing in code attaches it +to any event. The XAML attach (if one ever existed) never reaches the +extractor; a bare handler-shaped name may equally be stale dead code after the +XAML attribute was removed. The name proves nothing about execution. + +**The bug.** The first #278 slice recognised the `*_Closed`/`*_Closing`/ +`*_Unloaded`/... naming convention as a teardown root, so this shape was +silently credited as released — a name-only silent-exemption path. + +**The fix.** The suffix rule is removed entirely. A `Window_Closing`-style +handler is a teardown context only when the class provably wires it in code +(`this.Closing += Window_Closing`, or an inline lambda on the same event). The +previously name-carried corpus control +(`corpus/real-world/screentogif-loaded-subscription/after.cs`) now carries the +wiring in code — the honest form of the same fix. XAML-backed release without +code wiring is deliberately left as a kept warning until a XAML-aware slice can +credit the attach with actual evidence. + +**`after.cs` pins both wired forms**: the method-group handler +(`this.Closing += Window_Closing`, `-=` inside the handler) and the +inline-lambda handler on the same lifecycle event. Both silent. Note the wiring +recognition works even when the lifecycle event itself cannot be resolved (a +WPF `Window.Closing` on a Linux runner): a method GROUP carries no argument +list, so falling back to the group's name selects the same overload set the +group denotes — distinct from the invocation-overload conflation ruled out in +`subscription-overload-conflated-cleanup`. + +**What the checker says (`.own` reduction).** The ctor scope acquires the token +and no provable teardown path releases it => **OWN001** with the +subscription-token resource tag. + +**Regression guard.** `scripts/benchmark.py`: `before.cs` must be **caught**, +`after.cs` must be **silent**. From 4d11ec4f534420e55a42972b3bac79ca7e797cf3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 13:59:39 +0000 Subject: [PATCH 4/7] =?UTF-8?q?fix(extractor):=20green=20=E2=80=94=20#278?= =?UTF-8?q?=20follow-up:=20close=20the=20four=20unproven-release=20teardow?= =?UTF-8?q?n=20paths?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. A FINALIZER is no longer a teardown context (InTeardownContext: DestructorDeclarationSyntax => false). For a subscription leak the publisher's delegate keeps the subscriber reachable, so the finalizer never runs while the subscription is live — its `-=` can never break the hold. 2. The `*_Closed`/`*_Closing`/`*_Unloaded`/... NAME-SUFFIX exemption is removed. A XAML attach never reaches the extractor, so a name alone proves nothing (it may be stale dead code). A `Window_Closing`-style handler counts only when the class provably wires it in code; XAML-backed release stays a kept warning until a XAML-aware slice can credit the attach with evidence. 3. The intra-class teardown closure is SYMBOL-based (IMethodSymbol + SymbolEqualityComparer): an invocation extends the set only with the specific own method/local function it RESOLVES to, so `Dispose() => Cleanup();` credits exactly `Cleanup()` — never an uncalled `Cleanup(bool)` overload. Unresolved calls extend nothing. One narrow name fallback stays, for method-GROUP handlers wired to an UNRESOLVED lifecycle event (`Closing +=` under an unreferenced WPF Window base): a method group carries no argument list, so its name denotes the whole overload set — not the invocation-overload conflation above. 4. Nested callables no longer inherit their lexical teardown context. A local function counts only when the symbol closure proves a teardown CALLS it; a lambda only as the handler wired to a lifecycle event. The closure walks each callable's own body (never descending into nested function bodies), so an invocation inside an uncalled nested function extends nothing either. Timer .Stop(), rotation, self-detach, OwnIR schema, the core, and the fix pipeline are unchanged. Evidence (docs/notes/own278-corpus-diff.md, follow-up section): corpus benchmark 46/50 caught, 50/50 fixes clean, 0 FPs, all pre-existing rows unchanged; samples output byte-identical to slice 1; golden untouched; full suite/ruff/mypy green. 5-repo sweep: CsvHelper, Dapper, Newtonsoft.Json, RestSharp identical; ScreenToGif +9, all one triaged shape — real `-=` in `*_Closing` handlers wired ONLY in XAML, the deliberate rule-2 kept-warning trade-off and the first candidates for the XAML-aware slice. SectorTS reduction unchanged: GTD/PGC/KDT flagged, Dispose sibling silent. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01MdYFKUaygHz1T9H1qJ7BqK --- docs/notes/own278-corpus-diff.md | 79 ++++++ docs/notes/subscription-leaks-and-profiles.md | 26 +- frontend/roslyn/OwnSharp.Extractor/Program.cs | 225 +++++++++++------- 3 files changed, 233 insertions(+), 97 deletions(-) diff --git a/docs/notes/own278-corpus-diff.md b/docs/notes/own278-corpus-diff.md index 97785de3..4f692a1e 100644 --- a/docs/notes/own278-corpus-diff.md +++ b/docs/notes/own278-corpus-diff.md @@ -1,5 +1,8 @@ # #278 teardown-scoped release — before/after corpus + sweep evidence +> **Follow-up slice appended below** ("Follow-up: four silent-exemption paths +> removed") — it supersedes the first slice's numbers where they differ. + The soundness fix (a matching `-=` credits release only in a recognised teardown context and never under a parameter guard) was measured against every surface available on the dev runner. Baseline = frozen `366bbf93` (the G50 acceptance @@ -83,3 +86,79 @@ flagged OWN001; CleanDoc is silent. Re-running remains a local, pre-merge step — together with the two merge gates from the scope: the G50 Acceptance Run on frozen `366bbf93`, and the OwnAudit STS baseline that classifies GTD as `runtime-only`. + +--- + +# Follow-up: four silent-exemption paths removed + +The review of the first slice found four remaining paths that credited a +release without proving it runs. All four are closed; each is red→green pinned +by a corpus case. Baseline for this section = the first slice's head +(`fix(extractor): green — #278 ...`), so the deltas isolate the follow-up. + +| # | removed path | why it was unsound | corpus pin | +|---|---|---|---| +| 1 | finalizer as teardown | the publisher's delegate keeps the subscriber REACHABLE, so the finalizer never runs while the subscription is live | `subscription-finalizer-release` | +| 2 | `*_Closed`/`*_Closing`/`*_Unloaded`/... name suffix | a name is not wiring — the XAML attach never reaches the extractor and a bare name may be stale dead code | `subscription-xaml-name-only-release` | +| 3 | name-keyed intra-class closure | `Dispose() -> Cleanup()` credited every method NAMED `Cleanup`, including an uncalled `Cleanup(bool)` overload | `subscription-overload-conflated-cleanup` | +| 4 | lexical inheritance for nested callables | a local function or lambda declared inside Dispose does not run because Dispose does | `subscription-uncalled-local-function` | + +The closure is now SYMBOL-based (`IMethodSymbol` + `SymbolEqualityComparer`): +an invocation extends the teardown set only with the specific own method or +local function it RESOLVES to; unresolved calls extend nothing. A lambda counts +only as the handler provably wired to the class's own lifecycle event; a local +function only when a teardown context provably calls it. One narrow name +fallback remains, for method-GROUP handlers wired to an UNRESOLVED lifecycle +event (`Closing += Window_Closing` under an unreferenced WPF `Window` base): a +method group carries no argument list, so its name denotes the whole overload +set — not the invocation-overload conflation of #3. + +## Corpus benchmark + +40/44 (pre-#278) → 42/46 (slice 1) → **46/50 caught · 50/50 fixes clean · +0 FPs**. All pre-existing rows unchanged; the four new rows are +`before[caught: OWN001] after[clean]`. The previously name-carried control +`screentogif-loaded-subscription/after.cs` now wires `Closing += +Window_Closing` in code (the honest, provable form of the same fix) and stays +clean. + +## Samples / goldens / suite + +`frontend/roslyn/samples` own-check output: **byte-identical** to the first +slice (no sample used any of the four removed paths). +`fix_candidates_off.golden.json`: unchanged, byte-parity gate passes; +`check_fix_candidates_facts.py`, weak-subscribe checks, full +`tests/run_tests.py`, ruff and mypy all green. + +## 5-repo sweep (before = slice 1, after = follow-up) + +| target | before | after | delta | +|---|---|---|---| +| ScreenToGif (WindowsDesktop ref pack) | 65 | 74 | +9, classified below | +| CsvHelper | 37 | 37 | identical | +| Dapper | 6 | 6 | identical | +| Newtonsoft.Json | 509 | 509 | identical | +| RestSharp | 3 | 3 | identical | + +### Classification of the 9 new ScreenToGif findings + +All nine are ONE shape — the deliberate blocker-2 trade-off. Five windows +(`Editor`, `Recorder`, `NewRecorder`, `Webcam`, `Other/Startup`) subscribe +static `SystemEvents.*`/`SystemParameters.*` events and detach them in a +`Window_Closing`/`Startup_Closing` handler that is wired **only in XAML** +(`Closing="Window_Closing"`, e.g. `Editor.xaml:17`) — the attach never reaches +the extractor, so the name-suffix rule was the only thing crediting these, and +that rule is exactly the unsound path removed (verified: the `-=` sites are +real, e.g. `Editor.xaml.cs:375-377`, `Startup.xaml.cs:66`). They surface as +OWN014 (static-source region escape with no provable release path) — a kept +honest warning, not silence, per the doctrine. These are the first candidates +for a future XAML-aware slice that credits `Closing="..."` attaches with actual +evidence; until then the corpus keeps the code-wired form as the good control +and pins the name-only form as bad. + +## SectorTS acceptance re-check + +Unchanged by the follow-up: the reduction still flags GTD, PGC and KDT +(OWN001) and keeps the Dispose-releasing sibling silent. The real +`STS_new/SectorTS` run and the OwnAudit STS baseline (GTD = `runtime-only`) +remain the two pre-merge gates, executed locally. diff --git a/docs/notes/subscription-leaks-and-profiles.md b/docs/notes/subscription-leaks-and-profiles.md index 4bafbbc0..2d5207cc 100644 --- a/docs/notes/subscription-leaks-and-profiles.md +++ b/docs/notes/subscription-leaks-and-profiles.md @@ -37,16 +37,24 @@ error itself. The critique is correct, and we mostly already shipped it. Since #278, "matching `-=`" is teardown-scoped, as the WPF001 row above always said: the `-=` must sit in a recognised teardown context (`Dispose`/ -`DisposeAsync`/`OnClosed`/`Unloaded`-style methods, a finalizer, a handler wired -to the class's own `Closed`/`Closing`/`Unloaded`-style lifecycle event, or a -method the teardown path calls intra-class) and must not be guarded by a -parameter of its enclosing method (the canonical positive `if (disposing)` of -`Dispose(bool)` excepted). A `-=` in an arbitrary method, or behind a -caller-controlled flag, is not proven to run and keeps the honest OWN001/OWN014 -— the #238 doctrine: the worst case of an exemption must be "keeps today's +`DisposeAsync`/`OnClosed`/`Unloaded`-style methods, a handler wired IN CODE to +the class's own `Closed`/`Closing`/`Unloaded`-style lifecycle event, or a +method/local function the teardown path provably calls intra-class — a +SYMBOL-resolved fixpoint, so `Cleanup()` never credits an uncalled +`Cleanup(bool)` overload) and must not be guarded by a parameter of its +enclosing method (the canonical positive `if (disposing)` of `Dispose(bool)` +excepted). Explicitly NOT teardown contexts: an arbitrary method, a +caller-controlled flag guard, a FINALIZER (the publisher's delegate keeps the +subscriber reachable, so it never runs while the subscription is live), a +`Window_Closing`-style name with no code wiring (XAML attaches never reach the +extractor; the bare name may be stale dead code), an unwired lambda, and an +uncalled local function — each keeps the honest OWN001/OWN014. The #238 +doctrine throughout: the worst case of an exemption must be "keeps today's honest warning", never "silently swallows a leak class" (heap-proven on -SectorTS `GTD`, corpus: `subscription-param-guarded-unregister`, -`subscription-nonteardown-release`). +SectorTS `GTD`; corpus: `subscription-param-guarded-unregister`, +`subscription-nonteardown-release`, `subscription-finalizer-release`, +`subscription-xaml-name-only-release`, `subscription-overload-conflated-cleanup`, +`subscription-uncalled-local-function`). ## The naming debt the critique correctly smells diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index b397b6b7..859fa16b 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -12,11 +12,13 @@ // not guess: a handler-shaped RHS surfaces as an OWN050 "leakage analysis // skipped" note, never a leak. A subscription is "released" by a matching // `target -= handler` in a recognised TEARDOWN CONTEXT of the class (#278: -// Dispose/DisposeAsync/OnClosed/Unloaded-style methods, a finalizer, a handler -// wired to the class's own Closed/Unloaded-style lifecycle event, or a method -// the teardown path calls intra-class) that is not guarded by a parameter of -// its enclosing method — a `-=` in an arbitrary method, or behind a -// caller-controlled flag, is not proven to run and keeps the honest warning. +// Dispose/DisposeAsync/OnClosed/Unloaded-style methods, a handler wired in code +// to the class's own Closed/Unloaded-style lifecycle event, or a method/local +// function the teardown path provably calls intra-class, symbol-resolved) that +// is not guarded by a parameter of its enclosing method — a `-=` in an +// arbitrary method, a finalizer, an unwired lambda, an uncalled local function, +// or behind a caller-controlled flag, is not proven to run and keeps the honest +// warning. // A `Tick`/`Elapsed` handler is tagged resource=timer (WPF002) and is released // if the timer's receiver also has a `.Stop()` call. The IDisposable/pool/local detectors remain syntactic for now // (P-014 rollout: the event fact goes type-aware first). @@ -831,29 +833,33 @@ left is MemberAccessExpressionSyntax m // Teardown contexts (deliberately NO whole-program call graph — intra-class only): // * a method the platform itself runs at end-of-life, by exact name: // Dispose / DisposeAsync / OnClosed / OnClosing / OnUnloaded / OnFormClosed / -// OnFormClosing, or a finalizer; +// OnFormClosing; // * a handler this class wires (`+=`, bare/`this.` receiver) to its own // lifecycle event: Closed / Closing / Unloaded / FormClosed / FormClosing / // Disposed — including an inline lambda handler on those events; -// * the XAML-wiring naming convention `*_Closed` / `*_Closing` / `*_Unloaded` / -// `*_FormClosed` / `*_FormClosing` / `*_Disposed` (a XAML `Closing="Window_ -// Closing"` attach never reaches this extractor, but the generated handler -// name does — corpus: screentogif-loaded-subscription); -// * any method such a context calls DIRECTLY on `this` (`Cleanup()` from -// `Dispose()`), transitively within the class — the "method the type's own -// disposal path calls" rule, closed intra-class only. +// * any method or local function such a context PROVABLY calls (`Cleanup()` +// from `Dispose()`), transitively within the class — a SYMBOL-based +// fixpoint: only an invocation that RESOLVES to a specific own method +// extends the set, so a call to `Cleanup()` never credits an uncalled +// `Cleanup(bool)` overload. +// +// Explicit NON-contexts (follow-up to #278 — each was a silent-exemption hole): +// * a FINALIZER: for a subscription leak the subscriber stays REACHABLE +// through the publisher's delegate, so its finalizer never runs while the +// subscription is live — a `-=` there can never break the hold; +// * a `Window_Closing`-style NAME with no code wiring: the XAML attach never +// reaches this extractor, so the name alone proves nothing (it may be stale +// dead code) — a XAML-aware slice can restore it with evidence; +// * a local function or lambda merely DECLARED inside a teardown method: +// declaration is not execution — a local function counts only when the +// symbol-based closure proves a teardown calls it, a lambda only when it is +// the handler wired to a lifecycle event. // A method name the platform (or the IDisposable contract) itself invokes at the -// end of the object's life, or the conventional XAML-wired lifecycle handler name. +// end of the object's life. static bool IsTeardownMethodName(string name) => name is "Dispose" or "DisposeAsync" - or "OnClosed" or "OnClosing" or "OnUnloaded" or "OnFormClosed" or "OnFormClosing" - || name.EndsWith("_Closed", StringComparison.Ordinal) - || name.EndsWith("_Closing", StringComparison.Ordinal) - || name.EndsWith("_Unloaded", StringComparison.Ordinal) - || name.EndsWith("_FormClosed", StringComparison.Ordinal) - || name.EndsWith("_FormClosing", StringComparison.Ordinal) - || name.EndsWith("_Disposed", StringComparison.Ordinal); + or "OnClosed" or "OnClosing" or "OnUnloaded" or "OnFormClosed" or "OnFormClosing"; // An event MEMBER name that fires at the subscriber's own end-of-life, so a handler // attached to it is a teardown context. @@ -876,97 +882,140 @@ static bool IsSelfLifecycleReceiver(ExpressionSyntax left) => left is IdentifierNameSyntax || (left is MemberAccessExpressionSyntax ma && ma.Expression is ThisExpressionSyntax); -// The set of THIS class's method names that are teardown contexts: the named/wired -// roots above plus everything they call directly on `this`, to a fixpoint. Keyed by -// simple name (overloads conflate — the same conservative-toward-keeping-the-pair -// granularity as the text-keyed `unsub` set itself; a same-named helper is at worst -// credited like its sibling, never silently dropped). -static HashSet TeardownContextMethods(ClassDeclarationSyntax cls) -{ - var own = new Dictionary>(StringComparer.Ordinal); - foreach (var md in cls.Members.OfType()) +// The descendant nodes of one callable's OWN body — never descending into a +// nested lambda or local function, whose bodies do not run just because the +// enclosing method does (they get their own teardown decision). +static IEnumerable DirectBodyNodes(SyntaxNode body) => + body.DescendantNodes(n => + n is not AnonymousFunctionExpressionSyntax and not LocalFunctionStatementSyntax); + +// The set of THIS class's method (and local-function) SYMBOLS that are teardown +// contexts: the named/wired roots plus everything a member of the set PROVABLY +// calls, to a fixpoint. Symbol-based on purpose (#278 follow-up): the closure +// extends only through an invocation that RESOLVES to a specific own method or +// local function, so `Dispose() => Cleanup();` credits exactly `Cleanup()` — +// never a same-named, uncalled `Cleanup(bool)` overload. +static HashSet TeardownContextMethods( + ClassDeclarationSyntax cls, SemanticModel model, INamedTypeSymbol? clsSymbol) +{ + var teardown = new HashSet(SymbolEqualityComparer.Default); + var work = new Queue(); + void Add(IMethodSymbol? m) { - if (!own.TryGetValue(md.Identifier.Text, out var list)) - own[md.Identifier.Text] = list = new List(); - list.Add(md); + if (m is not null && teardown.Add(m)) + work.Enqueue(m); } - var teardown = new HashSet(StringComparer.Ordinal); - var work = new Queue(); - void Root(string name) - { - if (own.ContainsKey(name) && teardown.Add(name)) - work.Enqueue(name); - } - foreach (var name in own.Keys) - if (IsTeardownMethodName(name)) - Root(name); - // handlers this class wires to its OWN lifecycle events (`Closed += OnDone;`). + // roots: exact platform teardown names among the class's own methods. + foreach (var md in cls.Members.OfType()) + if (IsTeardownMethodName(md.Identifier.Text)) + Add(model.GetDeclaredSymbol(md)); + + // roots: method-group handlers this class wires to its OWN lifecycle events + // (`Closed += OnDone;` / `this.Closing += Window_Closing;`). foreach (var a in cls.DescendantNodes().OfType()) - if (a.IsKind(SyntaxKind.AddAssignmentExpression) - && EventMemberName(a.Left) is { } evName && IsTeardownEventName(evName) - && IsSelfLifecycleReceiver(a.Left)) + { + if (!a.IsKind(SyntaxKind.AddAssignmentExpression) + || EventMemberName(a.Left) is not { } evName || !IsTeardownEventName(evName) + || !IsSelfLifecycleReceiver(a.Left)) + continue; + var h = NormalizeHandler(a.Right); + if (h is not (IdentifierNameSyntax or MemberAccessExpressionSyntax { Expression: ThisExpressionSyntax })) + continue; + var info = model.GetSymbolInfo(h); + var bound = false; + if (info.Symbol is IMethodSymbol hm) { Add(hm); bound = true; } + else + foreach (var cand in info.CandidateSymbols.OfType()) { Add(cand); bound = true; } + if (!bound) { - var h = NormalizeHandler(a.Right); - if (h is IdentifierNameSyntax hid) - Root(hid.Identifier.Text); - else if (h is MemberAccessExpressionSyntax { Expression: ThisExpressionSyntax } hma) - Root(hma.Name.Identifier.Text); + // The lifecycle EVENT is unresolved (`Closing +=` on a base the runner + // cannot reference, e.g. WPF `Window` on Linux), so the method group + // binds no symbol. Fall back to the group's NAME over the class's own + // methods: a method group carries no argument list, so the name selects + // the same overload SET the group itself denotes — this is not the + // invocation-style overload conflation the closure below rules out. + var hn = h is IdentifierNameSyntax hid + ? hid.Identifier.Text + : ((MemberAccessExpressionSyntax)h).Name.Identifier.Text; + foreach (var md in cls.Members.OfType()) + if (md.Identifier.Text == hn) + Add(model.GetDeclaredSymbol(md)); } - // intra-class closure: a method a teardown context calls on `this` is itself a - // teardown context ("a method the type's own disposal path calls"). + } + + // intra-class closure: a method/local function a teardown context provably + // calls (bare or `this.`-qualified, RESOLVED to a symbol of this class) is + // itself a teardown context. Unresolved calls extend nothing — the worst + // case stays "keeps today's honest warning". while (work.Count > 0) - foreach (var md in own[work.Dequeue()]) + { + var m = work.Dequeue(); + foreach (var sref in m.DeclaringSyntaxReferences) { - SyntaxNode? body = (SyntaxNode?)md.Body ?? md.ExpressionBody; + if (sref.SyntaxTree != model.SyntaxTree) + continue; // a partial's other-file half: outside this model's scope + SyntaxNode? body = sref.GetSyntax() switch + { + BaseMethodDeclarationSyntax bmd => (SyntaxNode?)bmd.Body ?? bmd.ExpressionBody, + LocalFunctionStatementSyntax lf => (SyntaxNode?)lf.Body ?? lf.ExpressionBody, + _ => null, + }; if (body is null) continue; - foreach (var inv in body.DescendantNodes().OfType()) + foreach (var inv in DirectBodyNodes(body).OfType()) { - var callee = inv.Expression switch - { - IdentifierNameSyntax id => id.Identifier.Text, - MemberAccessExpressionSyntax { Expression: ThisExpressionSyntax } ma => ma.Name.Identifier.Text, - _ => null, - }; - if (callee is not null) - Root(callee); + if (inv.Expression is not (IdentifierNameSyntax + or MemberAccessExpressionSyntax { Expression: ThisExpressionSyntax })) + continue; + if (model.GetSymbolInfo(inv).Symbol is not IMethodSymbol callee) + continue; + if (callee.MethodKind == MethodKind.LocalFunction + || (clsSymbol is not null + && SymbolEqualityComparer.Default.Equals(callee.ContainingType, clsSymbol))) + Add(callee); } } + } return teardown; } -// Does this `-=` sit in a teardown context of `cls`? Walks the lexical ancestors: -// a finalizer or a teardown-set method of THIS class => yes; a ctor, an accessor -// (a rebinding setter is the #163 gap, not a proven teardown), or a non-teardown -// method => no. An anonymous function resolves to the event it is attached to when -// it IS a `+=` handler (only a lifecycle event counts); a plain inline lambda -// (`ForEach(x => x.E -= H)` inside Dispose) falls through to its lexical context. +// Does this `-=` sit in a PROVEN teardown context of `cls`? Walks the lexical +// ancestors to the nearest callable and asks whether that callable provably runs +// at teardown. A ctor, an accessor (a rebinding setter is the #163 gap, not a +// proven teardown), a non-teardown method, a FINALIZER (never runs while the +// publisher's delegate keeps the subscriber reachable), an unwired lambda, or an +// uncalled local function => no. static bool InTeardownContext(AssignmentExpressionSyntax sub, ClassDeclarationSyntax cls, - HashSet teardownMethods) + HashSet teardownMethods, SemanticModel model) { for (SyntaxNode? cur = sub.Parent; cur is not null; cur = cur.Parent) { switch (cur) { case AnonymousFunctionExpressionSyntax lam: - // `Closed += (s, e) => { ... -= ... }` — the lambda runs at teardown. - // Unwrap `new EventHandler(...)`-style wrappers by walking parents to - // the assignment whose (normalized) RHS is this very lambda. - if (lam.FirstAncestorOrSelf() is { } attach + // teardown ONLY as the handler wired to a lifecycle event + // (`Closed += (s, e) => { ... -= ... }`, unwrapping `new + // EventHandler(...)` wrappers). Anything else — including a lambda + // merely declared inside Dispose — is a deferred delegate nothing + // here proves is invoked. + return lam.FirstAncestorOrSelf() is { } attach && attach.IsKind(SyntaxKind.AddAssignmentExpression) - && ReferenceEquals(NormalizeHandler(attach.Right), lam)) - return EventMemberName(attach.Left) is { } evName - && IsTeardownEventName(evName) - && IsSelfLifecycleReceiver(attach.Left); - continue; // not a handler: inherit the lexical context - case LocalFunctionStatementSyntax: - continue; // part of its declaring method's body + && ReferenceEquals(NormalizeHandler(attach.Right), lam) + && EventMemberName(attach.Left) is { } evName + && IsTeardownEventName(evName) + && IsSelfLifecycleReceiver(attach.Left); + case LocalFunctionStatementSyntax lf: + // declaration is not execution: only a local function the + // symbol-based closure proved a teardown CALLS counts. + return model.GetDeclaredSymbol(lf) is { } lfs + && teardownMethods.Contains(lfs); case DestructorDeclarationSyntax: - return true; + return false; case MethodDeclarationSyntax md: return ReferenceEquals(md.Parent, cls) - && teardownMethods.Contains(md.Identifier.Text); + && model.GetDeclaredSymbol(md) is { } ms + && teardownMethods.Contains(ms); case BaseMethodDeclarationSyntax: // ctor / operator: not a teardown return false; case AccessorDeclarationSyntax: @@ -5115,12 +5164,13 @@ static bool IsPublicCtor(SyntaxTokenList modifiers) // credits release now; everything else keeps the honest OWN001/OWN014. // (Self-detaching handlers, old->new rotation and timer `.Stop()` have // their own dedicated checks below, unchanged.) - var teardownMethods = TeardownContextMethods(cls); + var clsSymbol = model.GetDeclaredSymbol(cls); + var teardownMethods = TeardownContextMethods(cls, model, clsSymbol); var unsub = new HashSet(); foreach (var a in assigns) if (a.IsKind(SyntaxKind.SubtractAssignmentExpression) && IsHandler(NormalizeHandler(a.Right)) - && InTeardownContext(a, cls, teardownMethods) + && InTeardownContext(a, cls, teardownMethods, model) && !IsParamGuardedRelease(a, model)) unsub.Add($"{a.Left}|{NormalizeHandler(a.Right)}"); @@ -5174,7 +5224,6 @@ or ImplicitObjectCreationExpressionSyntax // lived publisher — so it is NOT exempted, else a real leak is suppressed. // * template parts — `_part = GetTemplateChild("PART_x") as T`: a control // owns the parts of its own template (collectable part<->control cycle). - var clsSymbol = model.GetDeclaredSymbol(cls); var selfOwned = new HashSet(constructed); foreach (var arg in cls.DescendantNodes().OfType()) if ((arg.RefKindKeyword.IsKind(SyntaxKind.RefKeyword) From d8cb079a8ebb419bf2996910c1b57835c00621df Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 14:02:41 +0000 Subject: [PATCH 5/7] test(parity): regenerate CFG/diag parity fixtures for the six #278 corpus cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tests/run_tests.py went red in CI: the P-022 Rust-parity fixtures (tests/fixtures/cfg_parity.json, diag_parity.json) are generated over the whole corpus, so the six new corpus/wpf case.own files made the committed copies stale. Regenerated per the tests' own instruction (test_cfg_fixtures.py --write / test_diag_fixtures.py --write) — the diff is purely additive (the new cases' lowerings and (line, code) pairs) — and the Rust side replays them clean: cargo test passes, including full_parity_on_the_frozen_corpus, with no Rust changes. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01MdYFKUaygHz1T9H1qJ7BqK --- tests/fixtures/cfg_parity.json | 36 ++++++++++++++++++++ tests/fixtures/diag_parity.json | 60 +++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/tests/fixtures/cfg_parity.json b/tests/fixtures/cfg_parity.json index 63504c1d..df2d74d2 100644 --- a/tests/fixtures/cfg_parity.json +++ b/tests/fixtures/cfg_parity.json @@ -235,12 +235,48 @@ "cfg": "{\n \"functions\": [\n {\n \"blocks\": [\n {\n \"id\": 0,\n \"instrs\": [\n {\n \"line\": 22,\n \"op\": \"acquire\",\n \"resource\": \"Subscription\",\n \"sym\": 1\n }\n ],\n \"label\": \"entry\",\n \"succ\": []\n }\n ],\n \"entry\": 0,\n \"has_return_type\": false,\n \"name\": \"SourceView\",\n \"params\": [\n 0\n ],\n \"symbols\": [\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 21,\n \"is_param_borrow\": false,\n \"kind\": \"plain\",\n \"name\": \"source\",\n \"origin\": \"source#21\",\n \"resource_kind\": null,\n \"type_name\": \"int\"\n },\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 22,\n \"is_param_borrow\": false,\n \"kind\": \"owned\",\n \"name\": \"sub\",\n \"origin\": \"sub#22\",\n \"resource_kind\": \"subscription token\",\n \"type_name\": \"Subscription\"\n }\n ]\n }\n ],\n \"ownlang_cfg_version\": 0\n}", "diags": [] }, + { + "name": "corpus/wpf/subscription-finalizer-release/case.own", + "source": "module WpfSubscriptionFinalizerRelease\n\n// A subscription token: `+= handler` acquires the source<->listener edge; the\n// matching `-= handler` releases it. `kind` tags the resource so the generic\n// ownership finding carries a [resource: ...] note.\nresource Subscription {\n acquire Subscribe\n release Unsubscribe\n kind \"subscription token\"\n}\n\n// The ctor subscribes; the only `-=` lives in the finalizer. A finalizer runs\n// only after the object becomes unreachable — but the live subscription is\n// exactly what keeps it reachable, so the release path is blocked by the leak\n// it is supposed to fix. Modelled as the ctor scope with no release => OWN001.\nfn FinalizerDetachDocument(properties: int) {\n let sub = acquire Subscription(properties);\n // the finalizer `-=` is not modelled as a release: it cannot run while\n // the subscription holds `this` reachable (before.cs)\n}\n", + "cfg": "{\n \"functions\": [\n {\n \"blocks\": [\n {\n \"id\": 0,\n \"instrs\": [\n {\n \"line\": 17,\n \"op\": \"acquire\",\n \"resource\": \"Subscription\",\n \"sym\": 1\n }\n ],\n \"label\": \"entry\",\n \"succ\": []\n }\n ],\n \"entry\": 0,\n \"has_return_type\": false,\n \"name\": \"FinalizerDetachDocument\",\n \"params\": [\n 0\n ],\n \"symbols\": [\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 16,\n \"is_param_borrow\": false,\n \"kind\": \"plain\",\n \"name\": \"properties\",\n \"origin\": \"properties#16\",\n \"resource_kind\": null,\n \"type_name\": \"int\"\n },\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 17,\n \"is_param_borrow\": false,\n \"kind\": \"owned\",\n \"name\": \"sub\",\n \"origin\": \"sub#17\",\n \"resource_kind\": \"subscription token\",\n \"type_name\": \"Subscription\"\n }\n ]\n }\n ],\n \"ownlang_cfg_version\": 0\n}", + "diags": [] + }, + { + "name": "corpus/wpf/subscription-nonteardown-release/case.own", + "source": "module WpfSubscriptionNonTeardownRelease\n\n// A subscription token: `+= handler` acquires the source<->listener edge; the\n// matching `-= handler` releases it. `kind` tags the resource so the generic\n// ownership finding carries a [resource: ...] note.\nresource Subscription {\n acquire Subscribe\n release Unsubscribe\n kind \"subscription token\"\n}\n\n// The ctor subscribes; the only `-=` lives in an arbitrary non-teardown method\n// (`StopListening`) that no lifecycle path is proven to call. Modelled as the\n// ctor scope alone: the token is acquired and never released within any\n// teardown => OWN001. (The `-=`'s existence elsewhere is not modelled as a\n// release — that is exactly the #278 rule: existence is not execution.)\nfn PriceListener(prices: int) {\n let sub = acquire Subscription(prices);\n // no `release sub;` on any teardown path -> unreleased subscription (before.cs)\n}\n", + "cfg": "{\n \"functions\": [\n {\n \"blocks\": [\n {\n \"id\": 0,\n \"instrs\": [\n {\n \"line\": 18,\n \"op\": \"acquire\",\n \"resource\": \"Subscription\",\n \"sym\": 1\n }\n ],\n \"label\": \"entry\",\n \"succ\": []\n }\n ],\n \"entry\": 0,\n \"has_return_type\": false,\n \"name\": \"PriceListener\",\n \"params\": [\n 0\n ],\n \"symbols\": [\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 17,\n \"is_param_borrow\": false,\n \"kind\": \"plain\",\n \"name\": \"prices\",\n \"origin\": \"prices#17\",\n \"resource_kind\": null,\n \"type_name\": \"int\"\n },\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 18,\n \"is_param_borrow\": false,\n \"kind\": \"owned\",\n \"name\": \"sub\",\n \"origin\": \"sub#18\",\n \"resource_kind\": \"subscription token\",\n \"type_name\": \"Subscription\"\n }\n ]\n }\n ],\n \"ownlang_cfg_version\": 0\n}", + "diags": [] + }, + { + "name": "corpus/wpf/subscription-overload-conflated-cleanup/case.own", + "source": "module WpfSubscriptionOverloadConflatedCleanup\n\n// A subscription token: `+= handler` acquires the source<->listener edge; the\n// matching `-= handler` releases it. `kind` tags the resource so the generic\n// ownership finding carries a [resource: ...] note.\nresource Subscription {\n acquire Subscribe\n release Unsubscribe\n kind \"subscription token\"\n}\n\n// The ctor subscribes; Dispose calls the Cleanup() overload that detaches\n// nothing, while the `-=` lives only in the never-called Cleanup(bool).\n// Modelled as the ctor+Dispose scope where the called path performs no\n// release => OWN001.\nfn ReportView(report: int) {\n let sub = acquire Subscription(report);\n // Dispose() -> Cleanup(): no release on the called overload; the\n // Cleanup(bool) holding the `-=` is never called (before.cs)\n}\n", + "cfg": "{\n \"functions\": [\n {\n \"blocks\": [\n {\n \"id\": 0,\n \"instrs\": [\n {\n \"line\": 17,\n \"op\": \"acquire\",\n \"resource\": \"Subscription\",\n \"sym\": 1\n }\n ],\n \"label\": \"entry\",\n \"succ\": []\n }\n ],\n \"entry\": 0,\n \"has_return_type\": false,\n \"name\": \"ReportView\",\n \"params\": [\n 0\n ],\n \"symbols\": [\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 16,\n \"is_param_borrow\": false,\n \"kind\": \"plain\",\n \"name\": \"report\",\n \"origin\": \"report#16\",\n \"resource_kind\": null,\n \"type_name\": \"int\"\n },\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 17,\n \"is_param_borrow\": false,\n \"kind\": \"owned\",\n \"name\": \"sub\",\n \"origin\": \"sub#17\",\n \"resource_kind\": \"subscription token\",\n \"type_name\": \"Subscription\"\n }\n ]\n }\n ],\n \"ownlang_cfg_version\": 0\n}", + "diags": [] + }, + { + "name": "corpus/wpf/subscription-param-guarded-unregister/case.own", + "source": "module WpfSubscriptionParamGuardedUnregister\n\n// A subscription token: `+= handler` acquires the source<->listener edge; the\n// matching `-= handler` releases it. `kind` tags the resource so the generic\n// ownership finding carries a [resource: ...] note.\nresource Subscription {\n acquire Subscribe\n release Unsubscribe\n kind \"subscription token\"\n}\n\n// The ctor subscribes; the only release sits behind a caller-controlled flag in\n// a non-teardown method (`UnregisterEventHandlers(bool UnregOnlyGoodys)` under\n// `if (!UnregOnlyGoodys)`). Modelled as one scope: on the flag=true path the\n// release never runs, so the token is not released on all paths => OWN001.\n// The real callers DO pass true (GTDService, DocCloud) — the leak path is the\n// production path, not a corner.\nfn GoodsDocument(properties: int, unregOnlyGoodys: int) {\n let sub = acquire Subscription(properties);\n if (unregOnlyGoodys) {\n return; // caller passed true -> the `-=` block is skipped\n }\n release sub; // the guarded `-=`: runs only when the flag is false\n}\n", + "cfg": "{\n \"functions\": [\n {\n \"blocks\": [\n {\n \"id\": 0,\n \"instrs\": [\n {\n \"line\": 19,\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\": 21,\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\": 23,\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\": \"GoodsDocument\",\n \"params\": [\n 0,\n 1\n ],\n \"symbols\": [\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 18,\n \"is_param_borrow\": false,\n \"kind\": \"plain\",\n \"name\": \"properties\",\n \"origin\": \"properties#18\",\n \"resource_kind\": null,\n \"type_name\": \"int\"\n },\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 18,\n \"is_param_borrow\": false,\n \"kind\": \"plain\",\n \"name\": \"unregOnlyGoodys\",\n \"origin\": \"unregOnlyGoodys#18\",\n \"resource_kind\": null,\n \"type_name\": \"int\"\n },\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 19,\n \"is_param_borrow\": false,\n \"kind\": \"owned\",\n \"name\": \"sub\",\n \"origin\": \"sub#19\",\n \"resource_kind\": \"subscription token\",\n \"type_name\": \"Subscription\"\n }\n ]\n }\n ],\n \"ownlang_cfg_version\": 0\n}", + "diags": [] + }, { "name": "corpus/wpf/subscription-target-typed-delegate-release/case.own", "source": "module WpfSubscriptionTargetTypedDelegateRelease\n\n// A subscription token: `+= handler` acquires the source<->listener edge; the\n// matching `-= handler` releases it. `kind` tags the resource so the finding\n// carries a [resource: ...] note.\nresource Subscription {\n acquire Subscribe\n release Dispose\n kind \"subscription token\"\n}\n\n// The view modelled as one scope: the ctor subscribes to the injected source and\n// (in before.cs) never unsubscribes — no Dispose => OWN001.\n//\n// The C# after.cs releases it in Dispose, but the ctor `+=` uses C# 9 target-typed\n// delegate creation `new(H)` while the `-=` is a bare method group. That syntactic\n// asymmetry is what the extractor must normalize; this .own reduction only carries\n// the acquire/release logic the core reasons about.\nfn SourceView(source: int) {\n let sub = acquire Subscription(source);\n // no `release sub;` -> unreleased subscription (before.cs)\n}\n", "cfg": "{\n \"functions\": [\n {\n \"blocks\": [\n {\n \"id\": 0,\n \"instrs\": [\n {\n \"line\": 20,\n \"op\": \"acquire\",\n \"resource\": \"Subscription\",\n \"sym\": 1\n }\n ],\n \"label\": \"entry\",\n \"succ\": []\n }\n ],\n \"entry\": 0,\n \"has_return_type\": false,\n \"name\": \"SourceView\",\n \"params\": [\n 0\n ],\n \"symbols\": [\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 19,\n \"is_param_borrow\": false,\n \"kind\": \"plain\",\n \"name\": \"source\",\n \"origin\": \"source#19\",\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\": \"owned\",\n \"name\": \"sub\",\n \"origin\": \"sub#20\",\n \"resource_kind\": \"subscription token\",\n \"type_name\": \"Subscription\"\n }\n ]\n }\n ],\n \"ownlang_cfg_version\": 0\n}", "diags": [] }, + { + "name": "corpus/wpf/subscription-uncalled-local-function/case.own", + "source": "module WpfSubscriptionUncalledLocalFunction\n\n// A subscription token: `+= handler` acquires the source<->listener edge; the\n// matching `-= handler` releases it. `kind` tags the resource so the generic\n// ownership finding carries a [resource: ...] note.\nresource Subscription {\n acquire Subscribe\n release Unsubscribe\n kind \"subscription token\"\n}\n\n// The ctor subscribes; the only `-=` sits in a nested callable (local\n// function / lambda) that Dispose declares but never invokes. Declaration is\n// not execution, so the teardown path performs no release => OWN001.\nfn UncalledLocalFunctionView(model: int) {\n let sub = acquire Subscription(model);\n // Dispose declares Detach() but never calls it: no release on any\n // executed path (before.cs)\n}\n", + "cfg": "{\n \"functions\": [\n {\n \"blocks\": [\n {\n \"id\": 0,\n \"instrs\": [\n {\n \"line\": 16,\n \"op\": \"acquire\",\n \"resource\": \"Subscription\",\n \"sym\": 1\n }\n ],\n \"label\": \"entry\",\n \"succ\": []\n }\n ],\n \"entry\": 0,\n \"has_return_type\": false,\n \"name\": \"UncalledLocalFunctionView\",\n \"params\": [\n 0\n ],\n \"symbols\": [\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 15,\n \"is_param_borrow\": false,\n \"kind\": \"plain\",\n \"name\": \"model\",\n \"origin\": \"model#15\",\n \"resource_kind\": null,\n \"type_name\": \"int\"\n },\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 16,\n \"is_param_borrow\": false,\n \"kind\": \"owned\",\n \"name\": \"sub\",\n \"origin\": \"sub#16\",\n \"resource_kind\": \"subscription token\",\n \"type_name\": \"Subscription\"\n }\n ]\n }\n ],\n \"ownlang_cfg_version\": 0\n}", + "diags": [] + }, + { + "name": "corpus/wpf/subscription-xaml-name-only-release/case.own", + "source": "module WpfSubscriptionXamlNameOnlyRelease\n\n// A subscription token: `+= handler` acquires the source<->listener edge; the\n// matching `-= handler` releases it. `kind` tags the resource so the generic\n// ownership finding carries a [resource: ...] note.\nresource Subscription {\n acquire Subscribe\n release Unsubscribe\n kind \"subscription token\"\n}\n\n// The ctor subscribes; the only `-=` lives in a method whose NAME matches the\n// XAML handler convention but which nothing in code wires to any event. A name\n// is not wiring — the method is unreachable as far as the code can prove.\n// Modelled as the ctor scope with no release => OWN001.\nfn SettingsView(settings: int) {\n let sub = acquire Subscription(settings);\n // the name-only `Window_Closing` is not modelled as a release: nothing\n // proves the platform ever calls it (before.cs)\n}\n", + "cfg": "{\n \"functions\": [\n {\n \"blocks\": [\n {\n \"id\": 0,\n \"instrs\": [\n {\n \"line\": 17,\n \"op\": \"acquire\",\n \"resource\": \"Subscription\",\n \"sym\": 1\n }\n ],\n \"label\": \"entry\",\n \"succ\": []\n }\n ],\n \"entry\": 0,\n \"has_return_type\": false,\n \"name\": \"SettingsView\",\n \"params\": [\n 0\n ],\n \"symbols\": [\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 16,\n \"is_param_borrow\": false,\n \"kind\": \"plain\",\n \"name\": \"settings\",\n \"origin\": \"settings#16\",\n \"resource_kind\": null,\n \"type_name\": \"int\"\n },\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 17,\n \"is_param_borrow\": false,\n \"kind\": \"owned\",\n \"name\": \"sub\",\n \"origin\": \"sub#17\",\n \"resource_kind\": \"subscription token\",\n \"type_name\": \"Subscription\"\n }\n ]\n }\n ],\n \"ownlang_cfg_version\": 0\n}", + "diags": [] + }, { "name": "corpus/wpf/systemevents-region-escape/case.own", "source": "module SystemEventsRegionEscape\n\n// Lifetime regions. The static `Microsoft.Win32.SystemEvents` class lives for the\n// whole process; a WPF window/dialog that subscribes to it is strictly\n// shorter-lived. This `<` order is exactly what the C# bridge gives a `capture`\n// OwnIR fact whose source is a `static` (process-lived) event — see\n// ownlang/ownir.py (`Subscriber < Process`); the corpus uses the WPF-authentic\n// name `Window` for the short region.\nlifetime Process; // Microsoft.Win32.SystemEvents — a static, process-lived source\nlifetime Window < Process; // the dialog/window — strictly shorter-lived\n\n// The window strongly subscribes itself to the process-lived\n// SystemEvents.DisplaySettingsChanged static event and keeps no unsubscribe\n// token. Because Process strictly outlives Window, the strong delegate promotes\n// the window to process lifetime -> it can never be collected while the app runs\n// => OWN014. This is the SAME real bug as\n// corpus/real-world/screentogif-systemevents-leak, seen through the REGION model\n// (escape) rather than the token model (OWN001) — and the precise shape the\n// extractor lowers a static-event `+=` to (the `capture` fact, P-004 WPF005).\nfn GraphicsConfigurationDialog(systemEvents: SystemEvents lifetime Process) lifetime Window {\n subscribe self to systemEvents;\n}\n", diff --git a/tests/fixtures/diag_parity.json b/tests/fixtures/diag_parity.json index 0588fe6d..d720c25b 100644 --- a/tests/fixtures/diag_parity.json +++ b/tests/fixtures/diag_parity.json @@ -395,6 +395,46 @@ ] ] }, + { + "name": "corpus/wpf/subscription-finalizer-release/case.own", + "source": "module WpfSubscriptionFinalizerRelease\n\n// A subscription token: `+= handler` acquires the source<->listener edge; the\n// matching `-= handler` releases it. `kind` tags the resource so the generic\n// ownership finding carries a [resource: ...] note.\nresource Subscription {\n acquire Subscribe\n release Unsubscribe\n kind \"subscription token\"\n}\n\n// The ctor subscribes; the only `-=` lives in the finalizer. A finalizer runs\n// only after the object becomes unreachable — but the live subscription is\n// exactly what keeps it reachable, so the release path is blocked by the leak\n// it is supposed to fix. Modelled as the ctor scope with no release => OWN001.\nfn FinalizerDetachDocument(properties: int) {\n let sub = acquire Subscription(properties);\n // the finalizer `-=` is not modelled as a release: it cannot run while\n // the subscription holds `this` reachable (before.cs)\n}\n", + "diags": [ + [ + 17, + "OWN001" + ] + ] + }, + { + "name": "corpus/wpf/subscription-nonteardown-release/case.own", + "source": "module WpfSubscriptionNonTeardownRelease\n\n// A subscription token: `+= handler` acquires the source<->listener edge; the\n// matching `-= handler` releases it. `kind` tags the resource so the generic\n// ownership finding carries a [resource: ...] note.\nresource Subscription {\n acquire Subscribe\n release Unsubscribe\n kind \"subscription token\"\n}\n\n// The ctor subscribes; the only `-=` lives in an arbitrary non-teardown method\n// (`StopListening`) that no lifecycle path is proven to call. Modelled as the\n// ctor scope alone: the token is acquired and never released within any\n// teardown => OWN001. (The `-=`'s existence elsewhere is not modelled as a\n// release — that is exactly the #278 rule: existence is not execution.)\nfn PriceListener(prices: int) {\n let sub = acquire Subscription(prices);\n // no `release sub;` on any teardown path -> unreleased subscription (before.cs)\n}\n", + "diags": [ + [ + 18, + "OWN001" + ] + ] + }, + { + "name": "corpus/wpf/subscription-overload-conflated-cleanup/case.own", + "source": "module WpfSubscriptionOverloadConflatedCleanup\n\n// A subscription token: `+= handler` acquires the source<->listener edge; the\n// matching `-= handler` releases it. `kind` tags the resource so the generic\n// ownership finding carries a [resource: ...] note.\nresource Subscription {\n acquire Subscribe\n release Unsubscribe\n kind \"subscription token\"\n}\n\n// The ctor subscribes; Dispose calls the Cleanup() overload that detaches\n// nothing, while the `-=` lives only in the never-called Cleanup(bool).\n// Modelled as the ctor+Dispose scope where the called path performs no\n// release => OWN001.\nfn ReportView(report: int) {\n let sub = acquire Subscription(report);\n // Dispose() -> Cleanup(): no release on the called overload; the\n // Cleanup(bool) holding the `-=` is never called (before.cs)\n}\n", + "diags": [ + [ + 17, + "OWN001" + ] + ] + }, + { + "name": "corpus/wpf/subscription-param-guarded-unregister/case.own", + "source": "module WpfSubscriptionParamGuardedUnregister\n\n// A subscription token: `+= handler` acquires the source<->listener edge; the\n// matching `-= handler` releases it. `kind` tags the resource so the generic\n// ownership finding carries a [resource: ...] note.\nresource Subscription {\n acquire Subscribe\n release Unsubscribe\n kind \"subscription token\"\n}\n\n// The ctor subscribes; the only release sits behind a caller-controlled flag in\n// a non-teardown method (`UnregisterEventHandlers(bool UnregOnlyGoodys)` under\n// `if (!UnregOnlyGoodys)`). Modelled as one scope: on the flag=true path the\n// release never runs, so the token is not released on all paths => OWN001.\n// The real callers DO pass true (GTDService, DocCloud) — the leak path is the\n// production path, not a corner.\nfn GoodsDocument(properties: int, unregOnlyGoodys: int) {\n let sub = acquire Subscription(properties);\n if (unregOnlyGoodys) {\n return; // caller passed true -> the `-=` block is skipped\n }\n release sub; // the guarded `-=`: runs only when the flag is false\n}\n", + "diags": [ + [ + 21, + "OWN001" + ] + ] + }, { "name": "corpus/wpf/subscription-target-typed-delegate-release/case.own", "source": "module WpfSubscriptionTargetTypedDelegateRelease\n\n// A subscription token: `+= handler` acquires the source<->listener edge; the\n// matching `-= handler` releases it. `kind` tags the resource so the finding\n// carries a [resource: ...] note.\nresource Subscription {\n acquire Subscribe\n release Dispose\n kind \"subscription token\"\n}\n\n// The view modelled as one scope: the ctor subscribes to the injected source and\n// (in before.cs) never unsubscribes — no Dispose => OWN001.\n//\n// The C# after.cs releases it in Dispose, but the ctor `+=` uses C# 9 target-typed\n// delegate creation `new(H)` while the `-=` is a bare method group. That syntactic\n// asymmetry is what the extractor must normalize; this .own reduction only carries\n// the acquire/release logic the core reasons about.\nfn SourceView(source: int) {\n let sub = acquire Subscription(source);\n // no `release sub;` -> unreleased subscription (before.cs)\n}\n", @@ -405,6 +445,26 @@ ] ] }, + { + "name": "corpus/wpf/subscription-uncalled-local-function/case.own", + "source": "module WpfSubscriptionUncalledLocalFunction\n\n// A subscription token: `+= handler` acquires the source<->listener edge; the\n// matching `-= handler` releases it. `kind` tags the resource so the generic\n// ownership finding carries a [resource: ...] note.\nresource Subscription {\n acquire Subscribe\n release Unsubscribe\n kind \"subscription token\"\n}\n\n// The ctor subscribes; the only `-=` sits in a nested callable (local\n// function / lambda) that Dispose declares but never invokes. Declaration is\n// not execution, so the teardown path performs no release => OWN001.\nfn UncalledLocalFunctionView(model: int) {\n let sub = acquire Subscription(model);\n // Dispose declares Detach() but never calls it: no release on any\n // executed path (before.cs)\n}\n", + "diags": [ + [ + 16, + "OWN001" + ] + ] + }, + { + "name": "corpus/wpf/subscription-xaml-name-only-release/case.own", + "source": "module WpfSubscriptionXamlNameOnlyRelease\n\n// A subscription token: `+= handler` acquires the source<->listener edge; the\n// matching `-= handler` releases it. `kind` tags the resource so the generic\n// ownership finding carries a [resource: ...] note.\nresource Subscription {\n acquire Subscribe\n release Unsubscribe\n kind \"subscription token\"\n}\n\n// The ctor subscribes; the only `-=` lives in a method whose NAME matches the\n// XAML handler convention but which nothing in code wires to any event. A name\n// is not wiring — the method is unreachable as far as the code can prove.\n// Modelled as the ctor scope with no release => OWN001.\nfn SettingsView(settings: int) {\n let sub = acquire Subscription(settings);\n // the name-only `Window_Closing` is not modelled as a release: nothing\n // proves the platform ever calls it (before.cs)\n}\n", + "diags": [ + [ + 17, + "OWN001" + ] + ] + }, { "name": "corpus/wpf/systemevents-region-escape/case.own", "source": "module SystemEventsRegionEscape\n\n// Lifetime regions. The static `Microsoft.Win32.SystemEvents` class lives for the\n// whole process; a WPF window/dialog that subscribes to it is strictly\n// shorter-lived. This `<` order is exactly what the C# bridge gives a `capture`\n// OwnIR fact whose source is a `static` (process-lived) event — see\n// ownlang/ownir.py (`Subscriber < Process`); the corpus uses the WPF-authentic\n// name `Window` for the short region.\nlifetime Process; // Microsoft.Win32.SystemEvents — a static, process-lived source\nlifetime Window < Process; // the dialog/window — strictly shorter-lived\n\n// The window strongly subscribes itself to the process-lived\n// SystemEvents.DisplaySettingsChanged static event and keeps no unsubscribe\n// token. Because Process strictly outlives Window, the strong delegate promotes\n// the window to process lifetime -> it can never be collected while the app runs\n// => OWN014. This is the SAME real bug as\n// corpus/real-world/screentogif-systemevents-leak, seen through the REGION model\n// (escape) rather than the token model (OWN001) — and the precise shape the\n// extractor lowers a static-event `+=` to (the `capture` fact, P-004 WPF005).\nfn GraphicsConfigurationDialog(systemEvents: SystemEvents lifetime Process) lifetime Window {\n subscribe self to systemEvents;\n}\n", From 747b8fc0cf54c235b01959b9cb7022a49e9308d2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 14:25:18 +0000 Subject: [PATCH 6/7] =?UTF-8?q?test(corpus):=20red=20=E2=80=94=20#278=20an?= =?UTF-8?q?=20ambiguous=20unresolved-wiring=20name=20must=20not=20ground?= =?UTF-8?q?=20a=20teardown?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit corpus/wpf/subscription-ambiguous-overload-wiring pins the last silent- exemption residue: `Closing += Window_Closing` on an UNRESOLVED lifecycle event with TWO `Window_Closing` overloads, where the delegate-compatible overload detaches nothing and the `-=` sits in the never-attached sibling. The runtime delegate attaches exactly one overload — chosen by the event's delegate signature, the very information the extractor lacks — so the name fallback that credited every same-named method silently cleared OWN001 (verified silent at the previous head). before.cs must be OWN001; after.cs keeps a single `Window_Closing` holding the `-=` (unambiguous name) and must stay silent. CFG/diag parity fixtures regenerated for the new case.own (additive); the Rust side replays them clean with no Rust changes. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01MdYFKUaygHz1T9H1qJ7BqK --- .../after.cs | 29 +++++++++++++ .../before.cs | 41 +++++++++++++++++++ .../case.own | 20 +++++++++ .../expected-diagnostics.txt | 1 + .../notes.md | 40 ++++++++++++++++++ tests/fixtures/cfg_parity.json | 6 +++ tests/fixtures/diag_parity.json | 10 +++++ 7 files changed, 147 insertions(+) create mode 100644 corpus/wpf/subscription-ambiguous-overload-wiring/after.cs create mode 100644 corpus/wpf/subscription-ambiguous-overload-wiring/before.cs create mode 100644 corpus/wpf/subscription-ambiguous-overload-wiring/case.own create mode 100644 corpus/wpf/subscription-ambiguous-overload-wiring/expected-diagnostics.txt create mode 100644 corpus/wpf/subscription-ambiguous-overload-wiring/notes.md diff --git a/corpus/wpf/subscription-ambiguous-overload-wiring/after.cs b/corpus/wpf/subscription-ambiguous-overload-wiring/after.cs new file mode 100644 index 00000000..9b093f5e --- /dev/null +++ b/corpus/wpf/subscription-ambiguous-overload-wiring/after.cs @@ -0,0 +1,29 @@ +// FIXED. Exactly ONE `Window_Closing` remains — the delegate-compatible one — +// and it holds the `-=`. With the lifecycle event still unresolved, the name +// is now UNAMBIGUOUS (a single same-named method in the immediate class), so +// the fallback may credit it: whichever overload the delegate would pick, it +// is this one. +// +// own-check MUST treat the subscription as released (silent); the unresolved +// `Closing +=` itself stays the usual OWN050 advisory. +using System; +using System.ComponentModel; + +public partial class OrdersWindow : Window +{ + private readonly INotifyPropertyChanged _orders; // injected, unknown lifetime + + public OrdersWindow(INotifyPropertyChanged orders) + { + _orders = orders; + _orders.PropertyChanged += OnOrdersChanged; + Closing += Window_Closing; + } + + private void Window_Closing(object sender, CancelEventArgs e) + { + _orders.PropertyChanged -= OnOrdersChanged; // the one and only candidate + } + + private void OnOrdersChanged(object sender, PropertyChangedEventArgs e) { /* ... */ } +} diff --git a/corpus/wpf/subscription-ambiguous-overload-wiring/before.cs b/corpus/wpf/subscription-ambiguous-overload-wiring/before.cs new file mode 100644 index 00000000..e7d5904e --- /dev/null +++ b/corpus/wpf/subscription-ambiguous-overload-wiring/before.cs @@ -0,0 +1,41 @@ +// BUGGY (#278 follow-up 2 — the unresolved-overload fallback; hand-reduced +// into case.own). +// +// The class wires `Closing += Window_Closing` on an UNRESOLVED lifecycle event +// (the WPF `Window` base never resolves on a Linux runner without the +// reference pack) and declares TWO `Window_Closing` overloads. The runtime +// delegate attaches exactly ONE of them — chosen by the event's delegate +// signature, which is precisely the information the extractor is missing. The +// delegate-compatible overload detaches nothing; the `-=` sits in the OTHER, +// never-attached overload. +// +// own-check MUST flag the subscription OWN001: an ambiguous name (2+ same-named +// methods) may not ground the teardown, else a `-=` in the wrong overload +// silently swallows the leak. (`Window` is deliberately not defined in-file — +// the unresolved `Closing +=` itself surfaces as the usual OWN050 advisory.) +using System; +using System.ComponentModel; + +public partial class OrdersWindow : Window +{ + private readonly INotifyPropertyChanged _orders; // injected, unknown lifetime + + public OrdersWindow(INotifyPropertyChanged orders) + { + _orders = orders; + _orders.PropertyChanged += OnOrdersChanged; + Closing += Window_Closing; + } + + private void Window_Closing(object sender, CancelEventArgs e) + { + // the delegate-compatible overload: detaches nothing + } + + private void Window_Closing(object sender, EventArgs e) + { + _orders.PropertyChanged -= OnOrdersChanged; // never attached at runtime + } + + private void OnOrdersChanged(object sender, PropertyChangedEventArgs e) { /* ... */ } +} diff --git a/corpus/wpf/subscription-ambiguous-overload-wiring/case.own b/corpus/wpf/subscription-ambiguous-overload-wiring/case.own new file mode 100644 index 00000000..3699f5ae --- /dev/null +++ b/corpus/wpf/subscription-ambiguous-overload-wiring/case.own @@ -0,0 +1,20 @@ +module WpfSubscriptionAmbiguousOverloadWiring + +// A subscription token: `+= handler` acquires the source<->listener edge; the +// matching `-= handler` releases it. `kind` tags the resource so the generic +// ownership finding carries a [resource: ...] note. +resource Subscription { + acquire Subscribe + release Unsubscribe + kind "subscription token" +} + +// The ctor subscribes; the wired teardown handler name is AMBIGUOUS (two +// overloads, the event unresolved), and the `-=` sits in the overload the +// delegate never attaches. The executed teardown path performs no release +// => OWN001. +fn OrdersWindow(orders: int) { + let sub = acquire Subscription(orders); + // the attached Window_Closing overload detaches nothing; the `-=` lives + // in the never-attached sibling overload (before.cs) +} diff --git a/corpus/wpf/subscription-ambiguous-overload-wiring/expected-diagnostics.txt b/corpus/wpf/subscription-ambiguous-overload-wiring/expected-diagnostics.txt new file mode 100644 index 00000000..ed2a1929 --- /dev/null +++ b/corpus/wpf/subscription-ambiguous-overload-wiring/expected-diagnostics.txt @@ -0,0 +1 @@ +OWN001 diff --git a/corpus/wpf/subscription-ambiguous-overload-wiring/notes.md b/corpus/wpf/subscription-ambiguous-overload-wiring/notes.md new file mode 100644 index 00000000..26bad67f --- /dev/null +++ b/corpus/wpf/subscription-ambiguous-overload-wiring/notes.md @@ -0,0 +1,40 @@ +# Subscription whose `-=` sits in the never-attached overload of an ambiguously-named wired handler (soundness FN) + +**Pattern (#278 follow-up 2 — the unresolved-overload fallback blocker).** The +class wires `Closing += Window_Closing` where the lifecycle event does NOT +resolve (WPF `Window` base on a Linux runner without the reference pack), and +declares TWO `Window_Closing` overloads. A method group syntactically denotes +its whole overload set, but the runtime delegate attaches exactly ONE member — +selected by the event's delegate signature, which is precisely the information +the extractor is missing. Here the delegate-compatible overload +(`(object, CancelEventArgs)`) detaches nothing, and the `-=` sits in the other, +never-attached overload. + +**The bug.** The previous slice's unresolved-event fallback added EVERY +same-named own method to the teardown set, so the `-=` in the never-attached +overload was silently credited as a release — the unresolved twin of the +invocation-overload conflation pinned by +`subscription-overload-conflated-cleanup`. + +**The fix.** When the handler binds no definite symbol, the name grounds a +teardown ONLY if it is unambiguous — exactly one `IMethodSymbol` with that name +in the immediate class. Zero or 2+ matches credit nothing and keep the honest +warning. The symbol-resolved path is unchanged: when the event resolves, the +delegate's exact target is credited even among overloads (pinned in the smoke +matrix; `subscription-xaml-name-only-release/after.cs` keeps the resolved +single-handler control). The prior fallback's `CandidateSymbols` crediting is +gone with it — candidates of a failed method-group binding are the same +ambiguous overload set by another name. + +**`before.cs`** → OWN001 (ambiguous, `-=` unproven). **`after.cs`** — the +positive control: same unresolved event, exactly one `Window_Closing`, `-=` +inside it → silent (whichever overload the delegate would pick, it is that +one). Both keep the usual OWN050 advisory for the unresolved `Closing +=` +itself. + +**What the checker says (`.own` reduction).** The scope acquires the token and +the executed teardown path performs no release => **OWN001** with the +subscription-token resource tag. + +**Regression guard.** `scripts/benchmark.py`: `before.cs` must be **caught**, +`after.cs` must be **silent**. diff --git a/tests/fixtures/cfg_parity.json b/tests/fixtures/cfg_parity.json index df2d74d2..67d97e06 100644 --- a/tests/fixtures/cfg_parity.json +++ b/tests/fixtures/cfg_parity.json @@ -229,6 +229,12 @@ "cfg": "{\n \"functions\": [\n {\n \"blocks\": [\n {\n \"id\": 0,\n \"instrs\": [\n {\n \"line\": 17,\n \"op\": \"acquire\",\n \"resource\": \"PooledBuffer\",\n \"sym\": 1\n },\n {\n \"line\": 18,\n \"op\": \"release\",\n \"sym\": 1\n },\n {\n \"line\": 19,\n \"op\": \"use\",\n \"sym\": 1\n }\n ],\n \"label\": \"entry\",\n \"succ\": []\n }\n ],\n \"entry\": 0,\n \"has_return_type\": false,\n \"name\": \"OnFrameReady\",\n \"params\": [\n 0\n ],\n \"symbols\": [\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 16,\n \"is_param_borrow\": false,\n \"kind\": \"plain\",\n \"name\": \"bus\",\n \"origin\": \"bus#16\",\n \"resource_kind\": null,\n \"type_name\": \"int\"\n },\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 17,\n \"is_param_borrow\": false,\n \"kind\": \"owned\",\n \"name\": \"owner\",\n \"origin\": \"owner#17\",\n \"resource_kind\": \"pooled buffer\",\n \"type_name\": \"PooledBuffer\"\n }\n ]\n }\n ],\n \"ownlang_cfg_version\": 0\n}", "diags": [] }, + { + "name": "corpus/wpf/subscription-ambiguous-overload-wiring/case.own", + "source": "module WpfSubscriptionAmbiguousOverloadWiring\n\n// A subscription token: `+= handler` acquires the source<->listener edge; the\n// matching `-= handler` releases it. `kind` tags the resource so the generic\n// ownership finding carries a [resource: ...] note.\nresource Subscription {\n acquire Subscribe\n release Unsubscribe\n kind \"subscription token\"\n}\n\n// The ctor subscribes; the wired teardown handler name is AMBIGUOUS (two\n// overloads, the event unresolved), and the `-=` sits in the overload the\n// delegate never attaches. The executed teardown path performs no release\n// => OWN001.\nfn OrdersWindow(orders: int) {\n let sub = acquire Subscription(orders);\n // the attached Window_Closing overload detaches nothing; the `-=` lives\n // in the never-attached sibling overload (before.cs)\n}\n", + "cfg": "{\n \"functions\": [\n {\n \"blocks\": [\n {\n \"id\": 0,\n \"instrs\": [\n {\n \"line\": 17,\n \"op\": \"acquire\",\n \"resource\": \"Subscription\",\n \"sym\": 1\n }\n ],\n \"label\": \"entry\",\n \"succ\": []\n }\n ],\n \"entry\": 0,\n \"has_return_type\": false,\n \"name\": \"OrdersWindow\",\n \"params\": [\n 0\n ],\n \"symbols\": [\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 16,\n \"is_param_borrow\": false,\n \"kind\": \"plain\",\n \"name\": \"orders\",\n \"origin\": \"orders#16\",\n \"resource_kind\": null,\n \"type_name\": \"int\"\n },\n {\n \"borrow_is_mut\": null,\n \"buffer\": null,\n \"def_line\": 17,\n \"is_param_borrow\": false,\n \"kind\": \"owned\",\n \"name\": \"sub\",\n \"origin\": \"sub#17\",\n \"resource_kind\": \"subscription token\",\n \"type_name\": \"Subscription\"\n }\n ]\n }\n ],\n \"ownlang_cfg_version\": 0\n}", + "diags": [] + }, { "name": "corpus/wpf/subscription-explicit-delegate-release/case.own", "source": "module WpfSubscriptionExplicitDelegateRelease\n\n// A subscription token: `+= handler` acquires the source<->listener edge; the\n// matching `-= handler` releases it. `kind` tags the resource so the generic\n// ownership finding carries a [resource: ...] note.\nresource Subscription {\n acquire Subscribe\n release Dispose\n kind \"subscription token\"\n}\n\n// The view modelled as one scope: the ctor subscribes to the injected source and\n// (in before.cs) never unsubscribes — no Dispose. Acquiring the token without\n// releasing it = the injected source keeps the listener alive => OWN001.\n//\n// The C# after.cs releases it in Dispose — but writes the `-=` as a bare method\n// group while\n// the `+=` wraps the handler in `new PropertyChangedEventHandler(...)`. That\n// syntactic asymmetry is what the extractor must normalize; this .own reduction\n// only carries the acquire/release logic the core reasons about.\nfn SourceView(source: int) {\n let sub = acquire Subscription(source);\n // no `release sub;` -> unreleased subscription (before.cs)\n}\n", diff --git a/tests/fixtures/diag_parity.json b/tests/fixtures/diag_parity.json index d720c25b..e65a1dcb 100644 --- a/tests/fixtures/diag_parity.json +++ b/tests/fixtures/diag_parity.json @@ -385,6 +385,16 @@ ] ] }, + { + "name": "corpus/wpf/subscription-ambiguous-overload-wiring/case.own", + "source": "module WpfSubscriptionAmbiguousOverloadWiring\n\n// A subscription token: `+= handler` acquires the source<->listener edge; the\n// matching `-= handler` releases it. `kind` tags the resource so the generic\n// ownership finding carries a [resource: ...] note.\nresource Subscription {\n acquire Subscribe\n release Unsubscribe\n kind \"subscription token\"\n}\n\n// The ctor subscribes; the wired teardown handler name is AMBIGUOUS (two\n// overloads, the event unresolved), and the `-=` sits in the overload the\n// delegate never attaches. The executed teardown path performs no release\n// => OWN001.\nfn OrdersWindow(orders: int) {\n let sub = acquire Subscription(orders);\n // the attached Window_Closing overload detaches nothing; the `-=` lives\n // in the never-attached sibling overload (before.cs)\n}\n", + "diags": [ + [ + 17, + "OWN001" + ] + ] + }, { "name": "corpus/wpf/subscription-explicit-delegate-release/case.own", "source": "module WpfSubscriptionExplicitDelegateRelease\n\n// A subscription token: `+= handler` acquires the source<->listener edge; the\n// matching `-= handler` releases it. `kind` tags the resource so the generic\n// ownership finding carries a [resource: ...] note.\nresource Subscription {\n acquire Subscribe\n release Dispose\n kind \"subscription token\"\n}\n\n// The view modelled as one scope: the ctor subscribes to the injected source and\n// (in before.cs) never unsubscribes — no Dispose. Acquiring the token without\n// releasing it = the injected source keeps the listener alive => OWN001.\n//\n// The C# after.cs releases it in Dispose — but writes the `-=` as a bare method\n// group while\n// the `+=` wraps the handler in `new PropertyChangedEventHandler(...)`. That\n// syntactic asymmetry is what the extractor must normalize; this .own reduction\n// only carries the acquire/release logic the core reasons about.\nfn SourceView(source: int) {\n let sub = acquire Subscription(source);\n // no `release sub;` -> unreleased subscription (before.cs)\n}\n", From 18000d01d24d7064a9053a85a60830112b5e5769 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 14:25:29 +0000 Subject: [PATCH 7/7] =?UTF-8?q?fix(extractor):=20green=20=E2=80=94=20#278?= =?UTF-8?q?=20credit=20an=20unresolved-wiring=20name=20only=20when=20unamb?= =?UTF-8?q?iguous?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The unresolved-lifecycle-event fallback (`Closing += Window_Closing` where the event binds no definite symbol) now grounds a teardown ONLY when exactly one method with that name exists in the immediate class. Zero or 2+ same-named methods credit nothing — the delegate attaches exactly one overload, selected by the event's delegate signature the extractor is missing, so an ambiguous name may not let a `-=` in the never-attached overload clear OWN001 (the unresolved twin of the invocation-overload conflation closed previously). The former CandidateSymbols crediting is removed along with it: candidates of a failed method-group binding are the same ambiguous overload set by another name. The symbol-RESOLVED path is unchanged — a resolved event credits the delegate's exact target, even among overloads. Evidence (docs/notes/own278-corpus-diff.md, follow-up 2): benchmark 47/51 caught, 51/51 fixes clean, 0 FPs, prior rows unchanged; samples byte-identical; golden unchanged; suite/ruff/mypy, fix-candidates, S2 gates, and Rust parity (full_parity_on_the_frozen_corpus) green. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01MdYFKUaygHz1T9H1qJ7BqK --- docs/notes/own278-corpus-diff.md | 30 ++++++++++++++++++ frontend/roslyn/OwnSharp.Extractor/Program.cs | 31 +++++++++++++------ 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/docs/notes/own278-corpus-diff.md b/docs/notes/own278-corpus-diff.md index 4f692a1e..a1754185 100644 --- a/docs/notes/own278-corpus-diff.md +++ b/docs/notes/own278-corpus-diff.md @@ -162,3 +162,33 @@ Unchanged by the follow-up: the reduction still flags GTD, PGC and KDT (OWN001) and keeps the Dispose-releasing sibling silent. The real `STS_new/SectorTS` run and the OwnAudit STS baseline (GTD = `runtime-only`) remain the two pre-merge gates, executed locally. + +--- + +# Follow-up 2: the unresolved-overload fallback made unambiguous + +One residual hole from the follow-up: when `Closing += Window_Closing` binds no +definite symbol (unresolved lifecycle event), the name fallback added EVERY +same-named own method to the teardown set. A method group syntactically denotes +its overload set, but the runtime delegate attaches exactly ONE member — +selected by the event's delegate signature, which is precisely what is missing +without the reference — so a `-=` in the wrong, never-attached overload could +still silently clear OWN001. + +Fix: the fallback credits the name ONLY when it is unambiguous — exactly one +`IMethodSymbol` with that name in the immediate class; zero or 2+ matches +credit nothing and keep the warning. The former `CandidateSymbols` crediting is +gone with it (candidates of a failed method-group binding are the same +ambiguous overload set by another name). The symbol-resolved path is unchanged +— a RESOLVED event credits the delegate's exact target even among overloads. + +Pinned red→green by `corpus/wpf/subscription-ambiguous-overload-wiring` +(before: two `Window_Closing` overloads, `-=` in the never-attached one → +OWN001, silent under the previous head; after: a single `Window_Closing` +holding the `-=` → silent). Verified: corpus benchmark **47/51 caught · 51/51 +fixes clean · 0 FPs** (all prior rows unchanged); samples byte-identical; +golden unchanged; CFG/diag parity fixtures regenerated (additive) and the Rust +side replays them clean (`cargo test`, incl. `full_parity_on_the_frozen_corpus`); +full suite, ruff, mypy, fix-candidates and S2 gate checks green (the single +gate failure is the known root-runner read-only-chmod environmental one, +reproduced on the frozen baseline). diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 859fa16b..a1c07a94 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -923,24 +923,35 @@ void Add(IMethodSymbol? m) if (h is not (IdentifierNameSyntax or MemberAccessExpressionSyntax { Expression: ThisExpressionSyntax })) continue; var info = model.GetSymbolInfo(h); - var bound = false; - if (info.Symbol is IMethodSymbol hm) { Add(hm); bound = true; } + if (info.Symbol is IMethodSymbol hm) + { + Add(hm); // symbol-resolved: the event delegate picked its exact target + } else - foreach (var cand in info.CandidateSymbols.OfType()) { Add(cand); bound = true; } - if (!bound) { // The lifecycle EVENT is unresolved (`Closing +=` on a base the runner // cannot reference, e.g. WPF `Window` on Linux), so the method group - // binds no symbol. Fall back to the group's NAME over the class's own - // methods: a method group carries no argument list, so the name selects - // the same overload SET the group itself denotes — this is not the - // invocation-style overload conflation the closure below rules out. + // binds no definite symbol. The runtime delegate still attaches exactly + // ONE overload — chosen by the event's delegate signature, which is + // precisely what is missing here — so the group's NAME may ground a + // teardown ONLY when it is unambiguous: exactly one method with that + // name in the immediate class. Zero or 2+ same-named methods credit + // NOTHING — a `-=` in the wrong, never-attached overload must keep the + // honest warning (the unresolved twin of the invocation-overload + // conflation the closure below rules out). var hn = h is IdentifierNameSyntax hid ? hid.Identifier.Text : ((MemberAccessExpressionSyntax)h).Name.Identifier.Text; + IMethodSymbol? only = null; + var unique = true; foreach (var md in cls.Members.OfType()) - if (md.Identifier.Text == hn) - Add(model.GetDeclaredSymbol(md)); + if (md.Identifier.Text == hn && model.GetDeclaredSymbol(md) is { } nds) + { + if (only is not null) { unique = false; break; } + only = nds; + } + if (unique && only is not null) + Add(only); } }