From 90b2a4cd4b0263ff0bab43ec308c8d23e0bce6ba Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 28 Jun 2026 06:59:25 +0000 Subject: [PATCH 01/13] oracle: TEMP dev-only run on protobuf-net to confirm Page.xaml.cs timer FP cleared Point the push-triggered oracle at protobuf-net to verify the using-statement local-release fix clears the assorted/ Silverlight Page.xaml.cs `timer` FP. With the baseline entry still present, the finding must be absent from BOTH own-only and the baselined section. Reverted in the next commit. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- .github/workflows/oracle.yml | 1 + corpus/oracle-target.txt | 20 ++++++++------------ 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/.github/workflows/oracle.yml b/.github/workflows/oracle.yml index 0a290055..cfc3fc66 100644 --- a/.github/workflows/oracle.yml +++ b/.github/workflows/oracle.yml @@ -45,6 +45,7 @@ on: push: branches: - claude/zen-pasteur-76hfs1 + - claude/mos-ownership-summary-n3q3j4 paths: - corpus/oracle-target.txt diff --git a/corpus/oracle-target.txt b/corpus/oracle-target.txt index 52cb6d64..6cc769f9 100644 --- a/corpus/oracle-target.txt +++ b/corpus/oracle-target.txt @@ -3,15 +3,11 @@ # or "local:" (a fixture copied into the oracle's target/). # Optional lines: ref=, paths=, build=, include_tests=. Dev-branch only. # -# Cross-tool oracle on a Linux-buildable fixture, so ALL THREE tools run (Infer# -# included — ScreenToGif's WPF won't build on Linux). Re-run after the OWN014 -# migration (PR #35): the fixture's SUBSCRIPTION leak (#1 — SystemEvents.Display -# SettingsChanged += with no -=) is now lowered by the extractor to a `capture` -# fact and reported as OWN014 (region escape), not OWN001; OWN014 was added to the -# comparator's OWN_LEAK set. So #1 must STILL land "Own.NET-only" (CodeQL / Infer# -# have no event-subscription-leak query) — this run confirms the migration kept the -# differentiation, on real cross-tool output rather than the selftest's fixtures. -# Expect: #1 Own.NET-only (now OWN014), #2/#3/#4 dispose leaks in "Agree", 0 -# oracle-only. -local:corpus/fixtures/systemevents-console -build=SystemEventsLeak.csproj +# TEMPORARY (revert before any PR): confirm the using-statement local-release fix +# (onThrowDefinite / the using(existingLocal) branch) clears the protobuf-net +# `assorted/ Silverlight Page.xaml.cs` `timer` FP. With the baseline still present, +# the finding must be ABSENT from BOTH "Own.NET only" AND "Known FP (baselined)" — +# that proves the extractor cleared it (not the baseline hiding it). include_tests +# so the assorted/ sample tree is not filtered out of the diff. +protobuf-net/protobuf-net +include_tests=true From 7bcd17c2cc281b49e7cc77aefd394c3e5902a27e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 28 Jun 2026 07:13:11 +0000 Subject: [PATCH 02/13] oracle: re-triage protobuf Page.xaml.cs timer FP (live run) + revert dev config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ran the cross-tool oracle on protobuf-net (via the push-trigger dev fallback; the token can't workflow_dispatch) to confirm whether the using-statement local-release fix clears the assorted/ Silverlight Page.xaml.cs `timer` FP. Result: NOT cleared — the finding is still emitted and still baselined. Re-triage of the real source shows the prior baseline reason was a misdiagnosis read from COMMENTED-OUT code: the only `using (timer)` block is commented out, and `timer` is a Stopwatch (RunTest) / a DispatcherTimer local (btnRunTest_Click) — neither is IDisposable. So the using-fix does not apply; the entry stays baselined as a non-product sample FP, and the real open question is why a non-IDisposable timer-shaped local is flagged at all (a precision follow-up needing a local repro). - Corrected the baseline entry reason and oracle-known-fps.md (root-cause #1 + the per-finding row). - Reverted the dev-only oracle config (oracle.yml push branch + oracle-target.txt back to the systemevents-console fixture). Selftests: oracle 31/31, corpus 30/30. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- .github/workflows/oracle.yml | 1 - corpus/oracle-fp-baseline.txt | 12 ++++++++---- corpus/oracle-target.txt | 20 ++++++++++++-------- docs/notes/oracle-known-fps.md | 25 ++++++++++++++++--------- 4 files changed, 36 insertions(+), 22 deletions(-) diff --git a/.github/workflows/oracle.yml b/.github/workflows/oracle.yml index cfc3fc66..0a290055 100644 --- a/.github/workflows/oracle.yml +++ b/.github/workflows/oracle.yml @@ -45,7 +45,6 @@ on: push: branches: - claude/zen-pasteur-76hfs1 - - claude/mos-ownership-summary-n3q3j4 paths: - corpus/oracle-target.txt diff --git a/corpus/oracle-fp-baseline.txt b/corpus/oracle-fp-baseline.txt index ce2708c5..5852e359 100644 --- a/corpus/oracle-fp-baseline.txt +++ b/corpus/oracle-fp-baseline.txt @@ -37,10 +37,14 @@ protobuf-net/protobuf-net | ProtoTranscoder.cs | OWN001 | 'sync' | non-product ( # a get-only property over a constructed XsltArgumentList field, handler captures `this`). # Now fixed at the source by PropertyReturnsOwnedMember — confirmed cleared on a live # protobuf oracle run (own-only 0, the finding absent from own-only and baselined) — deleted. -# FIX LANDED (using-statement local release): `using (timer) { ... }` over an already-acquired -# local is now threaded as a scope-exit release in --flow-locals. Kept here pending a live-oracle -# re-run on protobuf to confirm-and-delete. Fixture: local-dispose-via-using-statement. -protobuf-net/protobuf-net | Page.xaml.cs | OWN001 | local 'timer' | non-product (assorted/ Silverlight sample); the timer is disposed by an enclosing `using (timer) { ... }` the extractor missed +# RE-TRIAGED 2026-06-28 (live protobuf oracle run): the prior reason — "disposed by an enclosing +# using (timer) the extractor missed" — was a MISDIAGNOSIS read from COMMENTED-OUT code. In the +# real source the only `using (timer)` block is commented out, and `timer` is a Stopwatch (RunTest; +# btnRunTest_Click has a DispatcherTimer local) — neither is IDisposable. So the using-statement +# fix does NOT apply here (confirmed: still emitted, still baselined). It stays a non-product FP; +# the open question is WHY a non-IDisposable timer-shaped local is flagged at all (needs a local +# extractor repro). See docs/notes/oracle-known-fps.md root-cause #1. +protobuf-net/protobuf-net | Page.xaml.cs | OWN001 | local 'timer' | non-product (assorted/ Silverlight perf sample); `timer` is a non-IDisposable Stopwatch/DispatcherTimer and the only using(timer) block is commented out — not a missed-using; flagging it is a separate precision question, kept baselined # --- JamesNK/Newtonsoft.Json ---------------------------------------------------- # STAYS BASELINED (soundness wall): JsonTextWriter is a THIRD-PARTY wrapper — the BCL pass-through diff --git a/corpus/oracle-target.txt b/corpus/oracle-target.txt index 6cc769f9..52cb6d64 100644 --- a/corpus/oracle-target.txt +++ b/corpus/oracle-target.txt @@ -3,11 +3,15 @@ # or "local:" (a fixture copied into the oracle's target/). # Optional lines: ref=, paths=, build=, include_tests=. Dev-branch only. # -# TEMPORARY (revert before any PR): confirm the using-statement local-release fix -# (onThrowDefinite / the using(existingLocal) branch) clears the protobuf-net -# `assorted/ Silverlight Page.xaml.cs` `timer` FP. With the baseline still present, -# the finding must be ABSENT from BOTH "Own.NET only" AND "Known FP (baselined)" — -# that proves the extractor cleared it (not the baseline hiding it). include_tests -# so the assorted/ sample tree is not filtered out of the diff. -protobuf-net/protobuf-net -include_tests=true +# Cross-tool oracle on a Linux-buildable fixture, so ALL THREE tools run (Infer# +# included — ScreenToGif's WPF won't build on Linux). Re-run after the OWN014 +# migration (PR #35): the fixture's SUBSCRIPTION leak (#1 — SystemEvents.Display +# SettingsChanged += with no -=) is now lowered by the extractor to a `capture` +# fact and reported as OWN014 (region escape), not OWN001; OWN014 was added to the +# comparator's OWN_LEAK set. So #1 must STILL land "Own.NET-only" (CodeQL / Infer# +# have no event-subscription-leak query) — this run confirms the migration kept the +# differentiation, on real cross-tool output rather than the selftest's fixtures. +# Expect: #1 Own.NET-only (now OWN014), #2/#3/#4 dispose leaks in "Agree", 0 +# oracle-only. +local:corpus/fixtures/systemevents-console +build=SystemEventsLeak.csproj diff --git a/docs/notes/oracle-known-fps.md b/docs/notes/oracle-known-fps.md index ab17ca84..eb80c58b 100644 --- a/docs/notes/oracle-known-fps.md +++ b/docs/notes/oracle-known-fps.md @@ -87,7 +87,7 @@ this. | `src/protobuf-net.Core/ProtoWriter.BufferWriter.cs` `_nullWriter` | **FP → baseline** | intentional null-object kept attached for pooled reuse; `Dispose()` comments *"don't cascade dispose to the null one"* | | `assorted/.../ProtoTranscoder.cs` `sync` (×2 copies) | **true-but-benign → baseline (non-product sample)** | `NetTranscoder` isn't `IDisposable`; one `ReaderWriterLockSlim` for app lifetime in a sample/extension tree | | `assorted/ProtoGen/CommandLineOptions.cs` `XsltMessageEncountered` | **FP → baseline** | self-subscription: publisher (`xsltOptions`) and the lambda are both owned by the same `CommandLineOptions`, co-lifetimed | -| `assorted/SilverlightExtended/Page.xaml.cs` `timer` | **FP → baseline** | disposed by an enclosing `using (timer) { … }` the extractor missed (sample code) | +| `assorted/SilverlightExtended/Page.xaml.cs` `timer` | **FP → baseline (re-triaged 2026-06-28)** | NOT a missed `using` — the `using (timer)` block is commented out and `timer` is a non-`IDisposable` `Stopwatch`/`DispatcherTimer`; flagging it is a separate precision question. Sample code. | | `src/BuildToolsUnitTests/AnalyzerTestBase.cs` `logging.Log` | **test noise → path filter** | xUnit fixture; per-test lifetime | | `src/BuildToolsUnitTests/GeneratorTestBase.cs` `logging.Log` | **test noise → path filter** | xUnit fixture; per-test lifetime | @@ -142,14 +142,21 @@ entry and let the oracle re-confirm clean. its owned object), so the `current?.WaitForDispose(...)` is seen as a release — own-only 4 → 3, the NLog baseline now empty. Restricted to a `null`/`default` replacement: an exchange installing a new non-null value re-arms the field and is - declined (precision-first). **Also fixed — the protobuf `Page.xaml.cs` local.** The - `using (preExistingLocal)` form — `var timer = new ...; using (timer) { ... }`, where - the resource is an already-acquired tracked local — is now threaded as a scope-exit - release in the `--flow-locals` lowering (no `acquire`; only the missing release is - added, mirroring the `MemoryPool` owner branch). Its baseline entry is marked - fix-landed, pending a live-oracle re-run on protobuf to confirm-and-delete. Corpus - fixtures: `field-dispose-via-helper`, `field-dispose-via-exchange`, - `local-dispose-via-using-statement`. + declined (precision-first). **The `using (preExistingLocal)` form is now handled** — + `var r = new ...; using (r) { ... }`, an already-acquired tracked local, is threaded as + a scope-exit release in the `--flow-locals` lowering (no `acquire`; only the missing + release, mirroring the `MemoryPool` owner branch), with sound throw-routing + (`onThrowDefinite`). Corpus fixtures: `local-dispose-via-using-statement`, + `using-statement-throw-releases`. **It does NOT clear the protobuf `Page.xaml.cs` + baseline entry, though** — a live protobuf oracle run (2026-06-28) showed that finding + STILL emitted (still baselined), and re-triage of the real source found the prior + reason was a **misdiagnosis read from commented-out code**: the only `using (timer)` + block is commented out, and `timer` is a `Stopwatch` (`RunTest`) / a `DispatcherTimer` + local (`btnRunTest_Click`) — neither is `IDisposable`. So the entry stays baselined as a + non-product sample FP; the real open question is **why a non-`IDisposable` timer-shaped + local is flagged at all** (a precision follow-up needing a local extractor repro, not a + missed `using`). Other custom-sink fixtures: `field-dispose-via-helper`, + `field-dispose-via-exchange`. 2. **No-op `Dispose` not modelled — PARTLY FIXED** *(Newtonsoft `TraceJsonReader. _textWriter`).* We flag any undisposed `IDisposable` structurally, without modelling From c415d64d245b6080dd6e232cfca8ba516a4131cd Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 28 Jun 2026 07:16:23 +0000 Subject: [PATCH 03/13] docs: link issue #161 from the Page.xaml.cs timer FP re-triage notes Cross-reference the tracked precision follow-up (non-IDisposable timer local flagged OWN001) from the baseline entry and oracle-known-fps.md root-cause #1. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- corpus/oracle-fp-baseline.txt | 2 +- docs/notes/oracle-known-fps.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/corpus/oracle-fp-baseline.txt b/corpus/oracle-fp-baseline.txt index 5852e359..5bab0e76 100644 --- a/corpus/oracle-fp-baseline.txt +++ b/corpus/oracle-fp-baseline.txt @@ -43,7 +43,7 @@ protobuf-net/protobuf-net | ProtoTranscoder.cs | OWN001 | 'sync' | non-product ( # btnRunTest_Click has a DispatcherTimer local) — neither is IDisposable. So the using-statement # fix does NOT apply here (confirmed: still emitted, still baselined). It stays a non-product FP; # the open question is WHY a non-IDisposable timer-shaped local is flagged at all (needs a local -# extractor repro). See docs/notes/oracle-known-fps.md root-cause #1. +# extractor repro; tracked in issue #161). See docs/notes/oracle-known-fps.md root-cause #1. protobuf-net/protobuf-net | Page.xaml.cs | OWN001 | local 'timer' | non-product (assorted/ Silverlight perf sample); `timer` is a non-IDisposable Stopwatch/DispatcherTimer and the only using(timer) block is commented out — not a missed-using; flagging it is a separate precision question, kept baselined # --- JamesNK/Newtonsoft.Json ---------------------------------------------------- diff --git a/docs/notes/oracle-known-fps.md b/docs/notes/oracle-known-fps.md index eb80c58b..99d3ef98 100644 --- a/docs/notes/oracle-known-fps.md +++ b/docs/notes/oracle-known-fps.md @@ -155,7 +155,7 @@ entry and let the oracle re-confirm clean. local (`btnRunTest_Click`) — neither is `IDisposable`. So the entry stays baselined as a non-product sample FP; the real open question is **why a non-`IDisposable` timer-shaped local is flagged at all** (a precision follow-up needing a local extractor repro, not a - missed `using`). Other custom-sink fixtures: `field-dispose-via-helper`, + missed `using`; tracked in issue #161). Other custom-sink fixtures: `field-dispose-via-helper`, `field-dispose-via-exchange`. 2. **No-op `Dispose` not modelled — PARTLY FIXED** *(Newtonsoft `TraceJsonReader. From 47c46a674902ac2ba72d2789523126088158d5d5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 30 Jun 2026 16:53:33 +0000 Subject: [PATCH 04/13] oracle: Page.xaml.cs timer is a TRUE POSITIVE (custom IDisposable Stopwatch), not an FP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cracked the mechanism behind issue #161 from the core side: the finding renders via the flow-locals path (ownir.py rkind=="flow-local"), whose extractor candidate gate REQUIRES ImplementsIDisposable. The raw protobuf source shows the file has no `using System.Diagnostics;` — it has `using Nuxleus.Performance;` and uses `Stopwatch.UnitPrecision` / `UnitPrecision.NANOSECONDS` / `timer.Scope = () => …`. So `Stopwatch` is `Nuxleus.Performance.Stopwatch`, a custom IDisposable scope-timer (you can only `using` an IDisposable), NOT System.Diagnostics.Stopwatch. `Stopwatch timer = new Stopwatch()` is genuinely never disposed — the disposing `using (timer) { … }` block is commented out — so the OWN001 is a REAL leak the extractor correctly flagged. It stays baselined as NON-PRODUCT sample (assorted/ Silverlight demo), like the NetTranscoder `sync` entry — reclassified from FP to true-but-benign-sample. The earlier "missed using" and "non-IDisposable Stopwatch FP" readings were both wrong; issue #161 is closed as invalid. Corrected the baseline entry reason, oracle-known-fps.md root-cause #1, the per-finding verdict row, and the disposition summary (Baselined FP 6→5, true-but-benign sample 2→3). oracle selftest 31/31. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- corpus/oracle-fp-baseline.txt | 18 ++++++++++-------- docs/notes/oracle-known-fps.md | 33 ++++++++++++++++++++------------- 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/corpus/oracle-fp-baseline.txt b/corpus/oracle-fp-baseline.txt index 5bab0e76..738c0506 100644 --- a/corpus/oracle-fp-baseline.txt +++ b/corpus/oracle-fp-baseline.txt @@ -37,14 +37,16 @@ protobuf-net/protobuf-net | ProtoTranscoder.cs | OWN001 | 'sync' | non-product ( # a get-only property over a constructed XsltArgumentList field, handler captures `this`). # Now fixed at the source by PropertyReturnsOwnedMember — confirmed cleared on a live # protobuf oracle run (own-only 0, the finding absent from own-only and baselined) — deleted. -# RE-TRIAGED 2026-06-28 (live protobuf oracle run): the prior reason — "disposed by an enclosing -# using (timer) the extractor missed" — was a MISDIAGNOSIS read from COMMENTED-OUT code. In the -# real source the only `using (timer)` block is commented out, and `timer` is a Stopwatch (RunTest; -# btnRunTest_Click has a DispatcherTimer local) — neither is IDisposable. So the using-statement -# fix does NOT apply here (confirmed: still emitted, still baselined). It stays a non-product FP; -# the open question is WHY a non-IDisposable timer-shaped local is flagged at all (needs a local -# extractor repro; tracked in issue #161). See docs/notes/oracle-known-fps.md root-cause #1. -protobuf-net/protobuf-net | Page.xaml.cs | OWN001 | local 'timer' | non-product (assorted/ Silverlight perf sample); `timer` is a non-IDisposable Stopwatch/DispatcherTimer and the only using(timer) block is commented out — not a missed-using; flagging it is a separate precision question, kept baselined +# RE-TRIAGED 2026-06-28 (live protobuf oracle run + raw source). This is a TRUE POSITIVE, not an +# FP: `Stopwatch` here is `Nuxleus.Performance.Stopwatch` (the file has `using Nuxleus.Performance;` +# and uses `Stopwatch.UnitPrecision`/`UnitPrecision.NANOSECONDS`/`timer.Scope = () => …` — NOT +# System.Diagnostics.Stopwatch), a custom IDisposable scope-timer. `Stopwatch timer = new Stopwatch()` +# is genuinely never disposed: the disposing `using (timer) { … }` block is COMMENTED OUT. The +# extractor flags it via the flow-locals path, which requires ImplementsIDisposable — so it +# correctly resolved the custom type as IDisposable. Kept baselined as NON-PRODUCT sample (assorted/ +# Silverlight perf demo), exactly like the assorted/ NetTranscoder `sync` entry. The earlier +# "missed using" and "non-IDisposable Stopwatch FP" readings were both wrong; issue #161 is closed. +protobuf-net/protobuf-net | Page.xaml.cs | OWN001 | local 'timer' | non-product (assorted/ Silverlight perf demo); TRUE POSITIVE — `timer` is a custom IDisposable Nuxleus.Performance.Stopwatch genuinely never disposed (the using(timer) block is commented out). Baselined as non-product, not as an FP # --- JamesNK/Newtonsoft.Json ---------------------------------------------------- # STAYS BASELINED (soundness wall): JsonTextWriter is a THIRD-PARTY wrapper — the BCL pass-through diff --git a/docs/notes/oracle-known-fps.md b/docs/notes/oracle-known-fps.md index 99d3ef98..ca77f401 100644 --- a/docs/notes/oracle-known-fps.md +++ b/docs/notes/oracle-known-fps.md @@ -24,12 +24,16 @@ reason: we and the oracles occupy orthogonal niches. | disposition | count | what happens on re-run | |---|---:|---| | **Fixed in the extractor** | 6 | no longer fire (5 NLog `WaitForDispose` timers + protobuf `XsltOptions` self-cycle — see below) | -| **Baselined FP** | 6 | moved to "Known FP (baselined)", out of the triage queue | +| **Baselined FP** | 5 | moved to "Known FP (baselined)", out of the triage queue | | **Non-product (path filter)** | 2 | dropped by `--exclude-tests` (`unittest` rule) | | **True positive — kept visible** | 4 | stays in "Own.NET only" (real catch, oracle can't express) | -| **True-but-benign — kept, baselined-as-sample** | 2 | (protobuf `assorted/` samples) baselined as non-product | +| **True-but-benign — kept, baselined-as-sample** | 3 | (protobuf `assorted/` samples) baselined as non-product | -The 6 baselined FPs + the 2 non-product-sample reals = 8 findings, covered by +(Re-triage 2026-06-28: `Page.xaml.cs` `timer` moved from "Baselined FP" 6→5 to +"True-but-benign sample" 2→3 — it is a real leak of a custom `IDisposable` +`Nuxleus.Performance.Stopwatch`, not the BCL non-disposable type first assumed.) + +The 5 baselined FPs + the 3 non-product-sample reals = 8 findings, covered by **7 rules** in `corpus/oracle-fp-baseline.txt` (the two `NetTranscoder` copies share one basename-keyed rule); the 2 test-base findings are the `--exclude-tests` drops; the 4 true positives are deliberately **not** suppressed. @@ -87,7 +91,7 @@ this. | `src/protobuf-net.Core/ProtoWriter.BufferWriter.cs` `_nullWriter` | **FP → baseline** | intentional null-object kept attached for pooled reuse; `Dispose()` comments *"don't cascade dispose to the null one"* | | `assorted/.../ProtoTranscoder.cs` `sync` (×2 copies) | **true-but-benign → baseline (non-product sample)** | `NetTranscoder` isn't `IDisposable`; one `ReaderWriterLockSlim` for app lifetime in a sample/extension tree | | `assorted/ProtoGen/CommandLineOptions.cs` `XsltMessageEncountered` | **FP → baseline** | self-subscription: publisher (`xsltOptions`) and the lambda are both owned by the same `CommandLineOptions`, co-lifetimed | -| `assorted/SilverlightExtended/Page.xaml.cs` `timer` | **FP → baseline (re-triaged 2026-06-28)** | NOT a missed `using` — the `using (timer)` block is commented out and `timer` is a non-`IDisposable` `Stopwatch`/`DispatcherTimer`; flagging it is a separate precision question. Sample code. | +| `assorted/SilverlightExtended/Page.xaml.cs` `timer` | **true-but-non-product → baseline (re-triaged 2026-06-28)** | TRUE POSITIVE: `timer` is a custom `IDisposable` `Nuxleus.Performance.Stopwatch` (NOT `System.Diagnostics.Stopwatch`) never disposed — the disposing `using (timer)` is commented out. Correctly flagged; baselined as non-product sample. | | `src/BuildToolsUnitTests/AnalyzerTestBase.cs` `logging.Log` | **test noise → path filter** | xUnit fixture; per-test lifetime | | `src/BuildToolsUnitTests/GeneratorTestBase.cs` `logging.Log` | **test noise → path filter** | xUnit fixture; per-test lifetime | @@ -148,15 +152,18 @@ entry and let the oracle re-confirm clean. release, mirroring the `MemoryPool` owner branch), with sound throw-routing (`onThrowDefinite`). Corpus fixtures: `local-dispose-via-using-statement`, `using-statement-throw-releases`. **It does NOT clear the protobuf `Page.xaml.cs` - baseline entry, though** — a live protobuf oracle run (2026-06-28) showed that finding - STILL emitted (still baselined), and re-triage of the real source found the prior - reason was a **misdiagnosis read from commented-out code**: the only `using (timer)` - block is commented out, and `timer` is a `Stopwatch` (`RunTest`) / a `DispatcherTimer` - local (`btnRunTest_Click`) — neither is `IDisposable`. So the entry stays baselined as a - non-product sample FP; the real open question is **why a non-`IDisposable` timer-shaped - local is flagged at all** (a precision follow-up needing a local extractor repro, not a - missed `using`; tracked in issue #161). Other custom-sink fixtures: `field-dispose-via-helper`, - `field-dispose-via-exchange`. + baseline entry — because that entry is a TRUE POSITIVE, not an FP.** A live protobuf + oracle run (2026-06-28) + the raw source settled it: `Stopwatch` there is + `Nuxleus.Performance.Stopwatch` (the file has `using Nuxleus.Performance;` and uses + `Stopwatch.UnitPrecision` / `timer.Scope = () => …`, NOT `System.Diagnostics.Stopwatch`), + a custom **`IDisposable`** scope-timer that is genuinely never disposed (the disposing + `using (timer)` block is commented out). The extractor flags it via the flow-locals path, + which gates on `ImplementsIDisposable` — so it correctly resolved the custom type and + reported a real leak. It stays baselined as **non-product sample** (assorted/ Silverlight + demo), like the `NetTranscoder` `sync` entry — not as an FP. (Two earlier readings — + "missed `using`" and "non-`IDisposable` `Stopwatch` FP" — were both wrong; issue #161 was + opened on the second and then closed as invalid.) Other custom-sink fixtures: + `field-dispose-via-helper`, `field-dispose-via-exchange`. 2. **No-op `Dispose` not modelled — PARTLY FIXED** *(Newtonsoft `TraceJsonReader. _textWriter`).* We flag any undisposed `IDisposable` structurally, without modelling From 957dcb789a72479d9efbb2f1d7b203c60355e1e5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 30 Jun 2026 20:18:53 +0000 Subject: [PATCH 05/13] =?UTF-8?q?docs:=20Newtonsoft=20=5FtextWriter=20is?= =?UTF-8?q?=20not=20soundly=20auto-fixable=20=E2=80=94=20shelve=20the=20re?= =?UTF-8?q?cursive=20Dispose-no-op=20idea?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Investigated the deferred "recursive Dispose-is-a-no-op" feature against its only motivating case (Newtonsoft TraceJsonReader._textWriter). Reading the real JsonTextWriter source settles it: disposal is NOT a no-op type. - Close() -> base.Close() auto-completes open JSON tokens (writes closing brackets to the underlying writer), then CloseBufferAndWriter(). - CloseBufferAndWriter() RETURNS the rented _writeBuffer to _arrayPool when an ArrayPool is configured — a real pooled-buffer release (the POOL-leak class Own.NET tracks) — and closes the underlying writer. So a recursive Dispose-no-op recognizer would be unsound (it can leak a pooled buffer) or would correctly decline — either way it does not clear this finding. The _textWriter instance is benign only by instance facts (no ArrayPool set + the sink is a StringWriter), not a type-level no-op — the same reason writers are excluded from IsNoOpDisposeWrapper. Shelved as not worth building, not deferred. Sharpened the baseline entry reason and updated no-op-dispose-wrapper.md + oracle-known-fps.md root-cause #2 with the evidence. No code change. Selftests: oracle 31/31, corpus 30/30. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- corpus/oracle-fp-baseline.txt | 15 ++++++++---- docs/notes/no-op-dispose-wrapper.md | 37 ++++++++++++++++++----------- docs/notes/oracle-known-fps.md | 20 +++++++++------- 3 files changed, 45 insertions(+), 27 deletions(-) diff --git a/corpus/oracle-fp-baseline.txt b/corpus/oracle-fp-baseline.txt index 738c0506..7c964838 100644 --- a/corpus/oracle-fp-baseline.txt +++ b/corpus/oracle-fp-baseline.txt @@ -49,11 +49,16 @@ protobuf-net/protobuf-net | ProtoTranscoder.cs | OWN001 | 'sync' | non-product ( protobuf-net/protobuf-net | Page.xaml.cs | OWN001 | local 'timer' | non-product (assorted/ Silverlight perf demo); TRUE POSITIVE — `timer` is a custom IDisposable Nuxleus.Performance.Stopwatch genuinely never disposed (the using(timer) block is commented out). Baselined as non-product, not as an FP # --- JamesNK/Newtonsoft.Json ---------------------------------------------------- -# STAYS BASELINED (soundness wall): JsonTextWriter is a THIRD-PARTY wrapper — the BCL pass-through -# exemption (IsNoOpDisposeWrapper) covers only StreamReader/StreamWriter/BinaryReader/BinaryWriter, -# whose disposal contract is known. Proving JsonTextWriter.Dispose is a no-op needs body analysis we -# don't do. See docs/notes/no-op-dispose-wrapper.md. -JamesNK/Newtonsoft.Json | TraceJsonReader.cs | OWN001 | _textWriter | no-op dispose: a JsonTextWriter over an in-memory StringWriter/StringBuilder holds no unmanaged resource, so leaving it undisposed releases nothing +# STAYS BASELINED — and NOT soundly auto-fixable (investigated 2026-06-28, JsonTextWriter source). +# JsonTextWriter is NOT a no-op type to dispose: Close() runs base.Close() (auto-completes open JSON +# tokens — writes closing brackets to the underlying writer) and CloseBufferAndWriter(), which RETURNS +# its rented `_writeBuffer` to `_arrayPool` when an ArrayPool is configured — a real pooled-buffer +# release (the very POOL-leak class Own.NET tracks elsewhere) — and closes the underlying writer. So a +# recursive "Dispose-is-a-no-op" recognizer would be UNSOUND (it can leak a pooled buffer) or would +# correctly DECLINE — either way it does not clear this. The `_textWriter` instance is benign only by +# INSTANCE facts (no ArrayPool set + the sink is a StringWriter), not a type-level no-op, so it stays +# baselined — consistent with excluding writers from IsNoOpDisposeWrapper. See no-op-dispose-wrapper.md. +JamesNK/Newtonsoft.Json | TraceJsonReader.cs | OWN001 | _textWriter | benign-by-instance, NOT a no-op type: JsonTextWriter.Close() returns a (possibly pooled) write buffer and auto-completes JSON tokens; harmless here only because no ArrayPool is set and the sink is a StringWriter — kept baselined (not soundly auto-fixable) JamesNK/Newtonsoft.Json | JsonSerializer.cs | OWN001 | serializer.Error | intra-call self-subscription: the serializer is freshly created from the same JsonSerializerSettings whose .Error handler it subscribes; source and handler are co-lifetimed # --- JoshClose/CsvHelper -------------------------------------------------------- diff --git a/docs/notes/no-op-dispose-wrapper.md b/docs/notes/no-op-dispose-wrapper.md index d3457381..02497796 100644 --- a/docs/notes/no-op-dispose-wrapper.md +++ b/docs/notes/no-op-dispose-wrapper.md @@ -55,22 +55,31 @@ The motivating oracle finding was Newtonsoft.Json's `TraceJsonReader._textWriter ```csharp _sw = new StringWriter(CultureInfo.InvariantCulture); -_textWriter = new JsonTextWriter(_sw); // a no-op in truth — but JsonTextWriter is third-party +_textWriter = new JsonTextWriter(_sw); // looks like a no-op — but JsonTextWriter is NOT a no-op type ``` -Structurally identical, but `JsonTextWriter` is a **third-party** wrapper (`JsonWriter`, -not a BCL `TextWriter`). We cannot prove *its* `Dispose` is pass-through without modelling -its body — and asserting a no-op we cannot prove is exactly the unsound over-reach that -sank the static-class subscriber exemption (see -[`oracle-known-fps.md` → Rejected approaches](oracle-known-fps.md)). So Newtonsoft's -`_textWriter` stays a **baselined** finding in -[`oracle-fp-baseline.txt`](../../corpus/oracle-fp-baseline.txt), not a silent drop. - -Retiring that baseline entry would need a general, recursive "Dispose is a no-op" -analysis over a first-party type's body (it disposes only dispose-optional members and -holds no unmanaged handle). That is a larger, higher-risk capability; until it lands, only -the BCL pass-through adapters — whose disposal contract is documented and fixed — are -exempted. +It looks structurally identical to the BCL reader case, but a 2026-06-28 read of the real +`JsonTextWriter` source shows it is **not a no-op type to dispose**: + +- `Close()` runs `base.Close()` — which **auto-completes open JSON tokens** (writes closing + brackets to the underlying writer) — then `CloseBufferAndWriter()`; +- `CloseBufferAndWriter()` **returns its rented `_writeBuffer` to `_arrayPool`** when an + `ArrayPool` is configured — a real **pooled-buffer release** (the very POOL-leak class + Own.NET tracks elsewhere) — and closes the underlying writer. + +So disposal has genuine side effects (a conditional pooled-buffer return, token-completion +writes). This is the same reason we **exclude writers** (`StreamWriter`/`BinaryWriter`) from +`IsNoOpDisposeWrapper` — a writer's `Dispose` does real work. A recursive "Dispose is a +no-op" recognizer over the body would therefore either be **unsound** (it can leak a pooled +buffer) or correctly **decline** — so it would not clear this finding regardless. Asserting a +no-op we cannot prove is exactly the unsound over-reach that sank the static-class subscriber +exemption (see [`oracle-known-fps.md` → Rejected approaches](oracle-known-fps.md)). + +The `_textWriter` *instance* is benign only by **instance facts** — no `ArrayPool` is set on +it, and its sink is a `StringWriter` — not by any type-level no-op property. So it stays a +**baselined** finding in [`oracle-fp-baseline.txt`](../../corpus/oracle-fp-baseline.txt), not +a silent drop, and the "recursive Dispose-no-op analysis" idea is **shelved as not worth +building** (it wouldn't soundly clear the one case that motivated it). ## Corpus diff --git a/docs/notes/oracle-known-fps.md b/docs/notes/oracle-known-fps.md index ca77f401..31f2496d 100644 --- a/docs/notes/oracle-known-fps.md +++ b/docs/notes/oracle-known-fps.md @@ -178,14 +178,18 @@ entry and let the oracle re-confirm clean. native/extra resource), and a path that builds `new StreamReader(path)` (a real file handle) keeps the field flagged. Corpus fixture `field-noop-dispose-wrapper`; full rationale - [`no-op-dispose-wrapper.md`](no-op-dispose-wrapper.md). **Still open — the soundness - wall:** Newtonsoft's `_textWriter` is a `JsonTextWriter` over a `StringWriter` — - structurally the same no-op, but `JsonTextWriter` is a **third-party** wrapper whose - `Dispose` we cannot prove is pass-through without modelling its body. Suppressing it - would be the same unsound over-reach as the rejected static-class exemption, so it - **stays baselined**. Retiring it needs a general recursive "Dispose-is-a-no-op" body - analysis (a first-party type that disposes only dispose-optional members and holds no - unmanaged handle) — larger and higher-risk; deferred. Note the NLog `_xmlSource` / + [`no-op-dispose-wrapper.md`](no-op-dispose-wrapper.md). **Stays baselined — and NOT + soundly auto-fixable (investigated 2026-06-28):** Newtonsoft's `_textWriter` is a + `JsonTextWriter` over a `StringWriter`, but reading the real `JsonTextWriter` source shows + it is **not a no-op type** — `Close()` runs `base.Close()` (auto-completes open JSON + tokens, writing closing brackets) and `CloseBufferAndWriter()`, which **returns its rented + `_writeBuffer` to `_arrayPool`** when an `ArrayPool` is set (a real pooled-buffer release, + the POOL-leak class we track) and closes the writer. So a recursive "Dispose-is-a-no-op" + recognizer would be **unsound** (it can leak a pooled buffer) or correctly **decline** — + either way it would not clear this. The instance is benign only by **instance facts** (no + `ArrayPool` set + the sink is a `StringWriter`), not a type-level no-op — the same reason + we exclude writers from `IsNoOpDisposeWrapper`. The recursive-analysis idea is therefore + **shelved as not worth building**, not merely deferred. Note the NLog `_xmlSource` / `_reusable*Stream` reals are also third-party wrappers (`CharEnumerator`, `ReusableStreamCreator`) of the same benign shape and stay **kept visible** for the same reason — we can't prove their disposal is a no-op, so we don't silently drop them. From 7daf0aa0873dd77a0661dceb43f0a518209acecf Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 30 Jun 2026 20:46:50 +0000 Subject: [PATCH 06/13] =?UTF-8?q?docs:=20ROADMAP=20M2=20=E2=80=94=20inject?= =?UTF-8?q?ed-source=20subscription=20migration=20is=20shipped=20(di=5Fsou?= =?UTF-8?q?rce=5Flife)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Remaining: migrating the injected-source tier" note was stale. Investigation (ownir.py + test_ownir.py, 208/208 green) confirms the migration shipped via the DI graph (P-006 + P-004): an injected `+=` whose source_type resolves in the registration graph reroutes through the region engine — source proven to outlive the subscriber escalates OWN001-warning to OWN014 (error), a co-lifetimed/shorter source is refuted and dropped, and an unresolved (non-DI) source stays the honest OWN001 warning. The residual is by design; the curated "app-lived source type" escalation (MVVM messengers) is deliberately deferred as FP-prone — modern WeakReferenceMessenger holds recipients weakly, so a type-name allowlist would mis-flag the safe case. Doc-only; reflects shipped behavior. No code change. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- docs/ROADMAP.md | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 8dc7db56..d94ab173 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -186,9 +186,21 @@ architectural strictness, and the borrow-checker showcase): the subscriber stays silent (no false positive) where the token tier only warns, and a released `-=` mitigates the capture. Proven by the `capture` fixture, the `StaticEventEscapeViewModel` sample (CI `wpf-extractor` → OWN014), and the - `corpus/wpf/systemevents-region-escape` reduction (P-004 WPF005 ✅). Remaining: - migrating the *injected*-source subscription tier (today an honest OWN001 - warning) once lifetime modelling can prove or refute those sources. + `corpus/wpf/systemevents-region-escape` reduction (P-004 WPF005 ✅). The + **injected-source tier migration is now shipped** via the DI graph (P-006 + P-004, + `di_source_life`): an injected `+=` whose `source_type` resolves in the registration + graph reroutes through the region engine — a source proven to strictly **outlive** the + subscriber escalates the OWN001 *warning* to **OWN014** (a proven captive/region escape, + error-tier), a source **co-lifetimed or shorter** is *refuted* and dropped silently, and + an **unresolved** source (not in the DI graph) correctly stays the honest OWN001 warning + ("may outlive"). Covered by `test_ownir.py` (208/208 bridge checks, incl. the DI-sourced + escape + the "no DI info → stays a warning" regression). Residual is by design, not a gap: + a non-DI injected source has no provable lifetime, so warning is the honest verdict. + Escalating *those* by a curated "known app-lived source type" allowlist (e.g. MVVM + `IMessenger`/`IEventAggregator`) is **deliberately deferred as FP-prone** — modern + CommunityToolkit `WeakReferenceMessenger` holds recipients *weakly* (no leak), so a + type-name allowlist would mis-flag the safe case; it needs weak-vs-strong reference + evidence we don't model. 3. **DI lifetimes** — registration + constructor graph; captive dependency (P-006). ◑ *In progress* — DI001 lands end to end: the C# extractor builds the registration + constructor graph from `Add{Singleton,Scoped,Transient}` (generic From 167ba4de85baed1085a8f84070e92bb1566e6d81 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 1 Jul 2026 02:20:22 +0000 Subject: [PATCH 07/13] =?UTF-8?q?oracle:=20TEMP=20dev-only=20fresh-repo=20?= =?UTF-8?q?sweep=20=E2=80=94=20target=201=20Dapper=20(ADO.NET=20niche)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Point the push-triggered oracle at DapperLib/Dapper to look for new FP classes / recall gaps in our target niche (ADO.NET owned-API recognition). Dev-only; reverted after the sweep. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- .github/workflows/oracle.yml | 1 + corpus/oracle-target.txt | 17 +++++------------ 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/.github/workflows/oracle.yml b/.github/workflows/oracle.yml index 0a290055..cfc3fc66 100644 --- a/.github/workflows/oracle.yml +++ b/.github/workflows/oracle.yml @@ -45,6 +45,7 @@ on: push: branches: - claude/zen-pasteur-76hfs1 + - claude/mos-ownership-summary-n3q3j4 paths: - corpus/oracle-target.txt diff --git a/corpus/oracle-target.txt b/corpus/oracle-target.txt index 52cb6d64..b5f77b84 100644 --- a/corpus/oracle-target.txt +++ b/corpus/oracle-target.txt @@ -3,15 +3,8 @@ # or "local:" (a fixture copied into the oracle's target/). # Optional lines: ref=, paths=, build=, include_tests=. Dev-branch only. # -# Cross-tool oracle on a Linux-buildable fixture, so ALL THREE tools run (Infer# -# included — ScreenToGif's WPF won't build on Linux). Re-run after the OWN014 -# migration (PR #35): the fixture's SUBSCRIPTION leak (#1 — SystemEvents.Display -# SettingsChanged += with no -=) is now lowered by the extractor to a `capture` -# fact and reported as OWN014 (region escape), not OWN001; OWN014 was added to the -# comparator's OWN_LEAK set. So #1 must STILL land "Own.NET-only" (CodeQL / Infer# -# have no event-subscription-leak query) — this run confirms the migration kept the -# differentiation, on real cross-tool output rather than the selftest's fixtures. -# Expect: #1 Own.NET-only (now OWN014), #2/#3/#4 dispose leaks in "Agree", 0 -# oracle-only. -local:corpus/fixtures/systemevents-console -build=SystemEventsLeak.csproj +# TEMPORARY (revert before any PR): fresh-repo oracle sweep for new FP classes / recall gaps. +# Target 1: DapperLib/Dapper — an ADO.NET micro-ORM that exercises our owned-API recognition +# (DbConnection/DbCommand.CreateCommand/ExecuteReader → owned IDisposable), our target niche. +# Triaged: product code only (include_tests default false). See docs/notes/oracle-known-fps.md. +DapperLib/Dapper From 4589a94bb8cce6cd60fbe5629122a5944b05e0a0 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 1 Jul 2026 02:29:05 +0000 Subject: [PATCH 08/13] oracle: sweep target 2 StackExchange.Redis (Dapper was clean) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dapper: 0 own-only findings — ADO recognition added no noise. Oracle-only was Infer# WrappedBasicReader ownership-transfer FPs + CodeQL dispose-on-throw honest-skips (huge SqlMapper methods). No new FP class. Next: Redis (subscription tokens / ConnectionMultiplexer / timers). Dev-only; reverted after the sweep. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- corpus/oracle-target.txt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/corpus/oracle-target.txt b/corpus/oracle-target.txt index b5f77b84..e703162a 100644 --- a/corpus/oracle-target.txt +++ b/corpus/oracle-target.txt @@ -4,7 +4,8 @@ # Optional lines: ref=, paths=, build=, include_tests=. Dev-branch only. # # TEMPORARY (revert before any PR): fresh-repo oracle sweep for new FP classes / recall gaps. -# Target 1: DapperLib/Dapper — an ADO.NET micro-ORM that exercises our owned-API recognition -# (DbConnection/DbCommand.CreateCommand/ExecuteReader → owned IDisposable), our target niche. -# Triaged: product code only (include_tests default false). See docs/notes/oracle-known-fps.md. -DapperLib/Dapper +# Target 1 (done, clean): DapperLib/Dapper — 0 own-only; oracle-only = Infer# WrappedBasicReader +# ownership-transfer FPs + CodeQL dispose-on-throw honest-skips. No new FP class. +# Target 2: StackExchange/StackExchange.Redis — heavy Subscribe tokens / ConnectionMultiplexer +# IDisposable / timers; likelier to exercise subscription + disposable-field detectors. +StackExchange/StackExchange.Redis From 4351f4aeed7ae104e0007e315ea6651b89a68967 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 1 Jul 2026 04:27:07 +0000 Subject: [PATCH 09/13] extractor: pooled buffer transferred to a wrapper via an escaping local is not a leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Oracle sweep (StackExchange.Redis) surfaced a false-positive class: a rented ArrayPool buffer handed to `new Wrapper(buf)` that is bound to a LOCAL which then leaves the method — `var lease = new Lease(arr); return lease;` (Lease.Create) and `var r = new ScanResult(keys); SetResult(m, r);` (RedisServer scan) — was flagged OWN001 "rented but never returned". The buffer is transferred to the wrapper (which owns the Return), not leaked. PassedToEscapingCtor already exempts the DIRECT `return new Wrapper(buf)` / `_field = new Wrapper(buf)`. The Codex one-level rule deliberately kept a wrapper-in-a-local as a borrow (a method-scoped wrapper that never returns the buffer is a real leak). WrapperLocalEscapes adds the one-extra-hop case while preserving that: the `new Wrapper(buf)` bound to a local `w` is a transfer only when `w` provably leaves the method (`return w` / ` = w` / `w` as a call argument); a wrapper local that stays method-scoped still leaks. Corpus fixture pool-transfer-via-wrapper-local: before.cs (wrapper dropped method-scoped -> OWN001 caught) / after.cs (wrapper returned -> clean). Also re-runs the Redis oracle (oracle-target bump) to confirm the 4 own-only FPs clear. corpus 31/31, ruff clean. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- corpus/oracle-target.txt | 5 +- .../pool-transfer-via-wrapper-local/after.cs | 23 +++++++++ .../pool-transfer-via-wrapper-local/before.cs | 24 +++++++++ .../pool-transfer-via-wrapper-local/case.own | 19 +++++++ .../expected-diagnostics.txt | 1 + .../pool-transfer-via-wrapper-local/notes.md | 39 +++++++++++++++ frontend/roslyn/OwnSharp.Extractor/Program.cs | 50 ++++++++++++++++++- 7 files changed, 158 insertions(+), 3 deletions(-) create mode 100644 corpus/real-world/pool-transfer-via-wrapper-local/after.cs create mode 100644 corpus/real-world/pool-transfer-via-wrapper-local/before.cs create mode 100644 corpus/real-world/pool-transfer-via-wrapper-local/case.own create mode 100644 corpus/real-world/pool-transfer-via-wrapper-local/expected-diagnostics.txt create mode 100644 corpus/real-world/pool-transfer-via-wrapper-local/notes.md diff --git a/corpus/oracle-target.txt b/corpus/oracle-target.txt index e703162a..7800dde9 100644 --- a/corpus/oracle-target.txt +++ b/corpus/oracle-target.txt @@ -6,6 +6,7 @@ # TEMPORARY (revert before any PR): fresh-repo oracle sweep for new FP classes / recall gaps. # Target 1 (done, clean): DapperLib/Dapper — 0 own-only; oracle-only = Infer# WrappedBasicReader # ownership-transfer FPs + CodeQL dispose-on-throw honest-skips. No new FP class. -# Target 2: StackExchange/StackExchange.Redis — heavy Subscribe tokens / ConnectionMultiplexer -# IDisposable / timers; likelier to exercise subscription + disposable-field detectors. +# Target 2: StackExchange/StackExchange.Redis — surfaced the pooled-buffer-transfer-via-escaping- +# local FP class (Lease.cs:42, RedisServer.cs:917). RE-RUN to confirm the WrapperLocalEscapes fix +# clears those 4 own-only findings (expect Lease.cs / RedisServer.cs `keys`/`arr` gone from own-only). StackExchange/StackExchange.Redis diff --git a/corpus/real-world/pool-transfer-via-wrapper-local/after.cs b/corpus/real-world/pool-transfer-via-wrapper-local/after.cs new file mode 100644 index 00000000..a4f71bc8 --- /dev/null +++ b/corpus/real-world/pool-transfer-via-wrapper-local/after.cs @@ -0,0 +1,23 @@ +using System; +using System.Buffers; + +sealed class Holder : IDisposable +{ + readonly int[] _buf; + public Holder(int[] buf) => _buf = buf; + public void Dispose() => ArrayPool.Shared.Return(_buf); +} + +static class Demo +{ + // FIX: the rented buffer is wrapped into a local Holder that is RETURNED — ownership transfers to + // the Holder (which Returns the buffer on Dispose), so the buffer does not leak here. The extractor + // follows the buffer through the wrapper local because that local provably leaves the method + // (`return holder`), so it stays silent. Mirrors StackExchange.Redis Lease.Create. + public static Holder Run(int n) + { + var buf = ArrayPool.Shared.Rent(n); + var holder = new Holder(buf); + return holder; + } +} diff --git a/corpus/real-world/pool-transfer-via-wrapper-local/before.cs b/corpus/real-world/pool-transfer-via-wrapper-local/before.cs new file mode 100644 index 00000000..004dfda6 --- /dev/null +++ b/corpus/real-world/pool-transfer-via-wrapper-local/before.cs @@ -0,0 +1,24 @@ +using System; +using System.Buffers; + +// A wrapper that takes ownership of a rented buffer and returns it to the pool on Dispose. +sealed class Holder : IDisposable +{ + readonly int[] _buf; + public Holder(int[] buf) => _buf = buf; + public void Dispose() => ArrayPool.Shared.Return(_buf); +} + +static class Demo +{ + // BUG: the rented buffer is wrapped into a LOCAL Holder that never leaves the method — it is + // dropped (never disposed, never returned, never handed out) — so the buffer is genuinely never + // returned to the pool. A real pooled-buffer leak (OWN001). The wrapper being method-scoped is + // exactly the case the transfer exemption must NOT cover. + public static void Run(int n) + { + var buf = ArrayPool.Shared.Rent(n); + var holder = new Holder(buf); // method-scoped; dropped -> buffer leaks + _ = holder.GetHashCode(); + } +} diff --git a/corpus/real-world/pool-transfer-via-wrapper-local/case.own b/corpus/real-world/pool-transfer-via-wrapper-local/case.own new file mode 100644 index 00000000..5df323f3 --- /dev/null +++ b/corpus/real-world/pool-transfer-via-wrapper-local/case.own @@ -0,0 +1,19 @@ +// OwnLang model of a rented ArrayPool buffer that (in the fix) is transferred to a wrapper which +// leaves the method through a local — StackExchange.Redis Lease.Create (`var lease = new +// Lease(arr); return lease;`) and RedisServer scan (`var r = new ScanResult(keys); SetResult(m, r)`). +// The before.cs wraps the buffer in a METHOD-SCOPED Holder that is dropped, so the buffer is never +// returned -> a real leak, modelled here as an acquire with no `release` (OWN001). The after.cs +// returns the wrapper, so the extractor follows the transfer through the escaping local and stays +// silent (extractor-level; the before.cs/after.cs benchmark exercises it end to end). See notes.md. +module Corpus +resource Buffer { + acquire rent + release give + emit_type "int[]" + emit_acquire "ArrayPool.Shared.Rent({args})" + emit_release "ArrayPool.Shared.Return({0})" +} +fn Run(n: int) { + let buf = acquire Buffer(n); // ArrayPool.Rent; wrapped in a method-scoped Holder, never returned + // no `release buf;` -> OWN001 +} diff --git a/corpus/real-world/pool-transfer-via-wrapper-local/expected-diagnostics.txt b/corpus/real-world/pool-transfer-via-wrapper-local/expected-diagnostics.txt new file mode 100644 index 00000000..ed2a1929 --- /dev/null +++ b/corpus/real-world/pool-transfer-via-wrapper-local/expected-diagnostics.txt @@ -0,0 +1 @@ +OWN001 diff --git a/corpus/real-world/pool-transfer-via-wrapper-local/notes.md b/corpus/real-world/pool-transfer-via-wrapper-local/notes.md new file mode 100644 index 00000000..dffe5367 --- /dev/null +++ b/corpus/real-world/pool-transfer-via-wrapper-local/notes.md @@ -0,0 +1,39 @@ +# pool-transfer-via-wrapper-local + +A rented `ArrayPool` buffer handed to a **wrapper object bound to a local**, where that local +then **leaves the method** — `var w = new Wrapper(buf); return w;` / `Sink(m, w);` — is an ownership +**transfer** (the wrapper carries the buffer out and is responsible for `Return`), not a leak here. +Generalises the existing direct `return new Wrapper(buf)` / `_field = new Wrapper(buf)` transfer to +the one-extra-hop case an intermediate local introduces. + +Mined on **StackExchange.Redis** (2026-07-01 oracle sweep): + +- `Lease.Create` — `var arr = ArrayPool.Shared.Rent(length); var lease = new Lease(arr, + length); … return lease;` +- `RedisServer` scan — `keys = ArrayPool.Shared.Rent(count); … var r = new ScanResult( + cursor, keys, count, true); SetResult(message, r);` + +Both were false `OWN001` "rented but never returned" — the buffer is transferred to `Lease`/ +`ScanResult`, which owns the `Return`. + +- **before.cs** — the buffer is wrapped into a **method-scoped** `Holder` that is dropped (never + disposed / returned / handed out) → the buffer genuinely leaks → `OWN001` (the bug is caught; the + transfer exemption must **not** cover a wrapper that never leaves the method). +- **after.cs** — the `Holder` local is **returned** → the buffer transfers out with it → **clean**. + +## Recognition rule + +`PassedToEscapingCtor` already exempts a pooled buffer passed to a constructor whose result is the +direct `return` value or is assigned to a real field. `WrapperLocalEscapes` adds the one-extra-hop +case: the `new Wrapper(buf)` is bound to a **local** `w`, and `w` provably leaves the method — +`return w`, ` = w`, or `w` handed as a **call argument**. The Codex one-level rule is +preserved for the non-escaping case: a wrapper local that stays method-scoped is a borrow and the +buffer still leaks. + +## Honesty caveat + +Precision-first, mirroring the direct-transfer rule: we cannot prove the wrapper actually `Return`s +the buffer, so a wrapper that escapes but silently drops the buffer would be a (rare) missed leak. +The `w`-as-call-argument signal is the loosest — a `Sink(w)` that merely reads `w` and drops it would +be exempted — but handing a buffer-owning wrapper to a call is a transfer in idiomatic pooling code, +and erring toward no-false-positive matches the existing transfer stance. diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index bc4baca3..9a700d3d 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -2103,7 +2103,55 @@ static bool PassedToEscapingCtor(IdentifierNameSyntax idn, SemanticModel model) { Parent: BaseObjectCreationExpressionSyntax oce } } && (oce.Parent is ReturnStatementSyntax || (oce.Parent is AssignmentExpressionSyntax a && a.Right == oce - && model.GetSymbolInfo(a.Left).Symbol is IFieldSymbol)); + && model.GetSymbolInfo(a.Left).Symbol is IFieldSymbol) + // One extra hop: `var w = new Wrapper(…, buf, …); … return w / _field = w / Sink(…, w, …)`. + // The buffer transfers to the wrapper, which then ESCAPES through the local. Mined FP on + // StackExchange.Redis (Lease.Create `return new Lease(arr)` via a local; RedisServer scan + // `new ScanResult(keys)` handed to SetResult). The Codex one-level rule is PRESERVED for the + // non-escaping case: a wrapper local that never leaves the method (and never Returns the + // buffer) still leaks — WrapperLocalEscapes returns false there, so it stays flagged. + || WrapperLocalEscapes(oce, model)); + +// The object-creation `oce` (`new Wrapper(…, buf, …)`) is bound to a LOCAL that itself provably +// leaves the enclosing method — `var w = new Wrapper(buf); … return w` / `this._f = w` / `Sink(…, +// w, …)`. Only then is the buffer an ownership TRANSFER (the wrapper carries it out); a wrapper local +// that stays method-scoped is a borrow and the buffer still leaks (Codex). Precision-first, mirroring +// the direct `return new Wrapper(buf)` rule: we cannot prove the wrapper Returns the buffer, but a +// pooled buffer handed to an escaping owner is a transfer, not a leak here. +static bool WrapperLocalEscapes(BaseObjectCreationExpressionSyntax oce, SemanticModel model) +{ + // Bind the `new` to a local symbol: `var w = new …` (declarator) or `w = new …` (local assign). + ISymbol? w = oce.Parent switch + { + EqualsValueClauseSyntax { Parent: VariableDeclaratorSyntax vd } => model.GetDeclaredSymbol(vd), + AssignmentExpressionSyntax { Right: var r } asg when r == oce + && asg.Left is IdentifierNameSyntax lid + && model.GetSymbolInfo(lid).Symbol is ILocalSymbol ls => ls, + _ => null, + }; + if (w is null) + return false; + var method = oce.FirstAncestorOrSelf(); + SyntaxNode? body = method?.Body ?? (SyntaxNode?)method?.ExpressionBody; + if (body is null) + return false; + foreach (var id in body.DescendantNodes().OfType()) + { + if (!SymbolEqualityComparer.Default.Equals(model.GetSymbolInfo(id).Symbol, w)) + continue; + // return w; + if (id.Parent is ReturnStatementSyntax) + return true; + // = w; (w on the RHS, LHS a real field — not a look-alike local) + if (id.Parent is AssignmentExpressionSyntax fa && fa.Right == id + && model.GetSymbolInfo(fa.Left).Symbol is IFieldSymbol) + return true; + // Sink(…, w, …); — w handed as a call argument (the wrapper, and thus the buffer, leaves) + if (id.Parent is ArgumentSyntax { Parent: ArgumentListSyntax { Parent: InvocationExpressionSyntax } }) + return true; + } + return false; +} // Is `t` the System.Buffers.MemoryPool type — the Dispose-based pool. Mirrors IsArrayPoolType // (checked on the resolved symbol, so an aliased/injected `MemoryPool` receiver binds and a From 35d2eed1f3170bf6669cfc8ecc9772830472a7b9 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 1 Jul 2026 15:51:54 +0000 Subject: [PATCH 10/13] extractor: a bare-LOCAL assignment rent is not a field rent (syntactic POOL001 pass) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second Redis pooled-buffer FP: RedisServer scan does `RedisKey[] keys; … keys = ArrayPool.Shared.Rent(count); … new ScanResult(cursor, keys, count, true)`. The buffer is transferred to ScanResult, which owns the Return (its Recycle() -> ArrayPool.Return, called by CursorEnumerable on ProcessReply/ Dispose) — confirmed in source. So OWN001 "rented but never returned" was a false positive. Root cause: the syntactic POOL001 pass classifies an assignment-target rent via FieldName(asg.Left), which is purely syntactic (returns the identifier text for ANY target). A bare local (`keys = Rent()`, keys pre-declared) was thus treated as a field-backed rent with isField=true and NO escape/transfer analysis. The pass is meant for real fields (rented in ctor, Returned class-wide in Dispose); gate the assignment arm on `model.GetSymbolInfo(asg.Left).Symbol is IFieldSymbol` so a bare-local assignment rent is not flagged here. Such a local belongs to the flow pass, which does escape analysis but does not track the assignment form — so it is honestly untracked rather than flagged unsoundly (precision-first; the rare bare-assign-then-dropped local leak is a recall gap, not a false positive). No sample regression (all sample assignment rents are `this.field = Rent()` fields or `var`/`T[] x = Rent()` declarations). Re-runs the Redis oracle to confirm RedisServer.cs:917 clears. corpus 31/31. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- corpus/oracle-target.txt | 8 +++++--- frontend/roslyn/OwnSharp.Extractor/Program.cs | 19 ++++++++++++++++--- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/corpus/oracle-target.txt b/corpus/oracle-target.txt index 7800dde9..cace9161 100644 --- a/corpus/oracle-target.txt +++ b/corpus/oracle-target.txt @@ -6,7 +6,9 @@ # TEMPORARY (revert before any PR): fresh-repo oracle sweep for new FP classes / recall gaps. # Target 1 (done, clean): DapperLib/Dapper — 0 own-only; oracle-only = Infer# WrappedBasicReader # ownership-transfer FPs + CodeQL dispose-on-throw honest-skips. No new FP class. -# Target 2: StackExchange/StackExchange.Redis — surfaced the pooled-buffer-transfer-via-escaping- -# local FP class (Lease.cs:42, RedisServer.cs:917). RE-RUN to confirm the WrapperLocalEscapes fix -# clears those 4 own-only findings (expect Lease.cs / RedisServer.cs `keys`/`arr` gone from own-only). +# Target 2: StackExchange/StackExchange.Redis. WrapperLocalEscapes cleared Lease.cs:42 (var-decl +# transfer). RedisServer.cs:917 `keys` was a bare-LOCAL assignment rent (`keys = Rent()`) mis- +# classified as a field rent by the syntactic POOL001 pass (no escape analysis) — the ScanResult it +# is handed to owns the Return (Recycle()). RE-RUN #2 to confirm the IFieldSymbol gate clears the +# 3 RedisServer own-only findings (expect RedisServer.cs `keys` gone; own-only 9 -> 6). StackExchange/StackExchange.Redis diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 9a700d3d..482fbcd4 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -4024,9 +4024,22 @@ or ImplicitObjectCreationExpressionSyntax // --flow-locals; skip it here so it is not double-reported. EqualsValueClauseSyntax { Parent: VariableDeclaratorSyntax vd } => (flowLocals ? null : vd.Identifier.Text, false), - // a field/assignment rent (`_buf = pool.Rent(...)`) is NOT a - // flow candidate, so this pass keeps it in both modes. - AssignmentExpressionSyntax asg => (FieldName(asg.Left), true), + // a FIELD-backed rent (`_buf = pool.Rent(...)`) is NOT a flow + // candidate — rented in one member, Returned class-wide in another + // (Dispose) — so this pass keeps it in both modes. GATED on the target + // being a real field: `FieldName` is purely syntactic, so a bare LOCAL + // assignment (`keys = pool.Rent(count)`, keys pre-declared) would otherwise + // be mis-classified as a field rent and flagged with NO escape/transfer + // analysis — a false positive when the local is handed to an escaping owner + // (mined on StackExchange.Redis RedisServer scan: `keys = ArrayPool. + // Shared.Rent(count); … new ScanResult(cursor, keys, count, true)` — the + // ScanResult owns the Return via Recycle()). A bare-local assignment rent + // belongs to the flow pass (which does escape analysis); it does not track + // the assignment form, so such a rent is honestly not tracked here rather + // than flagged unsoundly. + AssignmentExpressionSyntax asg + when model.GetSymbolInfo(asg.Left).Symbol is IFieldSymbol + => (FieldName(asg.Left), true), _ => ((string?)null, false), }; if (name != null) From 596f20cbbf9091dda5a64d852f35a79b2ae075fe Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 1 Jul 2026 18:24:08 +0000 Subject: [PATCH 11/13] =?UTF-8?q?oracle:=20wrap=20up=202026-07-01=20sweep?= =?UTF-8?q?=20=E2=80=94=20revert=20dev=20config=20+=20document=20results?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sweep found and fixed two pooled-buffer-transfer FP classes on StackExchange.Redis (WrapperLocalEscapes for var-decl transfers, IFieldSymbol gate for bare-local assignment rents); Dapper was clean. Live Redis re-runs took own-only 10 -> 9 -> 6, all pooled-buffer FPs cleared; the remaining 6 are real/warning findings. - Reverted the dev-only oracle config (oracle.yml push branch + oracle-target.txt back to the systemevents-console fixture). - Documented the sweep in oracle-known-fps.md (Dapper clean; Redis two fixes + the remaining Timer/CTS/injected-sub dispositions). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- .github/workflows/oracle.yml | 1 - corpus/oracle-target.txt | 21 ++++++++++-------- docs/notes/oracle-known-fps.md | 39 ++++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 10 deletions(-) diff --git a/.github/workflows/oracle.yml b/.github/workflows/oracle.yml index cfc3fc66..0a290055 100644 --- a/.github/workflows/oracle.yml +++ b/.github/workflows/oracle.yml @@ -45,7 +45,6 @@ on: push: branches: - claude/zen-pasteur-76hfs1 - - claude/mos-ownership-summary-n3q3j4 paths: - corpus/oracle-target.txt diff --git a/corpus/oracle-target.txt b/corpus/oracle-target.txt index cace9161..52cb6d64 100644 --- a/corpus/oracle-target.txt +++ b/corpus/oracle-target.txt @@ -3,12 +3,15 @@ # or "local:" (a fixture copied into the oracle's target/). # Optional lines: ref=, paths=, build=, include_tests=. Dev-branch only. # -# TEMPORARY (revert before any PR): fresh-repo oracle sweep for new FP classes / recall gaps. -# Target 1 (done, clean): DapperLib/Dapper — 0 own-only; oracle-only = Infer# WrappedBasicReader -# ownership-transfer FPs + CodeQL dispose-on-throw honest-skips. No new FP class. -# Target 2: StackExchange/StackExchange.Redis. WrapperLocalEscapes cleared Lease.cs:42 (var-decl -# transfer). RedisServer.cs:917 `keys` was a bare-LOCAL assignment rent (`keys = Rent()`) mis- -# classified as a field rent by the syntactic POOL001 pass (no escape analysis) — the ScanResult it -# is handed to owns the Return (Recycle()). RE-RUN #2 to confirm the IFieldSymbol gate clears the -# 3 RedisServer own-only findings (expect RedisServer.cs `keys` gone; own-only 9 -> 6). -StackExchange/StackExchange.Redis +# Cross-tool oracle on a Linux-buildable fixture, so ALL THREE tools run (Infer# +# included — ScreenToGif's WPF won't build on Linux). Re-run after the OWN014 +# migration (PR #35): the fixture's SUBSCRIPTION leak (#1 — SystemEvents.Display +# SettingsChanged += with no -=) is now lowered by the extractor to a `capture` +# fact and reported as OWN014 (region escape), not OWN001; OWN014 was added to the +# comparator's OWN_LEAK set. So #1 must STILL land "Own.NET-only" (CodeQL / Infer# +# have no event-subscription-leak query) — this run confirms the migration kept the +# differentiation, on real cross-tool output rather than the selftest's fixtures. +# Expect: #1 Own.NET-only (now OWN014), #2/#3/#4 dispose leaks in "Agree", 0 +# oracle-only. +local:corpus/fixtures/systemevents-console +build=SystemEventsLeak.csproj diff --git a/docs/notes/oracle-known-fps.md b/docs/notes/oracle-known-fps.md index 31f2496d..dd873661 100644 --- a/docs/notes/oracle-known-fps.md +++ b/docs/notes/oracle-known-fps.md @@ -218,6 +218,45 @@ entry and let the oracle re-confirm clean. `docs-src/`) are handled per-entry in the baseline rather than by polluting the generic `_is_test_path` with repo-specific directory names. +## 2026-07-01 fresh-repo sweep (Dapper, StackExchange.Redis) + +A second cross-tool sweep over two general-purpose libraries not in the original +five, to hunt new FP classes / recall gaps. + +**DapperLib/Dapper — clean.** 0 own-only findings: the ADO.NET owned-API +recognition added no noise on a heavy 3000-line ADO codebase. Oracle-only was +Infer#'s `WrappedBasicReader` "not closed" (a wrapped reader **returned to the +caller** — the ownership-transfer FP class `oracle.md` already dings Infer# for) and +CodeQL `cs/dispose-not-called-on-throw` inside large `Read`/`NextResult` methods that +`--flow-locals` honestly skips. No product bug, no FP class. + +**StackExchange/StackExchange.Redis — two pooled-buffer-transfer FP classes, both +fixed.** A rented `ArrayPool` buffer handed to a wrapper that then escapes was flagged +`OWN001` "rented but never returned", though the wrapper owns the `Return`: + +1. `Lease.Create` — `var arr = Rent(length); var lease = new Lease(arr, length); + return lease;`. The `new Wrapper(buf)` bound to a **local that then leaves the + method** was not recognised as a transfer (the direct `return new Wrapper(buf)` rule + is one-level). **Fix:** `WrapperLocalEscapes` — a wrapper local that provably leaves + (`return w` / ` = w` / `w` as a call arg) transfers the buffer; a method-scoped + wrapper still leaks (Codex's one-level rule preserved). Corpus fixture + `pool-transfer-via-wrapper-local`. +2. `RedisServer` scan — `RedisKey[] keys; … keys = Rent(count); … new ScanResult(cursor, + keys, count, true)` (the `ScanResult` owns the `Return` via `Recycle()`, called by + `CursorEnumerable` — confirmed in source). The **bare-LOCAL assignment rent** was + mis-classified as a field rent by the syntactic POOL001 pass (`FieldName` is purely + syntactic) and flagged with no escape analysis. **Fix:** gate that pass's assignment + arm on `model.GetSymbolInfo(asg.Left).Symbol is IFieldSymbol` — a bare-local + assignment rent belongs to the flow pass (which does escape analysis but does not + track the assignment form), so it is honestly untracked rather than flagged unsoundly. + +Both verified end to end: live Redis oracle re-runs took own-only 10 → 9 → 6, all +pooled-buffer findings cleared; CI golden + corpus-benchmark green. The remaining 6 +Redis own-only are real or honest warnings — the `sentinelPrimaryReconnectTimer` / +`_inputCancel` / `_outputCancel` undisposed `Timer`/`CTS` fields (benign true positives, +like serilog's `_shutdownSignal`), the two `connection.Connection*` injected-source +subscriptions (warning tier — unknown lifetime), and the `toys/` sample host — none FPs. + ## Rejected approaches ### Static-class subscriber exemption (the CsvHelper `ConsoleHost` over-reach) From 8d747fe9fe44684e07f1bc807308be4d073ad723 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 1 Jul 2026 18:33:30 +0000 Subject: [PATCH 12/13] extractor: harden WrapperLocalEscapes against a reused wrapper local (Codex P2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WrapperLocalEscapes scanned every reference to the local across the whole method and treated any return/field-assign/call-arg as an escape of THIS `new Wrapper(buf)`. For a reused local that is unsound (a missed pool leak, not an FP): `var w = new Wrapper(buf); w = other; return w;` returns a different object, and `Sink(w); … w = new Wrapper(buf);` uses an earlier value — yet buf would be untracked. Fix: bind only the DECLARATION form (`var w = new Wrapper(buf)`, a fresh local), and bail if `w` is reassigned anywhere (`w = …` on the LHS) — then w no longer provably refers to the wrapper that owns buf, so the buffer stays flagged (sound; a false negative here would hide a real leak). Lease.Create still clears (var lease = new Lease(arr); return lease; — declared once, never reassigned). The assignment-form binding is dropped (rare; the risky reused case). corpus 31/31; ruff clean. Fixture notes updated with the reused-local caveat. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- .../pool-transfer-via-wrapper-local/notes.md | 6 +++ frontend/roslyn/OwnSharp.Extractor/Program.cs | 43 ++++++++++--------- 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/corpus/real-world/pool-transfer-via-wrapper-local/notes.md b/corpus/real-world/pool-transfer-via-wrapper-local/notes.md index dffe5367..aa8baf5f 100644 --- a/corpus/real-world/pool-transfer-via-wrapper-local/notes.md +++ b/corpus/real-world/pool-transfer-via-wrapper-local/notes.md @@ -37,3 +37,9 @@ the buffer, so a wrapper that escapes but silently drops the buffer would be a ( The `w`-as-call-argument signal is the loosest — a `Sink(w)` that merely reads `w` and drops it would be exempted — but handing a buffer-owning wrapper to a call is a transfer in idiomatic pooling code, and erring toward no-false-positive matches the existing transfer stance. + +To stay sound against a **reused local** (Codex P2), the exemption is bound to the **declaration +form only** (`var w = new Wrapper(buf)`) and **bails if `w` is reassigned anywhere** — otherwise +`var w = new Wrapper(buf); w = other; return w;` (or `Sink(w); … w = new Wrapper(buf);`) would let a +`return`/call of a *different* value untrack `buf`, missing a real leak. A reused wrapper local keeps +the buffer flagged. diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 482fbcd4..5769308b 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -2120,37 +2120,40 @@ static bool PassedToEscapingCtor(IdentifierNameSyntax idn, SemanticModel model) // pooled buffer handed to an escaping owner is a transfer, not a leak here. static bool WrapperLocalEscapes(BaseObjectCreationExpressionSyntax oce, SemanticModel model) { - // Bind the `new` to a local symbol: `var w = new …` (declarator) or `w = new …` (local assign). - ISymbol? w = oce.Parent switch - { - EqualsValueClauseSyntax { Parent: VariableDeclaratorSyntax vd } => model.GetDeclaredSymbol(vd), - AssignmentExpressionSyntax { Right: var r } asg when r == oce - && asg.Left is IdentifierNameSyntax lid - && model.GetSymbolInfo(lid).Symbol is ILocalSymbol ls => ls, - _ => null, - }; - if (w is null) + // Only the DECLARATION form `var w = new Wrapper(buf)` — a fresh local bound exactly once. The + // assignment form (`w = new Wrapper(buf)`, w pre-declared and reused) is NOT handled: a reused + // local makes "does *this* wrapper escape" flow-dependent (`Sink(w); … w = new Wrapper(buf)` uses + // an earlier value; `w = new Wrapper(buf); w = other; return w` returns a different one), which + // this syntactic scan cannot answer soundly — untracking `buf` there would MISS a real leak + // (Codex P2). So we bind only a declarator and additionally bail if `w` is reassigned anywhere. + if (oce.Parent is not EqualsValueClauseSyntax { Parent: VariableDeclaratorSyntax vd } + || model.GetDeclaredSymbol(vd) is not ILocalSymbol w) return false; var method = oce.FirstAncestorOrSelf(); SyntaxNode? body = method?.Body ?? (SyntaxNode?)method?.ExpressionBody; if (body is null) return false; + bool escapes = false; foreach (var id in body.DescendantNodes().OfType()) { if (!SymbolEqualityComparer.Default.Equals(model.GetSymbolInfo(id).Symbol, w)) continue; - // return w; + // `w` REASSIGNED (`w = …` / `w op= …`, w on the LHS): it no longer provably refers to the + // wrapper that owns `buf`, so we cannot claim an escape. Bail — the buffer stays flagged + // (sound: a false negative here would be a missed pool leak, worse than keeping the report). + if (id.Parent is AssignmentExpressionSyntax reassign && reassign.Left == id) + return false; + // return w; / = w; / Sink(…, w, …) — the wrapper (and thus `buf`) leaves the + // method. Recorded, not returned early, so a later reassignment can still veto it. if (id.Parent is ReturnStatementSyntax) - return true; - // = w; (w on the RHS, LHS a real field — not a look-alike local) - if (id.Parent is AssignmentExpressionSyntax fa && fa.Right == id - && model.GetSymbolInfo(fa.Left).Symbol is IFieldSymbol) - return true; - // Sink(…, w, …); — w handed as a call argument (the wrapper, and thus the buffer, leaves) - if (id.Parent is ArgumentSyntax { Parent: ArgumentListSyntax { Parent: InvocationExpressionSyntax } }) - return true; + escapes = true; + else if (id.Parent is AssignmentExpressionSyntax fa && fa.Right == id + && model.GetSymbolInfo(fa.Left).Symbol is IFieldSymbol) + escapes = true; + else if (id.Parent is ArgumentSyntax { Parent: ArgumentListSyntax { Parent: InvocationExpressionSyntax } }) + escapes = true; } - return false; + return escapes; } // Is `t` the System.Buffers.MemoryPool type — the Dispose-based pool. Mirrors IsArrayPoolType From a1f618ea2e0ed68f571afc6ae56c5f5ad4ee2096 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 1 Jul 2026 18:37:00 +0000 Subject: [PATCH 13/13] extractor: WrapperLocalEscapes must not descend into nested callables (CodeRabbit) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit body.DescendantNodes() walked into nested lambdas / local functions, so a use of the wrapper local inside a callable that never leaves the method (`Action a = () => Sink(w);`) counted as an escape and untracked buf — hiding a real pool leak. Prune descent into AnonymousFunctionExpressionSyntax / LocalFunctionStatementSyntax so only the wrapper's top-level uses drive the escape decision. corpus 31/31; ruff clean. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- frontend/roslyn/OwnSharp.Extractor/Program.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 5769308b..dce47c19 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -2134,7 +2134,13 @@ static bool WrapperLocalEscapes(BaseObjectCreationExpressionSyntax oce, Semantic if (body is null) return false; bool escapes = false; - foreach (var id in body.DescendantNodes().OfType()) + // Do NOT descend into nested lambdas / local functions: a use of `w` captured inside a + // callable that itself never leaves the method (`Action a = () => Sink(w);`) is not an escape + // of `w`, and counting it would untrack `buf` and hide a real leak (CodeRabbit). The `w` + // declaration and its top-level uses stay in scope; only nested-callable bodies are pruned. + foreach (var id in body.DescendantNodes(n => + n is not (AnonymousFunctionExpressionSyntax or LocalFunctionStatementSyntax)) + .OfType()) { if (!SymbolEqualityComparer.Default.Equals(model.GetSymbolInfo(id).Symbol, w)) continue;