From 3179175cd92631ae55cbdcdfe86fe1e68e6a4324 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 12:05:54 +0000 Subject: [PATCH 01/16] ci(oracle): enable the dev push-trigger on this branch (dev-only, not for main) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The oracle workflow_dispatch is blocked for the automation token (403). Point the existing dev-only push fallback at this branch so a corpus/oracle-target.txt bump runs the cross-tool oracle. Dev-loop only — not to be merged to main. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- .github/workflows/oracle.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/oracle.yml b/.github/workflows/oracle.yml index 0d64a80a..e410409b 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 From 6f072dc3568ec3479d9f71cc313fa35c48cd09f8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 12:06:19 +0000 Subject: [PATCH 02/16] oracle: run cross-tool oracle on JamesNK/Newtonsoft.Json Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- corpus/oracle-target.txt | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/corpus/oracle-target.txt b/corpus/oracle-target.txt index 52cb6d64..a60da526 100644 --- a/corpus/oracle-target.txt +++ b/corpus/oracle-target.txt @@ -1,17 +1,3 @@ -# Target for the push-triggered oracle (.github/workflows/oracle.yml, push path). -# Format = mine-target.txt: first non-comment line is "owner/repo", an https URL, -# 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 +# oracle target (dev push-trigger). Product code only. +JamesNK/Newtonsoft.Json +include_tests=false From 12b1c593bf1bef39a2ee9fefefc21e054f3dd338 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 12:06:20 +0000 Subject: [PATCH 03/16] oracle: run cross-tool oracle on JoshClose/CsvHelper Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- corpus/oracle-target.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/corpus/oracle-target.txt b/corpus/oracle-target.txt index a60da526..d1ae57b9 100644 --- a/corpus/oracle-target.txt +++ b/corpus/oracle-target.txt @@ -1,3 +1,3 @@ # oracle target (dev push-trigger). Product code only. -JamesNK/Newtonsoft.Json +JoshClose/CsvHelper include_tests=false From d0560b1f2fe0aa3635a9618cad4bf2ba09aa2ae6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 12:06:21 +0000 Subject: [PATCH 04/16] oracle: run cross-tool oracle on serilog/serilog Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- corpus/oracle-target.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/corpus/oracle-target.txt b/corpus/oracle-target.txt index d1ae57b9..47c0c431 100644 --- a/corpus/oracle-target.txt +++ b/corpus/oracle-target.txt @@ -1,3 +1,3 @@ # oracle target (dev push-trigger). Product code only. -JoshClose/CsvHelper +serilog/serilog include_tests=false From 47a4c0f7ebb163af0531596b109a040224885481 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 12:06:22 +0000 Subject: [PATCH 05/16] oracle: run cross-tool oracle on NLog/NLog Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- corpus/oracle-target.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/corpus/oracle-target.txt b/corpus/oracle-target.txt index 47c0c431..a3dd6e83 100644 --- a/corpus/oracle-target.txt +++ b/corpus/oracle-target.txt @@ -1,3 +1,3 @@ # oracle target (dev push-trigger). Product code only. -serilog/serilog +NLog/NLog include_tests=false From ba5c7d42d660f40cc521150e2ed43435415e4796 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 12:06:23 +0000 Subject: [PATCH 06/16] oracle: run cross-tool oracle on protobuf-net/protobuf-net Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- corpus/oracle-target.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/corpus/oracle-target.txt b/corpus/oracle-target.txt index a3dd6e83..ea43855a 100644 --- a/corpus/oracle-target.txt +++ b/corpus/oracle-target.txt @@ -1,3 +1,3 @@ # oracle target (dev push-trigger). Product code only. -NLog/NLog +protobuf-net/protobuf-net include_tests=false From cd06da8aa5ad72fe5a77781294e19ba6d9f38de9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 12:34:54 +0000 Subject: [PATCH 07/16] oracle: revert dev-only push-trigger scaffolding Remove the claude/mos-ownership-summary branch from the oracle.yml push filter and restore corpus/oracle-target.txt to the systemevents-console fixture. These were temporary edits to dispatch the cross-tool oracle on five NuGet targets (Newtonsoft.Json, CsvHelper, serilog, NLog, protobuf-net); the runs are done, so the dev scaffolding comes back out. 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, 17 insertions(+), 4 deletions(-) diff --git a/.github/workflows/oracle.yml b/.github/workflows/oracle.yml index e410409b..0d64a80a 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 ea43855a..52cb6d64 100644 --- a/corpus/oracle-target.txt +++ b/corpus/oracle-target.txt @@ -1,3 +1,17 @@ -# oracle target (dev push-trigger). Product code only. -protobuf-net/protobuf-net -include_tests=false +# Target for the push-triggered oracle (.github/workflows/oracle.yml, push path). +# Format = mine-target.txt: first non-comment line is "owner/repo", an https URL, +# 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 From daa4e83f7f13f687b919c93215333cf644fea2c6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 12:58:53 +0000 Subject: [PATCH 08/16] oracle: triage five-repo own-only findings; baseline verified FPs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Triaged all 20 own-only findings from the 2026-06-27 cross-tool oracle run (Newtonsoft.Json, CsvHelper, serilog, NLog, protobuf-net) against each target's real source. 12 are false positives, 4 are true-but-some-benign catches the oracle leak query can't express (kept visible), 2 are test/sample noise. None came from the new owned-API recognition — it added no noise on third-party code. To keep re-runs actionable: - oracle_compare.py gains --baseline: an allowlist of verified FPs, matched by (repo, basename, OWN code, message-substring) NOT line number (targets drift at HEAD), moved into a separate 'Known FP (baselined)' section so the triage queue surfaces only new findings. Selftest covers the loader, name-not-line match key, repo scoping, '*' wildcard, and render (30/30). - _is_test_path gains a safe 'unittest' substring rule so camelCase test projects (protobuf's BuildToolsUnitTests) are excluded as non-product. - corpus/oracle-fp-baseline.txt seeds 13 rules (covering 14 findings) with a per-entry root cause; the workflow passes it automatically. - docs/notes/oracle-known-fps.md records every verdict and the four analyzer limitations behind the FPs, each naming the fix that would retire its entry. True positives (serilog CTS, NLog benign field leaks) are deliberately not suppressed. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- .github/workflows/oracle.yml | 5 ++ corpus/oracle-fp-baseline.txt | 47 ++++++++++ docs/notes/oracle-known-fps.md | 158 +++++++++++++++++++++++++++++++++ docs/notes/oracle.md | 11 +++ scripts/oracle_compare.py | 155 ++++++++++++++++++++++++++++++-- 5 files changed, 371 insertions(+), 5 deletions(-) create mode 100644 corpus/oracle-fp-baseline.txt create mode 100644 docs/notes/oracle-known-fps.md diff --git a/.github/workflows/oracle.yml b/.github/workflows/oracle.yml index 0d64a80a..0a290055 100644 --- a/.github/workflows/oracle.yml +++ b/.github/workflows/oracle.yml @@ -260,6 +260,11 @@ jobs: # Compare on product code by default: tests/benchmarks pollute the diff # (and Infer# only built the product project). include_tests keeps them. [[ "${INCLUDE_TESTS,,}" == "true" ]] || args+=(--exclude-tests) + # Suppress verified false positives (triaged against real source) so the + # triage queue surfaces only NEW own-only findings. Matched by name, not + # line, so it survives the target drifting at HEAD. See the file header and + # docs/notes/oracle-known-fps.md. + [[ -f corpus/oracle-fp-baseline.txt ]] && args+=(--baseline corpus/oracle-fp-baseline.txt) [[ -f infer-out/report.sarif ]] && args+=(--infersharp infer-out/report.sarif) cq=$(find codeql-out -name '*.sarif' -type f 2>/dev/null | head -1 || true) [[ -n "$cq" ]] && args+=(--codeql "$cq") diff --git a/corpus/oracle-fp-baseline.txt b/corpus/oracle-fp-baseline.txt new file mode 100644 index 00000000..e44ee4a4 --- /dev/null +++ b/corpus/oracle-fp-baseline.txt @@ -0,0 +1,47 @@ +# Cross-tool oracle — verified false-positive baseline. +# +# Each line is one own-only finding that was triaged against the target's REAL +# source (at the run commit) and confirmed NOT to be a leak. The comparator +# (scripts/oracle_compare.py --baseline) moves these out of "Own.NET only" into a +# separate "Known FP (baselined)" section, so a re-run's triage queue shows only +# genuinely-new findings. Full per-entry rationale: docs/notes/oracle-known-fps.md. +# +# Format (｜-delimited, '#' starts a full-line comment, blank lines ignored): +# | | | | +# +# The match key is (repo, basename, OWN code, message-substring) — deliberately +# NOT the line number: re-runs clone the target at its current HEAD and the lines +# drift, but the field/event/local name in the message is stable. Keep substrings +# specific enough to bind to exactly the intended finding. +# +# This file suppresses ONLY confirmed false positives. True findings stay visible +# even when benign — e.g. serilog BatchingSink._shutdownSignal (a CTS never +# Dispose()d) and NLog's XmlParser._xmlSource / FileTarget._reusable*Stream +# (fields genuinely never disposed, but wrapping managed-memory-only resources). +# Those are NOT baselined; they are real catches the oracle's leak query misses. + +# --- NLog/NLog : timers disposed through an indirection we don't trace ---------- +# Every one is released via the `WaitForDispose(this Timer, TimeSpan)` extension +# method (AsyncHelpers.cs) on a local alias of the field — an interprocedural +# custom dispose-sink the extractor's intra-Dispose() field scan can't see. Two +# also dispose only in the dispose-pattern override / lifecycle hook (Dispose(bool +# disposing), CloseTarget()), neither of which the field scan visits. +NLog/NLog | TimeoutContinuation.cs | OWN001 | _timeoutTimer | disposed via Dispose() -> StopTimer() -> Timer.WaitForDispose(); interprocedural custom dispose-sink not traced +NLog/NLog | AsyncTaskTarget.cs | OWN001 | _taskTimeoutTimer | disposed in Dispose(bool disposing) via Timer.WaitForDispose(); dispose-pattern override + custom sink not traced +NLog/NLog | AsyncTaskTarget.cs | OWN001 | _lazyWriterTimer | disposed in Dispose(bool disposing) via Timer.WaitForDispose(); dispose-pattern override + custom sink not traced +NLog/NLog | AsyncTargetWrapper.cs | OWN001 | _lazyWriterTimer | disposed in CloseTarget() -> StopLazyWriterThread() -> Timer.WaitForDispose(); lifecycle hook + custom sink not traced +NLog/NLog | BufferingTargetWrapper.cs | OWN001 | _flushTimer | disposed in CloseTarget() via Timer.WaitForDispose(); lifecycle hook + custom sink not traced + +# --- protobuf-net/protobuf-net -------------------------------------------------- +protobuf-net/protobuf-net | ProtoWriter.BufferWriter.cs | OWN001 | _nullWriter | intentional null-object kept attached for pooled reuse; Dispose() documents "don't cascade dispose to the null one" +protobuf-net/protobuf-net | ProtoTranscoder.cs | OWN001 | 'sync' | non-product (protobuf 'assorted' sample/extension tree); NetTranscoder is a long-lived singleton holding one ReaderWriterLockSlim for app lifetime +protobuf-net/protobuf-net | CommandLineOptions.cs | OWN001 | XsltMessageEncountered | non-product (assorted/ProtoGen) + self-subscription: publisher (xsltOptions) and the lambda subscriber are both owned by the same CommandLineOptions, co-lifetimed +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 + +# --- JamesNK/Newtonsoft.Json ---------------------------------------------------- +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 +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 -------------------------------------------------------- +JoshClose/CsvHelper | ConsoleHost.cs | OWN014 | AppDomain.CurrentDomain.ProcessExit | non-product (docs-src/ doc-generator) + process-lived subscriber: ConsoleHost itself lives for the whole process, so promoting it to a process-lived event source changes nothing +JoshClose/CsvHelper | ConsoleHost.cs | OWN014 | Console.CancelKeyPress | non-product (docs-src/ doc-generator) + process-lived subscriber: same — a process-lived host subscribing to a process-lived event source is not a lifetime leak diff --git a/docs/notes/oracle-known-fps.md b/docs/notes/oracle-known-fps.md new file mode 100644 index 00000000..1c60fade --- /dev/null +++ b/docs/notes/oracle-known-fps.md @@ -0,0 +1,158 @@ +# Oracle known false positives — triage of the 2026-06-27 five-repo run + +Companion to [`oracle.md`](oracle.md). On 2026-06-27 the cross-tool oracle ran +over five top-200-NuGet, general-purpose libraries — **Newtonsoft.Json, +CsvHelper, serilog, NLog, protobuf-net** — diffing Own.NET against Infer# and +CodeQL. This note records the triage of every **own-only** finding against the +target's real source, so the verdicts are durable and the [FP +baseline](../../corpus/oracle-fp-baseline.txt) that suppresses them on re-runs is +auditable. + +Headline: across all five repos, **0 own-only findings came from the new +owned-API recognition** (ADO `ExecuteReader`/`CreateCommand`, Xml/Json +`.Create`/`.Parse`, Socket `Accept`) — those libraries don't use those APIs in a +leaking shape, so the recognition extensions added **no noise on third-party +code**. Every own-only finding is a disposable-field or event-subscription +catch — Own.NET's differentiating niche, which the oracles' leak queries (local +not-disposed) structurally cannot express. `Agree = 0` on all five for the same +reason: we and the oracles occupy orthogonal niches. + +## Disposition summary + +20 own-only findings, triaged to ground truth: + +| disposition | count | what happens on re-run | +|---|---:|---| +| **Baselined FP** | 12 | 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 | + +The 12 baselined FPs + the 2 non-product-sample reals = 14 findings, covered by +**13 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. + +## Per-finding verdicts + +### NLog/NLog — 8 findings (disposable fields) + +| field / class | verdict | why | +|---|---|---| +| `_timeoutTimer` / TimeoutContinuation | **FP → baseline** | `Dispose()` → `StopTimer()` → `Timer.WaitForDispose()` (ext method) | +| `_taskTimeoutTimer` / AsyncTaskTarget | **FP → baseline** | `Dispose(bool disposing)` → `Timer.WaitForDispose()` | +| `_lazyWriterTimer` / AsyncTaskTarget | **FP → baseline** | `Dispose(bool disposing)` → `Timer.WaitForDispose()` | +| `_lazyWriterTimer` / AsyncTargetWrapper | **FP → baseline** | `CloseTarget()` → `StopLazyWriterThread()` → `Timer.WaitForDispose()` | +| `_flushTimer` / BufferingTargetWrapper | **FP → baseline** | `CloseTarget()` → `Timer.WaitForDispose()` | +| `_xmlSource` / XmlParser | **true positive → keep** | never disposed (benign: a `CharEnumerator` over a `StringReader` over a string — no unmanaged resource) | +| `_reusableFileWriteStream` / FileTarget | **true positive → keep** | never disposed (benign: `ReusableStreamCreator` over a `MemoryStream` — managed memory only) | +| `_reusableBatchFileWriteStream` / FileTarget | **true positive → keep** | never disposed (same) | + +`WaitForDispose(this Timer, TimeSpan)` (NLog `Common/AsyncHelpers.cs`) really does +dispose the timer (`Change(Infinite,Infinite)` then `Dispose()`). All five timer +FPs route disposal through it, on a **local alias** of the field +(`Interlocked.Exchange(ref _timer, null)` / `var t = _timer`), inside either a +`Dispose(bool disposing)` override or a `CloseTarget()` lifecycle hook. The +extractor's field-disposal scan only inspects the **top-level statements of the +parameterless `Dispose()`** for a direct `field.Dispose()` — so it sees none of +this. + +### protobuf-net/protobuf-net — 7 findings + +| location | verdict | why | +|---|---|---| +| `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) | +| `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 | + +The two `BuildToolsUnitTests` findings are now dropped by `--exclude-tests`: that +camelCase project name is one dot-less path segment, which the exact dot-component +guards missed, so `_is_test_path` gained a safe `unittest` substring rule. + +### serilog/serilog — 1 finding + +| location | verdict | why | +|---|---|---| +| `BatchingSink.cs` `_shutdownSignal` (CancellationTokenSource) | **true positive → keep** | `Dispose()` *and* `DisposeAsync()` only call `_shutdownSignal.Cancel()`, never `Dispose()` — a genuine (if benign) undisposed CTS | + +Not an async-dispose-tracing miss: the CTS is disposed in **neither** path. This +is a real catch the oracles' local-not-disposed query can't express, and it stays +visible. + +### JamesNK/Newtonsoft.Json — 2 findings + +| location | verdict | why | +|---|---|---| +| `TraceJsonReader.cs` `_textWriter` | **FP → baseline** | no-op dispose: a `JsonTextWriter` over an in-memory `StringWriter`/`StringBuilder` holds no unmanaged resource | +| `JsonSerializer.cs` `serializer.Error` | **FP → baseline** | intra-call self-subscription: the serializer is freshly built from the same `JsonSerializerSettings` whose `.Error` it subscribes; co-lifetimed | + +### JoshClose/CsvHelper — 2 findings + +| location | verdict | why | +|---|---|---| +| `docs-src/.../ConsoleHost.cs` `AppDomain.CurrentDomain.ProcessExit` | **FP → baseline** | process-lived subscriber (a docs-generator host) to a process-lived event source — promoting it to that lifetime is vacuous | +| `docs-src/.../ConsoleHost.cs` `Console.CancelKeyPress` | **FP → baseline** | same | + +## Root-cause categories and the fix that would retire each baseline + +The baselined FPs cluster into four analyzer limitations. Each baseline entry is a +standing request for the corresponding capability — when it lands, retire the +entry and let the oracle re-confirm clean. + +1. **Interprocedural / custom dispose-sink not traced** *(5 NLog timers, + protobuf `Page.xaml.cs` using-block).* Field-disposal recognition only scans + top-level `Dispose()` statements for a direct `field.Dispose()`. It misses the + dispose-pattern override `Dispose(bool disposing)`, `DisposeAsync`, lifecycle + hooks (`CloseTarget()`/`Close()`), helper-method indirection, disposal of a + **local alias** of the field, custom dispose-sink methods like + `WaitForDispose(this Timer)`, and the `using (preExistingLocal)` statement form. + *Fix:* extend `DisposedOwningFields` (extractor) to additional dispose entry + points + one level of intra-class call/alias following; longer-term, real + interprocedural escape/dispose analysis. The cheap, sound first step + (recognise `Dispose(bool disposing)` + `DisposeAsync` direct field disposal) is + correct and low-risk but does **not** by itself clear these NLog cases, because + their disposal is laundered through a local alias **and** a custom-named sink. + +2. **No-op `Dispose` not modelled** *(Newtonsoft `TraceJsonReader._textWriter`).* + We flag any undisposed `IDisposable` structurally, without modelling that the + concrete `Dispose` releases nothing (`StringWriter`/`StringReader`/`MemoryStream` + over managed memory). *Fix:* a small "dispose-is-a-no-op" allowlist of BCL + in-memory types for the field case. (Related, already shipped for one case: + [`cts-field-dispose-optional.md`](cts-field-dispose-optional.md).) Note the NLog + `_xmlSource` / `_reusable*Stream` reals are the *same* benign shape but are + **kept visible** — they're genuinely undisposed; only Newtonsoft's is also + structurally a no-op AND not worth surfacing. Revisit whether benign-managed + field leaks should be downgraded as a class. + +3. **Lifetime-unaware subscription** *(protobuf `XsltOptions`, Newtonsoft + `serializer.Error` self-subscriptions; CsvHelper process-lived host).* OWN014's + premise — a long-lived source outlives a shorter-lived subscriber — fails when + publisher and subscriber are **co-lifetimed** (same owner, or both freshly + created in one call) or the subscriber is **itself process-lived**. *Fix:* + compare subscriber vs. source lifetime (extend the `source: "self"` handling to + co-created locals and to process-lived hosts) rather than keying on "source is + static/injected". See [`subscription-leaks-and-profiles.md`](subscription-leaks-and-profiles.md). + +4. **Non-product trees** *(protobuf `assorted/`, CsvHelper `docs-src/`, protobuf + `BuildToolsUnitTests/`).* Sample/extension/doc-generator code that was never + meant to be production-clean. The generic `unittest` rule now covers camelCase + test projects; the remaining repo-specific sample trees (`assorted/`, + `docs-src/`) are handled per-entry in the baseline rather than by polluting the + generic `_is_test_path` with repo-specific directory names. + +## How the baseline stays honest + +- **Matched by name, not line** — `(repo, file-basename, OWN code, + message-substring)`. Re-runs clone the target at HEAD and line numbers drift; + the field/event/local name in the message does not. +- **Suppresses only confirmed FPs.** True positives (serilog CTS; NLog benign + field leaks) are never baselined — they remain the visible proof the niche + works. +- **Self-retiring.** Each entry names the fix that obsoletes it. When that lands, + delete the line; if the FP was real after all, the oracle re-surfaces it. +- **Verified in CI.** `oracle_compare.py --selftest` covers the baseline + loader, the name-not-line match key, repo scoping, the `*` wildcard, the + render, and the `unittest` path rule. diff --git a/docs/notes/oracle.md b/docs/notes/oracle.md index 25c3ac26..6fa46fc3 100644 --- a/docs/notes/oracle.md +++ b/docs/notes/oracle.md @@ -28,6 +28,17 @@ Two classes sit **outside** the three-way diff and are reported separately: - **Oracle findings outside our scope** — Infer#'s `NULL_DEREFERENCE`, thread-safety, taint, etc. Listed as context (counts by rule), not a gap. +A fourth, optional bucket keeps the triage queue actionable across re-runs: + +- **Known FP (baselined)** — own-only findings already triaged against the + target's real source and confirmed false. The + [`corpus/oracle-fp-baseline.txt`](../../corpus/oracle-fp-baseline.txt) allowlist + (passed as `--baseline`) moves them out of "own-only" so a re-run surfaces only + *new* findings, matched by name (repo + basename + OWN code + message-substring) + rather than line so it survives the target drifting at HEAD. Confirmed **false + positives only** — true-but-benign catches stay visible. The five-repo triage + that seeded it is in [`oracle-known-fps.md`](oracle-known-fps.md). + ## Why this is a fair-but-honest comparison - **Own.NET needs no build.** The Roslyn extractor reads a best-effort diff --git a/scripts/oracle_compare.py b/scripts/oracle_compare.py index 32138354..878ade45 100644 --- a/scripts/oracle_compare.py +++ b/scripts/oracle_compare.py @@ -224,9 +224,69 @@ def near(a: Finding, b: Finding) -> bool: "files_both": own_files & ora_files, "files_own_only": own_files - ora_files, "files_oracle_only": ora_files - own_files, + "baselined": [], } +@dataclass +class BaselineRule: + """One verified-false-positive allowlist entry. Matched against an own-only + Finding by (repo, file-basename, OWN code, message-substring) — deliberately + NOT by line number, because re-runs clone the target at its current HEAD and + the line numbers drift. `repo` may be `*` to apply to every target. See + corpus/oracle-fp-baseline.txt and docs/notes/oracle-known-fps.md.""" + repo: str + basename: str + rule: str + substr: str + reason: str = "" + + def matches(self, target: str, f: Finding) -> bool: + if self.repo not in ("*", "") and self.repo.lower() != (target or "").lower(): + return False + return (self.basename.lower() == f.fkey + and self.rule == f.rule + and self.substr in f.message) + + +def _load_baseline(path: str) -> list[BaselineRule]: + """Parse the FP baseline file. One rule per line: + | | | | + `#` starts a full-line comment; blank lines are ignored. The reason is free + text (surfaced in the report); the first four fields are the match key. A line + with fewer than four `|`-fields is skipped (malformed, not a silent match-all).""" + rules: list[BaselineRule] = [] + for raw in Path(path).read_text(encoding="utf-8").splitlines(): + s = raw.strip() + if not s or s.startswith("#"): + continue + parts = [p.strip() for p in s.split("|")] + if len(parts) < 4: + continue + repo, basename, rule, substr = parts[:4] + reason = parts[4] if len(parts) > 4 else "" + rules.append(BaselineRule(repo, basename, rule, substr, reason)) + return rules + + +def apply_baseline(result: dict[str, Any], target: str, + rules: list[BaselineRule]) -> int: + """Move verified-FP findings out of `own_only` into `baselined` (each paired + with its reason), so the triage queue only ever shows genuinely-new own-only + findings. Returns the number moved.""" + kept: list[tuple[Finding, list[Finding]]] = [] + base: list[tuple[Finding, str]] = [] + for f, hits in result["own_only"]: + hit = next((r for r in rules if r.matches(target, f)), None) + if hit is not None: + base.append((f, hit.reason)) + else: + kept.append((f, hits)) + result["own_only"] = kept + result["baselined"] = base + return len(base) + + def _fmt_files(s: set[str], cap: int = 12) -> str: items = sorted(s) shown = ", ".join(f"`{x}`" for x in items[:cap]) @@ -238,7 +298,7 @@ def _fmt_files(s: set[str], cap: int = 12) -> str: def render_md(result: dict[str, Any], target: str, commit: str, oracles: list[str], tol: int, own_unparsed: int = 0, excluded_tests: int = 0, exclude_tests_mode: bool = False, - max_list: int = 50) -> str: + baseline_path: str = "", max_list: int = 50) -> str: """The human-facing comparison report.""" own_leak = result["own_leak"] ora_leak = result["oracle_leak"] @@ -284,12 +344,29 @@ def render_md(result: dict[str, Any], target: str, commit: str, for f, hits in agree[:max_list] ] + baselined = result.get("baselined", []) + own_only_note = (f" — {len(baselined)} verified FP(s) baselined out, see below" + if baselined else "") out += ["", f"## Own.NET only — {len(own_only)} " - "(candidate FP, or a catch the oracle misses)", ""] + f"(candidate FP, or a catch the oracle misses{own_only_note})", ""] out += ["_(none)_"] if not own_only else [ f"- `{f.path}:{f.line}` **[{f.rule}]** {f.message}" for f, _ in own_only[:max_list] ] + if baselined: + src = baseline_path or "the FP baseline" + out += ["", f"## Known FP (baselined) — {len(baselined)} " + f"(verified false positives, suppressed via `{src}`)", "", + "These were triaged against the real source and confirmed not to be " + "leaks (the resource is disposed through an indirection we don't trace, " + "has a no-op `Dispose`, is a self-/co-lifetimed subscription, or lives " + "in non-product code). They are kept out of the triage queue so a " + "re-run surfaces only genuinely-new findings. See " + "docs/notes/oracle-known-fps.md for the full rationale per entry.", ""] + out += [f"- `{f.path}:{f.line}` **[{f.rule}]** {f.message}" + + (f" — _{reason}_" if reason else "") + for f, reason in baselined[:max_list]] + out += ["", f"## Oracle only — {len(oracle_only)} (our recall gap, or an oracle FP)", ""] out += ["_(none)_"] if not oracle_only else [ f"- `{g.path}:{g.line}` **[{g.tool}:{g.rule}]** {g.message}" @@ -326,6 +403,9 @@ def render_md(result: dict[str, Any], target: str, commit: str, "- **Own.NET only** is the triage queue — each is a candidate false positive to " "harden, *or* a real catch the oracle's leak query can't express (double-dispose, " "use-after-dispose, a non-allowlisted owning type).", + "- **Known FP (baselined)**, when present, are own-only findings already triaged " + "to ground truth and confirmed false — suppressed so re-runs surface only new " + "findings. Add or retire entries in the FP baseline file as the analyser improves.", "- **Oracle only** is our recall gap: reduce one to a minimal `.cs`, decide if it " "is in scope (interprocedural? a field? a loop/`try` shape we skip?), then model " "it or record it as a known limitation.", @@ -354,10 +434,13 @@ def to_json(result: dict[str, Any], target: str, commit: str) -> dict[str, Any]: "oracle_only": len(result["oracle_only"]), "own_unique": len(result["own_unique"]), "oracle_other": len(result["oracle_other"]), + "baselined": len(result.get("baselined", [])), }, "agree": [{"finding": _fd(f), "oracles": [_fd(h) for h in hits]} for f, hits in result["agree"]], "own_only": [_fd(f) for f, _ in result["own_only"]], + "baselined": [{"finding": _fd(f), "reason": reason} + for f, reason in result.get("baselined", [])], "oracle_only": [_fd(g) for g in result["oracle_only"]], "own_unique": [_fd(f) for f in result["own_unique"]], "oracle_other": [_fd(g) for g in result["oracle_other"]], @@ -392,9 +475,15 @@ def _is_test_path(path: str) -> bool: # guards intact — exact match for short, collision-prone names (so a single # component `SnippetEngine`/`Documentation` is NOT dropped, only an exact # `snippet`/`doc`), prefix only for the long, unambiguous plural-able ones. + # `unittest` is matched as a *substring* (not exact / dot-component): camelCase + # test projects like `BuildToolsUnitTests` are one dot-less segment, so the + # exact guards miss them — but no product directory realistically embeds + # "unittest", so the substring is safe (unlike a bare "test", which "latest" + # would trip). for part in seg.split("."): if (part in ("test", "tests", "doc", "docs", "snippet", "snippets") - or part.startswith(("benchmark", "sample", "example"))): + or part.startswith(("benchmark", "sample", "example")) + or "unittest" in part): return True return False @@ -417,6 +506,10 @@ def main(argv: list[str] | None = None) -> int: ap.add_argument("--exclude-tests", action="store_true", help="drop findings under test/benchmark/sample/example paths, " "comparing the product code only across all tools") + ap.add_argument("--baseline", default="", + help="verified-FP allowlist file; matching own-only findings are " + "moved to a separate 'Known FP (baselined)' section so re-runs " + "surface only new findings (see corpus/oracle-fp-baseline.txt)") ap.add_argument("--json", dest="json_out", default="", help="also write the structured comparison as JSON") ap.add_argument("--selftest", action="store_true", @@ -463,12 +556,21 @@ def main(argv: list[str] | None = None) -> int: "test/benchmark/sample/example paths", file=sys.stderr) result = compare(own, oracles, args.line_tol) + + baselined = 0 + if args.baseline: + rules = _load_baseline(args.baseline) + baselined = apply_baseline(result, args.target, rules) + if baselined: + print(f"--baseline: moved {baselined} verified-FP finding(s) out of " + f"own-only (from {args.baseline})", file=sys.stderr) + if args.json_out: Path(args.json_out).write_text( json.dumps(to_json(result, args.target, args.commit), indent=2), encoding="utf-8") print(render_md(result, args.target, args.commit, present, args.line_tol, - own_unparsed, excluded, args.exclude_tests)) + own_unparsed, excluded, args.exclude_tests, args.baseline)) return 0 @@ -631,7 +733,50 @@ def _selftest() -> int: fails.append(f"non-SARIF JSON masked as clean: {len(not_sarif)} findings, " f"{ns_drift} unparsed") - total = 25 + # camelCase test projects (`BuildToolsUnitTests`) are one dot-less segment, so the + # exact dot-component guards miss them; the `unittest` substring catches them while + # leaving product code (and the `latest`-style "ends in test" trap) untouched. + if not _is_test_path("src/BuildToolsUnitTests/AnalyzerTestBase.cs"): + fails.append("_is_test_path should match camelCase *UnitTests* projects") + if any(_is_test_path(p) for p in ("src/Latest/Feed.cs", "src/UnitedThings/A.cs")): + fails.append("_is_test_path should not match 'latest'/'united' as unittest") + + # --baseline: a verified-FP allowlist moves matching own-only findings into a + # separate `baselined` bucket. The match key is (repo, basename, OWN code, + # message-substring) — NEVER the line number (re-runs drift lines). own_only here + # is [b.cs OWN001 "...local 'b'..."]. + bl_rule = BaselineRule("o/r", "B.cs", "OWN001", "local 'b'", "verified FP") + r_b = compare(own, oracles, tol=3) + moved = apply_baseline(r_b, "o/r", [bl_rule]) + if moved != 1 or [f.fkey for f, _ in r_b["baselined"]] != ["b.cs"] or r_b["own_only"]: + fails.append(f"baseline: expected b.cs moved, own_only emptied; got " + f"own_only={[f.fkey for f, _ in r_b['own_only']]} " + f"baselined={[f.fkey for f, _ in r_b['baselined']]}") + # repo-scoped: the same rule under a different target must NOT match. + r_b2 = compare(own, oracles, tol=3) + if apply_baseline(r_b2, "different/repo", [bl_rule]) != 0: + fails.append("baseline must not cross repos") + # a `*` repo applies to any target. + r_b3 = compare(own, oracles, tol=3) + if apply_baseline(r_b3, "anything", [BaselineRule("*", "B.cs", "OWN001", "local 'b'")]) != 1: + fails.append("baseline '*' repo should match any target") + # the report renders the baselined section (and only when there are entries). + if "## Known FP (baselined)" not in render_md(r_b, "o/r", "x", ["infersharp"], 3, + baseline_path="bl.txt"): + fails.append("baselined section not rendered when entries present") + if "## Known FP (baselined)" in render_md(r, "o/r", "x", ["infersharp"], 3): + fails.append("baselined section rendered with no entries") + # the loader: comments/blank lines ignored, <4 fields skipped, reason optional. + import tempfile + with tempfile.NamedTemporaryFile("w", suffix=".txt", delete=False) as tf: + tf.write("# comment\n\no/r | B.cs | OWN001 | local 'b' | a reason\n" + "too | few | fields\n") + tf.flush() + loaded = _load_baseline(tf.name) + if len(loaded) != 1 or loaded[0].reason != "a reason": + fails.append(f"baseline loader wrong: {loaded}") + + total = 30 for f in fails: print(f"ORACLE SELFTEST FAIL: {f}") print(f"oracle_compare selftest: {total - len(fails)}/{total} checks passed") From 6f4818c90ef78748ee1d36096d1bf795ec914cfe Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 13:37:12 +0000 Subject: [PATCH 09/16] extractor: recognize field disposal via a receiver-consuming helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A field released through a 'drain and dispose' sink — a first-party extension method that disposes its receiver, like NLog's WaitForDispose(this Timer, TimeSpan) — was reported as an OWN001 owned-field leak, because the disposal scan only keys on a literal .Dispose()/.Close()/.DisposeAsync() on the field. Credit field.M(...) as a release when M is a first-party extension method whose receiver it disposes. CallReleasesReceiver reuses the existing ConsumesParam on the reduced extension method's receiver parameter, so the release is proved by inspecting M's real body (following first-party forwarding, cycle-guarded, IDisposable-only) — never guessed from the name. Scope is extension methods only; an instance method disposing its own 'this' is left out. Clears the NLog timer FPs where the sink is called on the field directly or through a 'var t = _field;' alias (AsyncTaskTarget, AsyncTargetWrapper, BufferingTargetWrapper). TimeoutContinuation, which disposes the result of Interlocked.Exchange(ref _timer, null), stays baselined until ref-exchange alias tracking lands. New corpus fixture field-dispose-via-helper proves it: before.cs leaks the Timer field (OWN001 caught), after.cs releases it via the sink and must be silent — the benchmark's after-clean gate is the end-to-end check. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- .../field-dispose-via-helper/after.cs | 27 ++++++++++++ .../field-dispose-via-helper/before.cs | 29 +++++++++++++ .../field-dispose-via-helper/case.own | 23 ++++++++++ .../expected-diagnostics.txt | 1 + .../field-dispose-via-helper/notes.md | 42 +++++++++++++++++++ frontend/roslyn/OwnSharp.Extractor/Program.cs | 35 ++++++++++++++-- 6 files changed, 153 insertions(+), 4 deletions(-) create mode 100644 corpus/real-world/field-dispose-via-helper/after.cs create mode 100644 corpus/real-world/field-dispose-via-helper/before.cs create mode 100644 corpus/real-world/field-dispose-via-helper/case.own create mode 100644 corpus/real-world/field-dispose-via-helper/expected-diagnostics.txt create mode 100644 corpus/real-world/field-dispose-via-helper/notes.md diff --git a/corpus/real-world/field-dispose-via-helper/after.cs b/corpus/real-world/field-dispose-via-helper/after.cs new file mode 100644 index 00000000..c9b071d4 --- /dev/null +++ b/corpus/real-world/field-dispose-via-helper/after.cs @@ -0,0 +1,27 @@ +using System; +using System.Threading; + +// A "drain and dispose" sink: stops the timer, then disposes it. Because its +// receiver parameter is disposed in the body, a `_timer.WaitForDispose(...)` call +// releases the field — the extractor proves this by inspecting the sink's body +// (ConsumesParam on the reduced extension method's receiver), not by name. +static class TimerSink +{ + public static void WaitForDispose(this Timer timer, TimeSpan timeout) + { + timer.Change(Timeout.Infinite, Timeout.Infinite); + timer.Dispose(); + } +} + +// FIX: the owned Timer field is released through the sink on Dispose. No literal +// `_timer.Dispose()` appears, so recognising this requires following the sink's +// dispose effect — once we do, there is no leak and the case is silent. +sealed class Worker : IDisposable +{ + readonly Timer _timer; + + public Worker() => _timer = new Timer(_ => { }, null, 0, 1000); + + public void Dispose() => _timer.WaitForDispose(TimeSpan.Zero); +} diff --git a/corpus/real-world/field-dispose-via-helper/before.cs b/corpus/real-world/field-dispose-via-helper/before.cs new file mode 100644 index 00000000..7d232b84 --- /dev/null +++ b/corpus/real-world/field-dispose-via-helper/before.cs @@ -0,0 +1,29 @@ +using System; +using System.Threading; + +// A "drain and dispose" sink: the canonical shape mined on NLog, where a Timer +// field is released not by a literal `_timer.Dispose()` but by a custom extension +// method that stops the timer and then disposes it (NLog's +// `WaitForDispose(this Timer, TimeSpan)` in Common/AsyncHelpers.cs). +static class TimerSink +{ + public static void WaitForDispose(this Timer timer, TimeSpan timeout) + { + timer.Change(Timeout.Infinite, Timeout.Infinite); + timer.Dispose(); + } +} + +// BUG: the owned Timer field is constructed but never released on any path — +// neither a literal `.Dispose()` nor the sink. A genuine OWN001 owned-field leak. +sealed class Worker : IDisposable +{ + readonly Timer _timer; + + public Worker() => _timer = new Timer(_ => { }, null, 0, 1000); + + public void Dispose() + { + // nothing — _timer is leaked + } +} diff --git a/corpus/real-world/field-dispose-via-helper/case.own b/corpus/real-world/field-dispose-via-helper/case.own new file mode 100644 index 00000000..dbdb566a --- /dev/null +++ b/corpus/real-world/field-dispose-via-helper/case.own @@ -0,0 +1,23 @@ +// OwnLang model of the "drain and dispose" field-release pattern mined on NLog +// (src/NLog/Targets/AsyncTaskTarget.cs and siblings). A Timer field is owned by the +// component; the real fix releases it not with a literal `_timer.Dispose()` but +// through a custom extension method `WaitForDispose(this Timer, TimeSpan)` that +// stops the timer and then disposes it (Common/AsyncHelpers.cs). The before.cs +// leaks the timer (no release on any path) — the generic OWN001 owned-field leak +// modelled here as an acquire with no `release`. The after.cs releases it via the +// sink, which the extractor now recognises by inspecting the sink's body +// (ConsumesParam on its receiver parameter); see notes.md for that recognition rule +// and which NLog variants it does and does not reach. +module Corpus +resource Timer { + acquire create + release dispose + kind "disposable" + emit_type "Timer" + emit_acquire "new Timer({args})" + emit_release "{0}.WaitForDispose()" +} +fn Worker(callback: int) { + let timer = acquire Timer(callback); // _timer = new Timer(_ => { }, null, 0, 1000) + // no `release timer;` — the timer is never stopped or disposed (OWN001) +} diff --git a/corpus/real-world/field-dispose-via-helper/expected-diagnostics.txt b/corpus/real-world/field-dispose-via-helper/expected-diagnostics.txt new file mode 100644 index 00000000..ed2a1929 --- /dev/null +++ b/corpus/real-world/field-dispose-via-helper/expected-diagnostics.txt @@ -0,0 +1 @@ +OWN001 diff --git a/corpus/real-world/field-dispose-via-helper/notes.md b/corpus/real-world/field-dispose-via-helper/notes.md new file mode 100644 index 00000000..ae5cbe6c --- /dev/null +++ b/corpus/real-world/field-dispose-via-helper/notes.md @@ -0,0 +1,42 @@ +# field-dispose-via-helper + +An owned `IDisposable` field is released not by a literal `_field.Dispose()` but by +a **first-party extension method that disposes its receiver** — the "drain and +dispose" sink. The canonical real-world shape is NLog's +`WaitForDispose(this Timer, TimeSpan)` (`Common/AsyncHelpers.cs`), which stops the +timer (`Change(Infinite, Infinite)`) and then disposes it; targets call +`_taskTimeoutTimer.WaitForDispose(...)` from `Dispose(bool disposing)` / +`CloseTarget()`. + +- **before.cs** — the `Timer` field is constructed and never released on any path + → `OWN001` (the bug is caught). +- **after.cs** — the field is released via `_timer.WaitForDispose(...)` → **clean**. + No literal `.Dispose()` on the field appears, so the only way to see the release + is to follow the sink's dispose effect. + +## Recognition rule + +The disposal scan already credits a field released anywhere in the class by +`field.Dispose()` / `.Close()` / `.DisposeAsync()` (directly, through a +`var t = _field;` alias, or null-conditional), so a field disposed in +`Dispose(bool disposing)` or `CloseTarget()` is already handled. This case adds the +missing hop: a call `field.M(...)` also releases the field when **`M` is a +first-party extension method whose receiver it disposes**. It is proved, not +guessed — `CallReleasesReceiver` reuses `ConsumesParam` on the reduced extension +method's receiver parameter (index 0), which inspects `M`'s real body, follows +first-party forwarding chains, is cycle-guarded, and requires an `IDisposable` +parameter. So an unknown or borrowing callee never credits a release. + +## Honesty caveat — what this does and does not reach + +This clears the NLog variants where the sink is called on the field **directly** or +through a simple `var t = _field;` alias (`AsyncTaskTarget`, `AsyncTargetWrapper`, +`BufferingTargetWrapper`). It does **not** reach `TimeoutContinuation`, where the +receiver is the result of `Interlocked.Exchange(ref _timer, null)` — an +exchange-and-null-out idiom whose result is not a tracked field alias. That one +stays in the oracle FP baseline +([`corpus/oracle-fp-baseline.txt`](../../oracle-fp-baseline.txt), +[`docs/notes/oracle-known-fps.md`](../../../docs/notes/oracle-known-fps.md)) until +ref-exchange alias tracking lands. Scope is intentionally limited to **extension +methods**: an instance method disposing its own `this` is not a real +dispose-delegation shape and would drag in virtual-dispatch reasoning. diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 048ed900..406ad53d 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -2494,6 +2494,27 @@ static bool DisposesLocal(SyntaxNode body, string name) return false; } +// Does the call `recv.M(...)` RELEASE its receiver — i.e. is `M` a first-party +// EXTENSION method whose body disposes the value it is invoked on? The dispose is +// then laundered through a custom "drain and dispose" sink (e.g. NLog's +// `timer.WaitForDispose(timeout)`, which calls `timer.Change(...)` then disposes +// it) rather than a literal `recv.Dispose()`, so the disposal scan that only keys +// on `Dispose`/`Close`/`DisposeAsync` misses it and reports a false leak. We reuse +// `ConsumesParam` on the reduced extension method's RECEIVER parameter (the +// `this T` at index 0): that inspects M's real body, follows first-party +// forwarding, is cycle-guarded, and demands an IDisposable param — so we only ever +// credit a release we can SEE happen, never an unknown/borrowing callee. Scope is +// deliberately narrow (extension methods only): an instance method disposing its +// own `this` is not a real dispose-delegation shape, and widening to it would also +// have to reason about virtual dispatch — left out to keep the surface minimal. +static bool CallReleasesReceiver(IMethodSymbol? sym, SemanticModel model) +{ + if (sym?.ReducedFrom is not { } def || def.Parameters.Length == 0) + return false; + return ConsumesParam(def, def.Parameters[0], model, + new HashSet(SymbolEqualityComparer.Default)); +} + // A field/local type treated as owned-disposable (syntax-only heuristic — no // semantic model): a curated set plus a few suffixes. Gated on the class `new`ing // the value, so injected/borrowed disposables are not flagged. Timer types are @@ -3300,8 +3321,13 @@ or ImplicitObjectCreationExpressionSyntax // `this`/bare receiver syntactically. foreach (var inv in cls.DescendantNodes().OfType()) if (inv.Expression is MemberAccessExpressionSyntax m - && m.Name.Identifier.Text is "Dispose" or "DisposeAsync" or "Close" - && ThisFieldName(m.Expression) is { } df) + && ThisFieldName(m.Expression) is { } df + // a literal `.Dispose()/.Close()/.DisposeAsync()`, OR a first-party + // extension method that disposes its receiver (`timer.WaitForDispose()`): + // both release the field. The name check is first (cheap) — the symbol + // resolution behind CallReleasesReceiver only runs for the rarer custom call. + && (m.Name.Identifier.Text is "Dispose" or "DisposeAsync" or "Close" + || CallReleasesReceiver(model.GetSymbolInfo(inv).Symbol as IMethodSymbol, model))) disposed.Add(model.GetSymbolInfo(m.Expression).Symbol is ILocalSymbol ls && aliasToField.TryGetValue(ls, out var fa) ? fa : df); // Also the NULL-CONDITIONAL form `field?.Dispose()` (a ConditionalAccess whose @@ -3312,8 +3338,9 @@ or ImplicitObjectCreationExpressionSyntax // (The same alias-by-symbol translation applies — `cts?.Dispose()` on an aliasing local.) foreach (var cae in cls.DescendantNodes().OfType()) if (ThisFieldName(cae.Expression) is { } cdf // this-instance field / alias only (not `other._f?.Close()`) - && cae.WhenNotNull is InvocationExpressionSyntax { Expression: MemberBindingExpressionSyntax mb } - && mb.Name.Identifier.Text is "Dispose" or "DisposeAsync" or "Close") + && cae.WhenNotNull is InvocationExpressionSyntax { Expression: MemberBindingExpressionSyntax mb } cinv + && (mb.Name.Identifier.Text is "Dispose" or "DisposeAsync" or "Close" + || CallReleasesReceiver(model.GetSymbolInfo(cinv).Symbol as IMethodSymbol, model))) disposed.Add(model.GetSymbolInfo(cae.Expression).Symbol is ILocalSymbol lc && aliasToField.TryGetValue(lc, out var fc) ? fc : cdf); From 53c4e1cd27657d115907a40d56a3f626cce1ef7d Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 13:45:49 +0000 Subject: [PATCH 10/16] oracle: re-run on NLog to confirm the dispose-helper fix clears 4 timer FPs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Temporary, dev-only: point the push-triggered oracle at NLog/NLog and disable the four direct/alias timer baseline entries that the CallReleasesReceiver fix (6f4818c) should now clear. If they stay out of own-only on the live run, the fix cleared them for real and the entries are deleted; if they resurface, they are restored. TimeoutContinuation stays baselined — its Interlocked.Exchange ref-alias is out of the fix's scope. Reverted after the run. 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 | 13 ++++++++----- corpus/oracle-target.txt | 6 ++++-- 3 files changed, 13 insertions(+), 7 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-fp-baseline.txt b/corpus/oracle-fp-baseline.txt index e44ee4a4..8280608f 100644 --- a/corpus/oracle-fp-baseline.txt +++ b/corpus/oracle-fp-baseline.txt @@ -26,11 +26,14 @@ # custom dispose-sink the extractor's intra-Dispose() field scan can't see. Two # also dispose only in the dispose-pattern override / lifecycle hook (Dispose(bool # disposing), CloseTarget()), neither of which the field scan visits. -NLog/NLog | TimeoutContinuation.cs | OWN001 | _timeoutTimer | disposed via Dispose() -> StopTimer() -> Timer.WaitForDispose(); interprocedural custom dispose-sink not traced -NLog/NLog | AsyncTaskTarget.cs | OWN001 | _taskTimeoutTimer | disposed in Dispose(bool disposing) via Timer.WaitForDispose(); dispose-pattern override + custom sink not traced -NLog/NLog | AsyncTaskTarget.cs | OWN001 | _lazyWriterTimer | disposed in Dispose(bool disposing) via Timer.WaitForDispose(); dispose-pattern override + custom sink not traced -NLog/NLog | AsyncTargetWrapper.cs | OWN001 | _lazyWriterTimer | disposed in CloseTarget() -> StopLazyWriterThread() -> Timer.WaitForDispose(); lifecycle hook + custom sink not traced -NLog/NLog | BufferingTargetWrapper.cs | OWN001 | _flushTimer | disposed in CloseTarget() via Timer.WaitForDispose(); lifecycle hook + custom sink not traced +NLog/NLog | TimeoutContinuation.cs | OWN001 | _timeoutTimer | disposed via Dispose() -> StopTimer() -> Timer.WaitForDispose() on the result of Interlocked.Exchange(ref _timeoutTimer, null) — a ref-exchange alias the extractor does not yet track (CallReleasesReceiver clears the direct/simple-alias siblings, not this one) +# [fix-pending experiment] The four entries below are cleared by the CallReleasesReceiver +# extractor fix (commit 6f4818c) — disabled here to confirm on a live NLog oracle run that +# they no longer fire; if confirmed they are deleted, if they resurface they are restored. +# NLog/NLog | AsyncTaskTarget.cs | OWN001 | _taskTimeoutTimer | disposed in Dispose(bool disposing) via Timer.WaitForDispose(); dispose-pattern override + custom sink not traced +# NLog/NLog | AsyncTaskTarget.cs | OWN001 | _lazyWriterTimer | disposed in Dispose(bool disposing) via Timer.WaitForDispose(); dispose-pattern override + custom sink not traced +# NLog/NLog | AsyncTargetWrapper.cs | OWN001 | _lazyWriterTimer | disposed in CloseTarget() -> StopLazyWriterThread() -> Timer.WaitForDispose(); lifecycle hook + custom sink not traced +# NLog/NLog | BufferingTargetWrapper.cs | OWN001 | _flushTimer | disposed in CloseTarget() via Timer.WaitForDispose(); lifecycle hook + custom sink not traced # --- protobuf-net/protobuf-net -------------------------------------------------- protobuf-net/protobuf-net | ProtoWriter.BufferWriter.cs | OWN001 | _nullWriter | intentional null-object kept attached for pooled reuse; Dispose() documents "don't cascade dispose to the null one" diff --git a/corpus/oracle-target.txt b/corpus/oracle-target.txt index 52cb6d64..68cd128c 100644 --- a/corpus/oracle-target.txt +++ b/corpus/oracle-target.txt @@ -13,5 +13,7 @@ # 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] pointed at NLog to confirm the CallReleasesReceiver fix clears the 4 +# direct/alias timer FPs; restored to the systemevents-console fixture after the run. +NLog/NLog +include_tests=false From 943a33c71224632432ac7ac5a2bd2f8e49aef08d Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 13:55:20 +0000 Subject: [PATCH 11/16] oracle: confirm dispose-helper fix on NLog (own-only 8->4), trim baseline Live NLog oracle re-run with the four WaitForDispose timer entries disabled confirms the CallReleasesReceiver fix cleared them at the source: leak-class own-only total 8 -> 4, and the four timers (AsyncTaskTarget _taskTimeoutTimer/ _lazyWriterTimer, AsyncTargetWrapper _lazyWriterTimer, BufferingTargetWrapper _flushTimer) appear in neither own-only nor the baselined section. The remaining NLog own-only is just the three benign-but-real undisposed fields (XmlParser _xmlSource, FileTarget _reusable*Stream), with only TimeoutContinuation _timeoutTimer baselined (its Interlocked.Exchange ref-alias is out of scope). - Delete the four now-redundant baseline entries (baseline: 13 -> 9 rules). - Document the fix landed in oracle-known-fps.md (root-cause #1 mostly-fixed). - Restore the dev scaffolding: oracle-target.txt back to the systemevents-console fixture, remove the branch from the oracle.yml push filter. 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 | 22 ++++++------- corpus/oracle-target.txt | 6 ++-- docs/notes/oracle-known-fps.md | 56 +++++++++++++++++++++------------- 4 files changed, 46 insertions(+), 39 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 8280608f..a9abc9f5 100644 --- a/corpus/oracle-fp-baseline.txt +++ b/corpus/oracle-fp-baseline.txt @@ -20,20 +20,16 @@ # (fields genuinely never disposed, but wrapping managed-memory-only resources). # Those are NOT baselined; they are real catches the oracle's leak query misses. -# --- NLog/NLog : timers disposed through an indirection we don't trace ---------- -# Every one is released via the `WaitForDispose(this Timer, TimeSpan)` extension -# method (AsyncHelpers.cs) on a local alias of the field — an interprocedural -# custom dispose-sink the extractor's intra-Dispose() field scan can't see. Two -# also dispose only in the dispose-pattern override / lifecycle hook (Dispose(bool -# disposing), CloseTarget()), neither of which the field scan visits. +# --- NLog/NLog : one timer disposed through an alias we don't yet track ---------- +# The four sibling timer FPs (AsyncTaskTarget._taskTimeoutTimer/_lazyWriterTimer, +# AsyncTargetWrapper._lazyWriterTimer, BufferingTargetWrapper._flushTimer) were here +# too, but the CallReleasesReceiver extractor fix now traces their release through +# the `WaitForDispose(this Timer)` sink (called on the field directly or via a simple +# `var t = _field;` alias) — confirmed cleared on a live NLog oracle run (own-only +# 8 -> 4), so they are deleted. Only TimeoutContinuation remains: it disposes the +# RESULT of `Interlocked.Exchange(ref _timeoutTimer, null)`, a ref-exchange alias the +# extractor does not yet bind to the field, so the sink call is not seen as a release. NLog/NLog | TimeoutContinuation.cs | OWN001 | _timeoutTimer | disposed via Dispose() -> StopTimer() -> Timer.WaitForDispose() on the result of Interlocked.Exchange(ref _timeoutTimer, null) — a ref-exchange alias the extractor does not yet track (CallReleasesReceiver clears the direct/simple-alias siblings, not this one) -# [fix-pending experiment] The four entries below are cleared by the CallReleasesReceiver -# extractor fix (commit 6f4818c) — disabled here to confirm on a live NLog oracle run that -# they no longer fire; if confirmed they are deleted, if they resurface they are restored. -# NLog/NLog | AsyncTaskTarget.cs | OWN001 | _taskTimeoutTimer | disposed in Dispose(bool disposing) via Timer.WaitForDispose(); dispose-pattern override + custom sink not traced -# NLog/NLog | AsyncTaskTarget.cs | OWN001 | _lazyWriterTimer | disposed in Dispose(bool disposing) via Timer.WaitForDispose(); dispose-pattern override + custom sink not traced -# NLog/NLog | AsyncTargetWrapper.cs | OWN001 | _lazyWriterTimer | disposed in CloseTarget() -> StopLazyWriterThread() -> Timer.WaitForDispose(); lifecycle hook + custom sink not traced -# NLog/NLog | BufferingTargetWrapper.cs | OWN001 | _flushTimer | disposed in CloseTarget() via Timer.WaitForDispose(); lifecycle hook + custom sink not traced # --- protobuf-net/protobuf-net -------------------------------------------------- protobuf-net/protobuf-net | ProtoWriter.BufferWriter.cs | OWN001 | _nullWriter | intentional null-object kept attached for pooled reuse; Dispose() documents "don't cascade dispose to the null one" diff --git a/corpus/oracle-target.txt b/corpus/oracle-target.txt index 68cd128c..52cb6d64 100644 --- a/corpus/oracle-target.txt +++ b/corpus/oracle-target.txt @@ -13,7 +13,5 @@ # 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. -# [temporary] pointed at NLog to confirm the CallReleasesReceiver fix clears the 4 -# direct/alias timer FPs; restored to the systemevents-console fixture after the run. -NLog/NLog -include_tests=false +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 1c60fade..1a8fdcac 100644 --- a/docs/notes/oracle-known-fps.md +++ b/docs/notes/oracle-known-fps.md @@ -23,27 +23,38 @@ reason: we and the oracles occupy orthogonal niches. | disposition | count | what happens on re-run | |---|---:|---| -| **Baselined FP** | 12 | moved to "Known FP (baselined)", out of the triage queue | +| **Fixed in the extractor** | 4 | no longer fire (NLog `WaitForDispose` timers — see below) | +| **Baselined FP** | 8 | 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 | -The 12 baselined FPs + the 2 non-product-sample reals = 14 findings, covered by -**13 rules** in `corpus/oracle-fp-baseline.txt` (the two `NetTranscoder` copies +The 8 baselined FPs + the 2 non-product-sample reals = 10 findings, covered by +**9 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. +**Update (extractor fix landed).** 4 of the original 12 baselined FPs — the NLog +`AsyncTaskTarget._taskTimeoutTimer`/`_lazyWriterTimer`, +`AsyncTargetWrapper._lazyWriterTimer`, `BufferingTargetWrapper._flushTimer` timers +disposed through `WaitForDispose(this Timer)` — are now **fixed at the source** +(`CallReleasesReceiver`, see root-cause #1) and their baseline entries deleted. A +live NLog oracle re-run with those entries disabled confirmed it: own-only leak +total **8 → 4**, the 4 timers absent from both own-only and the baselined section. +Only `TimeoutContinuation._timeoutTimer` remains baselined (its +`Interlocked.Exchange(ref _timer, null)` ref-alias is out of the fix's scope). + ## Per-finding verdicts ### NLog/NLog — 8 findings (disposable fields) | field / class | verdict | why | |---|---|---| -| `_timeoutTimer` / TimeoutContinuation | **FP → baseline** | `Dispose()` → `StopTimer()` → `Timer.WaitForDispose()` (ext method) | -| `_taskTimeoutTimer` / AsyncTaskTarget | **FP → baseline** | `Dispose(bool disposing)` → `Timer.WaitForDispose()` | -| `_lazyWriterTimer` / AsyncTaskTarget | **FP → baseline** | `Dispose(bool disposing)` → `Timer.WaitForDispose()` | -| `_lazyWriterTimer` / AsyncTargetWrapper | **FP → baseline** | `CloseTarget()` → `StopLazyWriterThread()` → `Timer.WaitForDispose()` | -| `_flushTimer` / BufferingTargetWrapper | **FP → baseline** | `CloseTarget()` → `Timer.WaitForDispose()` | +| `_timeoutTimer` / TimeoutContinuation | **FP → baseline** | `Dispose()` → `StopTimer()` → `Timer.WaitForDispose()` on the `Interlocked.Exchange(ref _timer, null)` result (ref-alias, out of fix scope) | +| `_taskTimeoutTimer` / AsyncTaskTarget | **FP → fixed** | `Dispose(bool disposing)` → `Timer.WaitForDispose()` (direct field receiver) | +| `_lazyWriterTimer` / AsyncTaskTarget | **FP → fixed** | `Dispose(bool disposing)` → `Timer.WaitForDispose()` (direct field receiver) | +| `_lazyWriterTimer` / AsyncTargetWrapper | **FP → fixed** | `CloseTarget()` → `StopLazyWriterThread()` → `Timer.WaitForDispose()` (simple alias) | +| `_flushTimer` / BufferingTargetWrapper | **FP → fixed** | `CloseTarget()` → `Timer.WaitForDispose()` (simple alias) | | `_xmlSource` / XmlParser | **true positive → keep** | never disposed (benign: a `CharEnumerator` over a `StringReader` over a string — no unmanaged resource) | | `_reusableFileWriteStream` / FileTarget | **true positive → keep** | never disposed (benign: `ReusableStreamCreator` over a `MemoryStream` — managed memory only) | | `_reusableBatchFileWriteStream` / FileTarget | **true positive → keep** | never disposed (same) | @@ -102,19 +113,22 @@ The baselined FPs cluster into four analyzer limitations. Each baseline entry is standing request for the corresponding capability — when it lands, retire the entry and let the oracle re-confirm clean. -1. **Interprocedural / custom dispose-sink not traced** *(5 NLog timers, - protobuf `Page.xaml.cs` using-block).* Field-disposal recognition only scans - top-level `Dispose()` statements for a direct `field.Dispose()`. It misses the - dispose-pattern override `Dispose(bool disposing)`, `DisposeAsync`, lifecycle - hooks (`CloseTarget()`/`Close()`), helper-method indirection, disposal of a - **local alias** of the field, custom dispose-sink methods like - `WaitForDispose(this Timer)`, and the `using (preExistingLocal)` statement form. - *Fix:* extend `DisposedOwningFields` (extractor) to additional dispose entry - points + one level of intra-class call/alias following; longer-term, real - interprocedural escape/dispose analysis. The cheap, sound first step - (recognise `Dispose(bool disposing)` + `DisposeAsync` direct field disposal) is - correct and low-risk but does **not** by itself clear these NLog cases, because - their disposal is laundered through a local alias **and** a custom-named sink. +1. **Custom dispose-sink — MOSTLY FIXED** *(was 5 NLog timers; 4 now cleared, 1 + remains).* The disposal scan already covers the whole class (so + `Dispose(bool disposing)`, `CloseTarget()`, helper methods, simple `var t = _f;` + aliases, and null-conditional `_f?.Dispose()` were all already handled) — the one + gap was the disposing-method **name**: NLog releases its timers through a custom + extension `WaitForDispose(this Timer)`, not a literal `.Dispose()`. **Shipped fix:** + `CallReleasesReceiver` (extractor) credits `field.M(...)` as a release when `M` is a + first-party extension method whose receiver it disposes — proved by reusing + `ConsumesParam` on `M`'s reduced receiver parameter (inspects the real body, follows + first-party forwarding, cycle-guarded, IDisposable-only), never guessed from the + name. A live NLog re-run confirmed it: own-only 8 → 4, the 4 direct/simple-alias + timers cleared. **Still open:** `TimeoutContinuation._timeoutTimer` disposes the + result of `Interlocked.Exchange(ref _timer, null)` — a ref-exchange alias not bound + to the field — and the protobuf `Page.xaml.cs` `using (preExistingLocal)` local form; + both remain baselined until ref-exchange/using-statement alias tracking lands. + Corpus fixture: `field-dispose-via-helper`. 2. **No-op `Dispose` not modelled** *(Newtonsoft `TraceJsonReader._textWriter`).* We flag any undisposed `IDisposable` structurally, without modelling that the From a8828281bf011448e48a57b683e175715b8c3180 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 17:43:23 +0000 Subject: [PATCH 12/16] extractor: trace field disposal through Interlocked.Exchange teardown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clears the last NLog WaitForDispose timer FP — TimeoutContinuation, which releases its timer via the atomic detach-and-dispose idiom: var current = Interlocked.Exchange(ref _timeoutTimer, null); current?.WaitForDispose(TimeSpan.Zero); Exchange atomically nulls the field and RETURNS the object it used to own, so the local aliases the field's just-detached disposable. RefExchangeNulledField binds such a local to the field in aliasToField (alongside the plain 'var x = _field;' alias), so the existing null-conditional + CallReleasesReceiver scan credits the field as released. Restricted to a null/default replacement — the unambiguous teardown; an exchange installing a new non-null value re-arms the field with an object the syntactic scan can't follow, so it is declined (precision-first: never hide a real leak). New corpus fixture field-dispose-via-exchange (faithful to TimeoutContinuation) proves it: before.cs leaks (OWN001), after.cs releases via the idiom and is silent. All five NLog timer FPs are now fixed at the source; the NLog FP baseline is empty. Re-pointing the oracle at NLog to confirm own-only 4 -> 3 on real code. 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 | 19 ++++---- corpus/oracle-target.txt | 6 ++- .../field-dispose-via-exchange/after.cs | 33 +++++++++++++ .../field-dispose-via-exchange/before.cs | 28 +++++++++++ .../field-dispose-via-exchange/case.own | 21 ++++++++ .../expected-diagnostics.txt | 1 + .../field-dispose-via-exchange/notes.md | 34 +++++++++++++ docs/notes/oracle-known-fps.md | 42 +++++++++------- frontend/roslyn/OwnSharp.Extractor/Program.cs | 48 +++++++++++++++++-- 10 files changed, 198 insertions(+), 35 deletions(-) create mode 100644 corpus/real-world/field-dispose-via-exchange/after.cs create mode 100644 corpus/real-world/field-dispose-via-exchange/before.cs create mode 100644 corpus/real-world/field-dispose-via-exchange/case.own create mode 100644 corpus/real-world/field-dispose-via-exchange/expected-diagnostics.txt create mode 100644 corpus/real-world/field-dispose-via-exchange/notes.md 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-fp-baseline.txt b/corpus/oracle-fp-baseline.txt index a9abc9f5..7648b296 100644 --- a/corpus/oracle-fp-baseline.txt +++ b/corpus/oracle-fp-baseline.txt @@ -20,16 +20,15 @@ # (fields genuinely never disposed, but wrapping managed-memory-only resources). # Those are NOT baselined; they are real catches the oracle's leak query misses. -# --- NLog/NLog : one timer disposed through an alias we don't yet track ---------- -# The four sibling timer FPs (AsyncTaskTarget._taskTimeoutTimer/_lazyWriterTimer, -# AsyncTargetWrapper._lazyWriterTimer, BufferingTargetWrapper._flushTimer) were here -# too, but the CallReleasesReceiver extractor fix now traces their release through -# the `WaitForDispose(this Timer)` sink (called on the field directly or via a simple -# `var t = _field;` alias) — confirmed cleared on a live NLog oracle run (own-only -# 8 -> 4), so they are deleted. Only TimeoutContinuation remains: it disposes the -# RESULT of `Interlocked.Exchange(ref _timeoutTimer, null)`, a ref-exchange alias the -# extractor does not yet bind to the field, so the sink call is not seen as a release. -NLog/NLog | TimeoutContinuation.cs | OWN001 | _timeoutTimer | disposed via Dispose() -> StopTimer() -> Timer.WaitForDispose() on the result of Interlocked.Exchange(ref _timeoutTimer, null) — a ref-exchange alias the extractor does not yet track (CallReleasesReceiver clears the direct/simple-alias siblings, not this one) +# --- NLog/NLog : all WaitForDispose timer FPs fixed at the source ---------------- +# All five NLog timers disposed through the `WaitForDispose(this Timer)` sink are now +# cleared in the extractor — no baseline entries remain. Four (AsyncTaskTarget +# _taskTimeoutTimer/_lazyWriterTimer, AsyncTargetWrapper _lazyWriterTimer, +# BufferingTargetWrapper _flushTimer) by CallReleasesReceiver (direct / simple-alias +# receiver), and TimeoutContinuation _timeoutTimer by RefExchangeNulledField (its +# receiver is the `Interlocked.Exchange(ref _timeoutTimer, null)` result, now bound to +# the field). The three remaining NLog own-only findings (XmlParser _xmlSource, +# FileTarget _reusable*Stream) are genuine undisposed fields, kept visible. # --- protobuf-net/protobuf-net -------------------------------------------------- protobuf-net/protobuf-net | ProtoWriter.BufferWriter.cs | OWN001 | _nullWriter | intentional null-object kept attached for pooled reuse; Dispose() documents "don't cascade dispose to the null one" diff --git a/corpus/oracle-target.txt b/corpus/oracle-target.txt index 52cb6d64..4cd74794 100644 --- a/corpus/oracle-target.txt +++ b/corpus/oracle-target.txt @@ -13,5 +13,7 @@ # 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] pointed at NLog to confirm RefExchangeNulledField clears the last timer +# FP (TimeoutContinuation); restored to the systemevents-console fixture after the run. +NLog/NLog +include_tests=false diff --git a/corpus/real-world/field-dispose-via-exchange/after.cs b/corpus/real-world/field-dispose-via-exchange/after.cs new file mode 100644 index 00000000..02e54d36 --- /dev/null +++ b/corpus/real-world/field-dispose-via-exchange/after.cs @@ -0,0 +1,33 @@ +using System; +using System.Threading; + +// The "detach and dispose" teardown: stop + dispose the timer through the sink. +static class TimerSink +{ + public static void WaitForDispose(this Timer timer, TimeSpan timeout) + { + timer.Change(Timeout.Infinite, Timeout.Infinite); + timer.Dispose(); + } +} + +// FIX: the owned Timer field is released through the canonical atomic teardown — +// `Interlocked.Exchange(ref _timer, null)` hands back the live timer and nulls the +// field, then the sink disposes it. Recognising this needs two hops: the local +// `current` is bound to `_timer` because Exchange returns the field's owned object +// (RefExchangeNulledField), and `current?.WaitForDispose(...)` is a release because +// the sink disposes its receiver (CallReleasesReceiver). Together → no leak, silent. +sealed class Continuation : IDisposable +{ + Timer? _timer; + + public Continuation() => _timer = new Timer(_ => { }, null, 0, 1000); + + public void Dispose() => StopTimer(); + + void StopTimer() + { + var current = Interlocked.Exchange(ref _timer, null); + current?.WaitForDispose(TimeSpan.Zero); + } +} diff --git a/corpus/real-world/field-dispose-via-exchange/before.cs b/corpus/real-world/field-dispose-via-exchange/before.cs new file mode 100644 index 00000000..b532135d --- /dev/null +++ b/corpus/real-world/field-dispose-via-exchange/before.cs @@ -0,0 +1,28 @@ +using System; +using System.Threading; + +// The "detach and dispose" teardown mined on NLog's TimeoutContinuation: the owned +// Timer field is meant to be released via `Interlocked.Exchange(ref _timer, null)` +// (which hands back the live timer and nulls the field) followed by the +// `WaitForDispose(this Timer)` sink. Here the teardown is missing entirely. +static class TimerSink +{ + public static void WaitForDispose(this Timer timer, TimeSpan timeout) + { + timer.Change(Timeout.Infinite, Timeout.Infinite); + timer.Dispose(); + } +} + +// BUG: the owned Timer field is constructed and never released on any path → OWN001. +sealed class Continuation : IDisposable +{ + Timer? _timer; + + public Continuation() => _timer = new Timer(_ => { }, null, 0, 1000); + + public void Dispose() + { + // nothing — _timer is leaked + } +} diff --git a/corpus/real-world/field-dispose-via-exchange/case.own b/corpus/real-world/field-dispose-via-exchange/case.own new file mode 100644 index 00000000..74ef29a9 --- /dev/null +++ b/corpus/real-world/field-dispose-via-exchange/case.own @@ -0,0 +1,21 @@ +// OwnLang model of NLog's TimeoutContinuation teardown (src/NLog/Internal/ +// TimeoutContinuation.cs, StopTimer). The owned Timer field is released by the atomic +// detach-and-dispose idiom: `Interlocked.Exchange(ref _timeoutTimer, null)` hands back +// the live timer and nulls the field, then `WaitForDispose(this Timer)` stops and +// disposes it. before.cs omits the teardown — the generic OWN001 owned-field leak, +// modelled here as an acquire with no `release`. after.cs releases it via the idiom, +// which the extractor recognises by binding the exchange result to the field +// (RefExchangeNulledField) and following the sink's dispose effect (CallReleasesReceiver). +module Corpus +resource Timer { + acquire create + release dispose + kind "disposable" + emit_type "Timer" + emit_acquire "new Timer({args})" + emit_release "Interlocked.Exchange(ref {0}, null)?.WaitForDispose()" +} +fn Continuation(callback: int) { + let timer = acquire Timer(callback); // _timer = new Timer(_ => { }, null, 0, 1000) + // no `release timer;` — the timer is never detached or disposed (OWN001) +} diff --git a/corpus/real-world/field-dispose-via-exchange/expected-diagnostics.txt b/corpus/real-world/field-dispose-via-exchange/expected-diagnostics.txt new file mode 100644 index 00000000..ed2a1929 --- /dev/null +++ b/corpus/real-world/field-dispose-via-exchange/expected-diagnostics.txt @@ -0,0 +1 @@ +OWN001 diff --git a/corpus/real-world/field-dispose-via-exchange/notes.md b/corpus/real-world/field-dispose-via-exchange/notes.md new file mode 100644 index 00000000..d06052a8 --- /dev/null +++ b/corpus/real-world/field-dispose-via-exchange/notes.md @@ -0,0 +1,34 @@ +# field-dispose-via-exchange + +The atomic **detach-and-dispose** teardown for an owned `IDisposable` field, mined on +NLog's `TimeoutContinuation.StopTimer` (`src/NLog/Internal/TimeoutContinuation.cs`): + +```csharp +var current = Interlocked.Exchange(ref _timeoutTimer, null); +current?.WaitForDispose(TimeSpan.Zero); +``` + +`Interlocked.Exchange(ref _field, null)` atomically nulls the field and **returns the +object it used to own**, so the local `current` aliases the field's just-detached +disposable; the `WaitForDispose(this Timer)` sink then stops and disposes it. + +- **before.cs** — the `Timer` field is constructed and never released → `OWN001`. +- **after.cs** — released via the exchange + sink → **clean**. + +## Recognition rule + +Two hops, each reusing existing machinery: + +1. **`RefExchangeNulledField`** binds the local `current` to `_timer`, because + `Interlocked.Exchange(ref _timer, null)` returns the field's owned object. Restricted + to a `null`/`default` replacement — the unambiguous teardown; an exchange that installs + a *new* non-null value re-arms the field with a fresh object the syntactic scan can't + follow, so crediting the field there could hide a real leak (declined, precision-first). + The bound alias joins the same `aliasToField` map as a plain `var x = _field;`. +2. **`CallReleasesReceiver`** recognises `current?.WaitForDispose(...)` as a release, + because the first-party extension sink disposes its receiver (proved via `ConsumesParam`). + +Together they clear `TimeoutContinuation`, the last NLog timer that the +[`field-dispose-via-helper`](../field-dispose-via-helper/notes.md) fix did not reach +(its receiver was the exchange result, not a tracked field alias). With this, all five +NLog `WaitForDispose` timer false positives are fixed at the source. diff --git a/docs/notes/oracle-known-fps.md b/docs/notes/oracle-known-fps.md index 1a8fdcac..582ce7ba 100644 --- a/docs/notes/oracle-known-fps.md +++ b/docs/notes/oracle-known-fps.md @@ -23,26 +23,27 @@ reason: we and the oracles occupy orthogonal niches. | disposition | count | what happens on re-run | |---|---:|---| -| **Fixed in the extractor** | 4 | no longer fire (NLog `WaitForDispose` timers — see below) | -| **Baselined FP** | 8 | moved to "Known FP (baselined)", out of the triage queue | +| **Fixed in the extractor** | 5 | no longer fire (all NLog `WaitForDispose` timers — see below) | +| **Baselined FP** | 7 | 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 | -The 8 baselined FPs + the 2 non-product-sample reals = 10 findings, covered by -**9 rules** in `corpus/oracle-fp-baseline.txt` (the two `NetTranscoder` copies +The 7 baselined FPs + the 2 non-product-sample reals = 9 findings, covered by +**8 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. -**Update (extractor fix landed).** 4 of the original 12 baselined FPs — the NLog -`AsyncTaskTarget._taskTimeoutTimer`/`_lazyWriterTimer`, -`AsyncTargetWrapper._lazyWriterTimer`, `BufferingTargetWrapper._flushTimer` timers -disposed through `WaitForDispose(this Timer)` — are now **fixed at the source** -(`CallReleasesReceiver`, see root-cause #1) and their baseline entries deleted. A -live NLog oracle re-run with those entries disabled confirmed it: own-only leak -total **8 → 4**, the 4 timers absent from both own-only and the baselined section. -Only `TimeoutContinuation._timeoutTimer` remains baselined (its -`Interlocked.Exchange(ref _timer, null)` ref-alias is out of the fix's scope). +**Update (extractor fix landed — all NLog timers).** All 5 of the original NLog +`WaitForDispose` timer FPs are now **fixed at the source**, baseline entries deleted. +Four (`AsyncTaskTarget._taskTimeoutTimer`/`_lazyWriterTimer`, +`AsyncTargetWrapper._lazyWriterTimer`, `BufferingTargetWrapper._flushTimer`) by +`CallReleasesReceiver` — a live NLog re-run confirmed own-only leak total **8 → 4**. +The fifth, `TimeoutContinuation._timeoutTimer` (disposed through the +`Interlocked.Exchange(ref _timer, null)` result), by `RefExchangeNulledField`, which +binds the exchange result to the field so the sink call is seen as a release +(own-only **4 → 3**, NLog baseline now empty). See root-cause #1. Corpus fixtures: +`field-dispose-via-helper`, `field-dispose-via-exchange`. ## Per-finding verdicts @@ -124,11 +125,16 @@ entry and let the oracle re-confirm clean. `ConsumesParam` on `M`'s reduced receiver parameter (inspects the real body, follows first-party forwarding, cycle-guarded, IDisposable-only), never guessed from the name. A live NLog re-run confirmed it: own-only 8 → 4, the 4 direct/simple-alias - timers cleared. **Still open:** `TimeoutContinuation._timeoutTimer` disposes the - result of `Interlocked.Exchange(ref _timer, null)` — a ref-exchange alias not bound - to the field — and the protobuf `Page.xaml.cs` `using (preExistingLocal)` local form; - both remain baselined until ref-exchange/using-statement alias tracking lands. - Corpus fixture: `field-dispose-via-helper`. + timers cleared. The fifth, `TimeoutContinuation._timeoutTimer`, disposes the result + of `Interlocked.Exchange(ref _timer, null)`; **`RefExchangeNulledField`** now binds + that exchange result to the field (the idiom atomically nulls the field and returns + 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). **Still open:** the protobuf `Page.xaml.cs` + `using (preExistingLocal)` local form, which remains baselined until using-statement + alias tracking lands. Corpus fixtures: `field-dispose-via-helper`, + `field-dispose-via-exchange`. 2. **No-op `Dispose` not modelled** *(Newtonsoft `TraceJsonReader._textWriter`).* We flag any undisposed `IDisposable` structurally, without modelling that the diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 406ad53d..8b1c4d5f 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -2515,6 +2515,35 @@ static bool CallReleasesReceiver(IMethodSymbol? sym, SemanticModel model) new HashSet(SymbolEqualityComparer.Default)); } +// `Interlocked.Exchange(ref _field, null)` — the atomic "detach and hand back" +// teardown idiom — atomically nulls the field and RETURNS the object it used to +// own, so a local bound to that result aliases the field's just-detached owned +// object: disposing the local releases the field's resource (mined on NLog's +// TimeoutContinuation.StopTimer, `Interlocked.Exchange(ref _timeoutTimer, null) +// ?.WaitForDispose(...)`). Returns that field's name so the alias map can bind it +// like a plain `var x = _field;` alias. Restricted to a `null`/`default` replacement +// — the unambiguous teardown: an Exchange that installs a NEW non-null value re-arms +// the field with a fresh object whose fate the syntactic scan cannot follow, so +// crediting the field there could hide a real leak (precision-first: decline). +static string? RefExchangeNulledField(ExpressionSyntax init, SemanticModel model) +{ + if (init is not InvocationExpressionSyntax inv + || model.GetSymbolInfo(inv).Symbol is not IMethodSymbol m + || m.Name != "Exchange" + || m.ContainingType is not { Name: "Interlocked" } ct + || ct.ContainingNamespace?.ToString() != "System.Threading") + return null; + var args = inv.ArgumentList.Arguments; + if (args.Count != 2 + || !args[0].RefKindKeyword.IsKind(SyntaxKind.RefKeyword) + || !(args[1].Expression.IsKind(SyntaxKind.NullLiteralExpression) + || args[1].Expression.IsKind(SyntaxKind.DefaultLiteralExpression))) + return null; + return ThisFieldName(args[0].Expression) is { } f + && model.GetSymbolInfo(args[0].Expression).Symbol is IFieldSymbol + ? f : null; +} + // A field/local type treated as owned-disposable (syntax-only heuristic — no // semantic model): a curated set plus a few suffixes. Gated on the class `new`ing // the value, so injected/borrowed disposables are not flagged. Timer types are @@ -3303,12 +3332,21 @@ or ImplicitObjectCreationExpressionSyntax reassignedAliases.Add(als); var aliasToField = new Dictionary(SymbolEqualityComparer.Default); foreach (var decl in cls.DescendantNodes().OfType()) - if (decl.Initializer?.Value is { } init - && ThisFieldName(init) is { } af - && model.GetSymbolInfo(init).Symbol is IFieldSymbol - && model.GetDeclaredSymbol(decl) is ILocalSymbol aliasSym - && !reassignedAliases.Contains(aliasSym)) + { + if (decl.Initializer?.Value is not { } init + || model.GetDeclaredSymbol(decl) is not ILocalSymbol aliasSym + || reassignedAliases.Contains(aliasSym)) + continue; + // a plain `var x = _field;` / `var x = this._field;` alias, OR the + // `Interlocked.Exchange(ref _field, null)` teardown whose result IS the + // field's just-detached owned object — both make `x` an alias of the field. + string? af = ThisFieldName(init) is { } direct + && model.GetSymbolInfo(init).Symbol is IFieldSymbol + ? direct + : RefExchangeNulledField(init, model); + if (af is not null) aliasToField[aliasSym] = af; + } // a `.Dispose()`/`.DisposeAsync()`/`.Close()` on a field — directly (`_f.Dispose()` / `this._f.…`) // or through an alias local (translated by SYMBOL via aliasToField) — releases that field. // `Close()` counts as a release here exactly as it already does for LOCAL disposables (DisposesLocal From 964369ce8699dd5fa1aa966e488a059d89874d68 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 17:53:16 +0000 Subject: [PATCH 13/16] oracle: confirm RefExchangeNulledField on NLog (own-only 4->3), restore scaffolding Live NLog oracle re-run confirms the Interlocked.Exchange teardown fix clears the last timer FP: Own.NET leak-class total 4 -> 3, own-only is now just the three genuine benign-but-real undisposed fields (XmlParser _xmlSource, FileTarget _reusable*Stream), TimeoutContinuation _timeoutTimer absent from both own-only and the baselined section, and the report has no Known-FP section at all (NLog baseline empty). Restore the dev scaffolding: oracle-target.txt back to the systemevents-console fixture, remove the branch from the oracle.yml push filter. Full arc: NLog FP baseline 5 -> 0, own-only 8 -> 3, every WaitForDispose timer FP closed at the source (4 by CallReleasesReceiver, 1 by RefExchangeNulledField). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- .github/workflows/oracle.yml | 1 - corpus/oracle-target.txt | 6 ++---- 2 files changed, 2 insertions(+), 5 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 4cd74794..52cb6d64 100644 --- a/corpus/oracle-target.txt +++ b/corpus/oracle-target.txt @@ -13,7 +13,5 @@ # 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. -# [temporary] pointed at NLog to confirm RefExchangeNulledField clears the last timer -# FP (TimeoutContinuation); restored to the systemevents-console fixture after the run. -NLog/NLog -include_tests=false +local:corpus/fixtures/systemevents-console +build=SystemEventsLeak.csproj From 3359edf08ea506561ce520e75ed0eb428c88683f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 19:00:22 +0000 Subject: [PATCH 14/16] extractor: self-owned-property is a collectable subscription source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A subscription whose event SOURCE is reached through a get-only property the class owns — this.Child.Event += this-capturing-handler, where Child is a property over a constructed field — is the same collectable self-cycle as subscribing on the owned field directly, not a leak. The self-owned exemption already covered this/fields/ locals the class constructs but not the PROPERTY receiver, so protobuf-net's CommandLineOptions (XsltOptions, a get-only property over a constructed XsltArgumentList, with a this-capturing handler) was falsely warned as an injected subscription leak. PropertyReturnsOwnedMember recognises a get-only property whose value the class owns: an auto-property '{ get; } = new T()', or a getter returning a constructed member (=> _owned / get => _owned / get { return _owned; }). Get-only is required — a settable property could be reassigned to an injected, longer-lived object, which we cannot prove bounded, so it falls through to the honest 'injected' warning (precision-first: never silently drop a real leak). New corpus fixture subscription-self-owned-property: before.cs subscribes to an injected bus (real OWN001 leak, caught), after.cs to a self-owned property (silent). Newtonsoft's serializer.Error (serializer = Create() that escapes, handler is a param's delegate) stays baselined: proving it non-leak needs real lifetime modelling, so the 'may outlive' warning is honest, not a clear FP. Re-pointing the oracle at protobuf to confirm the XsltOptions FP clears. 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 | 6 ++- corpus/oracle-target.txt | 6 ++- .../subscription-self-owned-property/after.cs | 20 ++++++++ .../before.cs | 19 ++++++++ .../subscription-self-owned-property/case.own | 19 ++++++++ .../expected-diagnostics.txt | 1 + .../subscription-self-owned-property/notes.md | 38 +++++++++++++++ frontend/roslyn/OwnSharp.Extractor/Program.cs | 47 ++++++++++++++++++- 9 files changed, 153 insertions(+), 4 deletions(-) create mode 100644 corpus/real-world/subscription-self-owned-property/after.cs create mode 100644 corpus/real-world/subscription-self-owned-property/before.cs create mode 100644 corpus/real-world/subscription-self-owned-property/case.own create mode 100644 corpus/real-world/subscription-self-owned-property/expected-diagnostics.txt create mode 100644 corpus/real-world/subscription-self-owned-property/notes.md 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-fp-baseline.txt b/corpus/oracle-fp-baseline.txt index 7648b296..3599dd32 100644 --- a/corpus/oracle-fp-baseline.txt +++ b/corpus/oracle-fp-baseline.txt @@ -33,7 +33,11 @@ # --- protobuf-net/protobuf-net -------------------------------------------------- protobuf-net/protobuf-net | ProtoWriter.BufferWriter.cs | OWN001 | _nullWriter | intentional null-object kept attached for pooled reuse; Dispose() documents "don't cascade dispose to the null one" protobuf-net/protobuf-net | ProtoTranscoder.cs | OWN001 | 'sync' | non-product (protobuf 'assorted' sample/extension tree); NetTranscoder is a long-lived singleton holding one ReaderWriterLockSlim for app lifetime -protobuf-net/protobuf-net | CommandLineOptions.cs | OWN001 | XsltMessageEncountered | non-product (assorted/ProtoGen) + self-subscription: publisher (xsltOptions) and the lambda subscriber are both owned by the same CommandLineOptions, co-lifetimed +# [fix-pending experiment] CommandLineOptions XsltMessageEncountered is cleared by the +# PropertyReturnsOwnedMember fix — the source `XsltOptions` is a get-only property over a +# constructed field, a self-cycle. Disabled here to confirm on a live protobuf oracle run +# that it no longer fires; if confirmed it is deleted, if it resurfaces it is restored. +# protobuf-net/protobuf-net | CommandLineOptions.cs | OWN001 | XsltMessageEncountered | non-product (assorted/ProtoGen) + self-subscription: publisher (xsltOptions) and the lambda subscriber are both owned by the same CommandLineOptions, co-lifetimed 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 # --- JamesNK/Newtonsoft.Json ---------------------------------------------------- diff --git a/corpus/oracle-target.txt b/corpus/oracle-target.txt index 52cb6d64..25b0856c 100644 --- a/corpus/oracle-target.txt +++ b/corpus/oracle-target.txt @@ -13,5 +13,7 @@ # 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] pointed at protobuf-net to confirm PropertyReturnsOwnedMember clears the +# CommandLineOptions XsltMessageEncountered self-cycle FP; restored after the run. +protobuf-net/protobuf-net +include_tests=false diff --git a/corpus/real-world/subscription-self-owned-property/after.cs b/corpus/real-world/subscription-self-owned-property/after.cs new file mode 100644 index 00000000..3be81561 --- /dev/null +++ b/corpus/real-world/subscription-self-owned-property/after.cs @@ -0,0 +1,20 @@ +using System; + +class Bus { public event EventHandler? Changed; } + +// FIX: the watcher now OWNS the bus — a get-only property over a field it constructs. +// The subscription is therefore a self-cycle: the Watcher, the owned Bus, and the +// handler form one object graph the GC collects together. Not a leak, even with no +// `-=` (mined on protobuf-net's CommandLineOptions, where `XsltOptions` is a get-only +// property over a constructed XsltArgumentList field and the handler captures `this`). +sealed class Watcher +{ + readonly Bus _bus = new Bus(); // constructed — owned by this + Bus Channel => _bus; // get-only property returning the owned field + int _count; + + public Watcher() + { + Channel.Changed += (s, e) => _count++; // self-owned source -> silent + } +} diff --git a/corpus/real-world/subscription-self-owned-property/before.cs b/corpus/real-world/subscription-self-owned-property/before.cs new file mode 100644 index 00000000..53ec5ba8 --- /dev/null +++ b/corpus/real-world/subscription-self-owned-property/before.cs @@ -0,0 +1,19 @@ +using System; + +class Bus { public event EventHandler? Changed; } + +// BUG: the watcher subscribes a `this`-capturing handler to an INJECTED bus — an +// external object of unknown, potentially longer lifetime — and never detaches it. +// The bus's handler list keeps the Watcher alive for the bus's lifetime: a real +// subscription leak (the source may outlive `this`). +sealed class Watcher +{ + readonly Bus _bus; // injected — lifetime owned by someone else + int _count; + + public Watcher(Bus bus) + { + _bus = bus; + _bus.Changed += (s, e) => _count++; // never -= : leak + } +} diff --git a/corpus/real-world/subscription-self-owned-property/case.own b/corpus/real-world/subscription-self-owned-property/case.own new file mode 100644 index 00000000..7d2664e8 --- /dev/null +++ b/corpus/real-world/subscription-self-owned-property/case.own @@ -0,0 +1,19 @@ +// OwnLang model of the self-owned-property subscription discrimination, mined on +// protobuf-net's CommandLineOptions: a `this`-capturing handler is subscribed to an +// event on a source the component OWNS (a get-only property over a constructed field), +// which is a collectable self-cycle, NOT a leak. before.cs is the leaky counterpart — +// the same subscription on an INJECTED bus (external lifetime), never detached — the +// generic OWN001 subscription leak, modelled here as a Subscription acquire with no +// release. The extractor tells the two apart by resolving the event source: an owned +// member (this/field/get-only-owned-property) is dropped (self-cycle), an injected one +// stays a warning. See notes.md for the recognition rule (PropertyReturnsOwnedMember). +module Corpus +resource Subscription { + acquire Subscribe + release Unsubscribe + kind "subscription token" +} +fn Watch(bus: int) { + let token = acquire Subscription(bus); // _bus.Changed += (s, e) => _count++ + // no `release token;` — the injected bus is never `-=`'d -> handler leak (OWN001) +} diff --git a/corpus/real-world/subscription-self-owned-property/expected-diagnostics.txt b/corpus/real-world/subscription-self-owned-property/expected-diagnostics.txt new file mode 100644 index 00000000..ed2a1929 --- /dev/null +++ b/corpus/real-world/subscription-self-owned-property/expected-diagnostics.txt @@ -0,0 +1 @@ +OWN001 diff --git a/corpus/real-world/subscription-self-owned-property/notes.md b/corpus/real-world/subscription-self-owned-property/notes.md new file mode 100644 index 00000000..79550c77 --- /dev/null +++ b/corpus/real-world/subscription-self-owned-property/notes.md @@ -0,0 +1,38 @@ +# subscription-self-owned-property + +An event subscription whose SOURCE is a member the component **owns** — accessed +through a get-only **property** — is a collectable self-cycle, not a leak. Mined on +protobuf-net's `CommandLineOptions`: + +```csharp +private readonly XsltArgumentList xsltOptions = new XsltArgumentList(); +public XsltArgumentList XsltOptions => xsltOptions; // get-only, owned +... +XsltOptions.XsltMessageEncountered += delegate { messageCount++; }; // handler captures `this` +``` + +The owned `XsltArgumentList`, the `CommandLineOptions` instance, and the handler form +one object graph the GC collects together — no `-=` needed. + +- **before.cs** — the same subscription on an **injected** `Bus` (external, unknown + lifetime), never detached → a real subscription leak (`OWN001`, warning: the source + may outlive `this`). +- **after.cs** — the source is now a **get-only property over a constructed field** the + component owns → self-cycle → **silent**. + +## Recognition rule + +The self-owned-source exemption already drops a subscription whose source is `this`, or +a field/local the class constructs (`owned`). This case adds the missing receiver shape: +a `this`-instance **get-only property** whose value the class owns — +`PropertyReturnsOwnedMember`: + +- an auto-property `public T X { get; } = new T();` (value constructed in place), or +- a getter that returns a constructed member: `=> _owned`, `get => _owned`, or + `get { return _owned; }` where the returned field/property is in `owned`. + +**Get-only is required.** A settable property could be reassigned to an injected, +longer-lived object after construction, which we cannot prove bounded — so a property +with any setter falls through to the honest "injected" warning (precision-first: never +silently drop a real leak). Computed getters and getters returning a parameter/injected +field likewise fall through. diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 8b1c4d5f..11ad75d9 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -486,7 +486,52 @@ static bool IsSelfOwnedSource(ExpressionSyntax left, IEventSymbol ev, if (m.Expression is ThisExpressionSyntax) return true; var recv = model.GetSymbolInfo(m.Expression).Symbol; - return (recv is IFieldSymbol or ILocalSymbol) && owned.Contains(recv.Name); + if ((recv is IFieldSymbol or ILocalSymbol) && owned.Contains(recv.Name)) + return true; + // A `this`-instance get-only PROPERTY whose value the class owns (`this.Child.Event + // += h`, Child a `=> _owned` / `{ get; } = new()` property) is the SAME collectable + // self-cycle as the owned field directly — this, the owned child, and the handler + // are collected together. The property must resolve to a member the class constructs. + return recv is IPropertySymbol { IsStatic: false } p + && PropertyReturnsOwnedMember(p, owned, model); +} + +// A GET-ONLY property whose value the class OWNS: an auto-property initialised to +// `new X()`, or one whose getter returns a field/property the class constructs +// (`=> _owned`, `get => _owned`, `get { return _owned; }`). Such a property is part of +// `this`'s own object graph, so a subscription on its event is the same collectable +// self-cycle as subscribing on the owned field. GET-ONLY is required: a settable +// property could be reassigned to an INJECTED, longer-lived object after construction, +// which we cannot prove bounded — so we decline it (precision-first: never silently +// drop a real leak). Only the dominant owned-property shapes are recognised; anything +// else (a computed getter, a getter returning a parameter/injected field) falls through. +static bool PropertyReturnsOwnedMember(IPropertySymbol prop, HashSet owned, + SemanticModel model) +{ + if (prop.SetMethod is not null) + return false; + foreach (var sref in prop.DeclaringSyntaxReferences) + { + if (sref.GetSyntax() is not PropertyDeclarationSyntax pd) + continue; + // auto-property `public T X { get; } = new T();` — the value is constructed here. + if (pd.Initializer?.Value is ObjectCreationExpressionSyntax + or ImplicitObjectCreationExpressionSyntax) + return true; + // `=> expr`, `get => expr`, or `get { return expr; }` returning an owned member. + var get = pd.AccessorList?.Accessors + .FirstOrDefault(ac => ac.IsKind(SyntaxKind.GetAccessorDeclaration)); + var returned = pd.ExpressionBody?.Expression + ?? get?.ExpressionBody?.Expression + ?? get?.Body?.Statements.OfType().FirstOrDefault()?.Expression; + if (returned is null) + continue; + var pm = model.Compilation.GetSemanticModel(pd.SyntaxTree); + var rsym = pm.GetSymbolInfo(returned).Symbol; + if (rsym is IFieldSymbol or IPropertySymbol && owned.Contains(rsym.Name)) + return true; + } + return false; } // P-004 (ext): a control fetching one of its OWN template parts — From edbf20318488c1bba266818e9a25fa1393af1943 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 19:07:45 +0000 Subject: [PATCH 15/16] oracle: confirm self-owned-property fix on protobuf, trim baseline + restore scaffolding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live protobuf oracle re-run confirms PropertyReturnsOwnedMember clears the CommandLineOptions XsltMessageEncountered self-cycle FP at the source: own-only 0, the finding absent from both own-only and the baselined section. Delete its baseline entry (protobuf baseline 5 -> 4 findings) and document the fix landed (root-cause #3 PARTLY FIXED). Restore the dev scaffolding: oracle-target.txt back to the systemevents-console fixture, remove the branch from the oracle.yml push filter. Newtonsoft serializer.Error stays baselined as an honest 'may outlive' warning (its source escapes and the handler is a parameter's delegate — non-leak is unprovable without lifetime modelling). 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 | 9 ++++--- corpus/oracle-target.txt | 6 ++--- docs/notes/oracle-known-fps.md | 43 ++++++++++++++++++++++++---------- 4 files changed, 37 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 3599dd32..d112123f 100644 --- a/corpus/oracle-fp-baseline.txt +++ b/corpus/oracle-fp-baseline.txt @@ -33,11 +33,10 @@ # --- protobuf-net/protobuf-net -------------------------------------------------- protobuf-net/protobuf-net | ProtoWriter.BufferWriter.cs | OWN001 | _nullWriter | intentional null-object kept attached for pooled reuse; Dispose() documents "don't cascade dispose to the null one" protobuf-net/protobuf-net | ProtoTranscoder.cs | OWN001 | 'sync' | non-product (protobuf 'assorted' sample/extension tree); NetTranscoder is a long-lived singleton holding one ReaderWriterLockSlim for app lifetime -# [fix-pending experiment] CommandLineOptions XsltMessageEncountered is cleared by the -# PropertyReturnsOwnedMember fix — the source `XsltOptions` is a get-only property over a -# constructed field, a self-cycle. Disabled here to confirm on a live protobuf oracle run -# that it no longer fires; if confirmed it is deleted, if it resurfaces it is restored. -# protobuf-net/protobuf-net | CommandLineOptions.cs | OWN001 | XsltMessageEncountered | non-product (assorted/ProtoGen) + self-subscription: publisher (xsltOptions) and the lambda subscriber are both owned by the same CommandLineOptions, co-lifetimed +# CommandLineOptions XsltMessageEncountered was here (self-cycle: source `XsltOptions` is +# 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. 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 # --- JamesNK/Newtonsoft.Json ---------------------------------------------------- diff --git a/corpus/oracle-target.txt b/corpus/oracle-target.txt index 25b0856c..52cb6d64 100644 --- a/corpus/oracle-target.txt +++ b/corpus/oracle-target.txt @@ -13,7 +13,5 @@ # 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. -# [temporary] pointed at protobuf-net to confirm PropertyReturnsOwnedMember clears the -# CommandLineOptions XsltMessageEncountered self-cycle FP; restored after the run. -protobuf-net/protobuf-net -include_tests=false +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 582ce7ba..1aa8da53 100644 --- a/docs/notes/oracle-known-fps.md +++ b/docs/notes/oracle-known-fps.md @@ -23,17 +23,28 @@ reason: we and the oracles occupy orthogonal niches. | disposition | count | what happens on re-run | |---|---:|---| -| **Fixed in the extractor** | 5 | no longer fire (all NLog `WaitForDispose` timers — see below) | -| **Baselined FP** | 7 | moved to "Known FP (baselined)", out of the triage queue | +| **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 | | **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 | -The 7 baselined FPs + the 2 non-product-sample reals = 9 findings, covered by -**8 rules** in `corpus/oracle-fp-baseline.txt` (the two `NetTranscoder` copies +The 6 baselined FPs + the 2 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. +**Update (extractor fix landed — protobuf self-cycle).** `CommandLineOptions.XsltOptions. +XsltMessageEncountered` — a `this`-capturing handler subscribed to an event on +`XsltOptions`, a get-only property over a constructed field the class owns — is now +**fixed at the source** by `PropertyReturnsOwnedMember` (the self-owned-source exemption +now covers a property receiver, not just `this`/fields/locals). A live protobuf re-run +confirmed it: own-only **0**, the finding absent from own-only and baselined. See +root-cause #3. Corpus fixture: `subscription-self-owned-property`. Newtonsoft's +`serializer.Error` stays baselined — its source escapes (a returned `Create()` result) +and the handler is a parameter's delegate, so proving non-leak needs lifetime modelling; +the "may outlive" warning is honest, not a clear FP. + **Update (extractor fix landed — all NLog timers).** All 5 of the original NLog `WaitForDispose` timer FPs are now **fixed at the source**, baseline entries deleted. Four (`AsyncTaskTarget._taskTimeoutTimer`/`_lazyWriterTimer`, @@ -147,14 +158,22 @@ entry and let the oracle re-confirm clean. structurally a no-op AND not worth surfacing. Revisit whether benign-managed field leaks should be downgraded as a class. -3. **Lifetime-unaware subscription** *(protobuf `XsltOptions`, Newtonsoft - `serializer.Error` self-subscriptions; CsvHelper process-lived host).* OWN014's - premise — a long-lived source outlives a shorter-lived subscriber — fails when - publisher and subscriber are **co-lifetimed** (same owner, or both freshly - created in one call) or the subscriber is **itself process-lived**. *Fix:* - compare subscriber vs. source lifetime (extend the `source: "self"` handling to - co-created locals and to process-lived hosts) rather than keying on "source is - static/injected". See [`subscription-leaks-and-profiles.md`](subscription-leaks-and-profiles.md). +3. **Lifetime-unaware subscription — PARTLY FIXED** *(was protobuf `XsltOptions`, + Newtonsoft `serializer.Error`; CsvHelper process-lived host).* OWN014's premise — + a long-lived source outlives a shorter-lived subscriber — fails when publisher and + subscriber are **co-lifetimed** or the subscriber is **itself process-lived**. + **Shipped fix:** `PropertyReturnsOwnedMember` extends the self-owned-source exemption + to a **property** receiver — `this.OwnedProp.Event += handler`, where `OwnedProp` is a + get-only property over a member the class constructs, is the same collectable + self-cycle as the owned field directly (get-only required: a settable property could + be reassigned to an injected object). Cleared protobuf `XsltOptions` on a live re-run + (own-only 0); corpus fixture `subscription-self-owned-property`. **Still open:** + Newtonsoft `serializer.Error` — the source is a returned `Create()` result (escapes) + and the handler is a parameter's delegate; the source's lifetime relative to the + handler is genuinely unprovable syntactically, so the "may outlive" warning is honest + (baselined, not a clear FP). CsvHelper's process-lived host needs a "subscriber is + itself process-lived" signal — still open. See + [`subscription-leaks-and-profiles.md`](subscription-leaks-and-profiles.md). 4. **Non-product trees** *(protobuf `assorted/`, CsvHelper `docs-src/`, protobuf `BuildToolsUnitTests/`).* Sample/extension/doc-generator code that was never From 74fcbc131c68681002c7a9d40851fb82f185d6f0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 28 Jun 2026 04:44:03 +0000 Subject: [PATCH 16/16] review: harden self-owned-property exemption + baseline parsing (PR #156) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the Codex/CodeRabbit review on #156: - IsSelfOwnedSource: gate the get-only-property exemption to a THIS/bare access (`Channel` / `this.Channel`). `other.Channel.Event += h` reaches another instance's property, which may outlive this subscriber, so it stays flagged (Codex P2). - PropertyReturnsOwnedMember: require the returned member's CONTAINING TYPE to be the analyzed class, not just a name match — a same-named field on another type no longer satisfies ownership (CodeRabbit, major). - RefExchangeNulledField: also accept `default(T)` (DefaultExpressionSyntax), not only the `default` literal — `Interlocked.Exchange(ref _f, default(Timer))` was missed (CodeRabbit, minor). - oracle_compare _load_baseline: reject an empty message-substring (it would make `substr in message` a wildcard a typo could turn into a blanket suppressor), and preserve `|` in the reason via join (CodeRabbit, major). Selftest extended (31 checks). - field-dispose-via-helper/notes.md: drop the obsolete caveat — the Interlocked.Exchange teardown is now covered by the field-dispose-via-exchange fixture, not baselined (CodeRabbit, minor). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- .../field-dispose-via-helper/notes.md | 17 +++++----- frontend/roslyn/OwnSharp.Extractor/Program.cs | 31 +++++++++++++------ scripts/oracle_compare.py | 19 ++++++++---- 3 files changed, 42 insertions(+), 25 deletions(-) diff --git a/corpus/real-world/field-dispose-via-helper/notes.md b/corpus/real-world/field-dispose-via-helper/notes.md index ae5cbe6c..3a8f0c90 100644 --- a/corpus/real-world/field-dispose-via-helper/notes.md +++ b/corpus/real-world/field-dispose-via-helper/notes.md @@ -31,12 +31,11 @@ parameter. So an unknown or borrowing callee never credits a release. This clears the NLog variants where the sink is called on the field **directly** or through a simple `var t = _field;` alias (`AsyncTaskTarget`, `AsyncTargetWrapper`, -`BufferingTargetWrapper`). It does **not** reach `TimeoutContinuation`, where the -receiver is the result of `Interlocked.Exchange(ref _timer, null)` — an -exchange-and-null-out idiom whose result is not a tracked field alias. That one -stays in the oracle FP baseline -([`corpus/oracle-fp-baseline.txt`](../../oracle-fp-baseline.txt), -[`docs/notes/oracle-known-fps.md`](../../../docs/notes/oracle-known-fps.md)) until -ref-exchange alias tracking lands. Scope is intentionally limited to **extension -methods**: an instance method disposing its own `this` is not a real -dispose-delegation shape and would drag in virtual-dispatch reasoning. +`BufferingTargetWrapper`). The `TimeoutContinuation` variant — where the receiver is +the result of `Interlocked.Exchange(ref _timer, null)`, the detach-and-dispose +teardown — is covered separately by the sibling +[`field-dispose-via-exchange`](../field-dispose-via-exchange/notes.md) fixture +(`RefExchangeNulledField` binds the exchange result to the field, then this same sink +recognition applies). Scope is intentionally limited to **extension methods**: an +instance method disposing its own `this` is not a real dispose-delegation shape and +would drag in virtual-dispatch reasoning. diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 11ad75d9..6976abf4 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -479,7 +479,8 @@ left is MemberAccessExpressionSyntax m // text), and `owned` is AST-based — not a regex. NOTE: callers must exclude timers // — a *running* timer is rooted by the dispatcher regardless of who owns the field. static bool IsSelfOwnedSource(ExpressionSyntax left, IEventSymbol ev, - SemanticModel model, HashSet owned) + SemanticModel model, HashSet owned, + ISymbol? cls) { if (left is not MemberAccessExpressionSyntax m) return !ev.IsStatic; // bare event => an instance event on `this` @@ -488,12 +489,15 @@ static bool IsSelfOwnedSource(ExpressionSyntax left, IEventSymbol ev, var recv = model.GetSymbolInfo(m.Expression).Symbol; if ((recv is IFieldSymbol or ILocalSymbol) && owned.Contains(recv.Name)) return true; - // A `this`-instance get-only PROPERTY whose value the class owns (`this.Child.Event - // += h`, Child a `=> _owned` / `{ get; } = new()` property) is the SAME collectable - // self-cycle as the owned field directly — this, the owned child, and the handler - // are collected together. The property must resolve to a member the class constructs. + // A get-only PROPERTY over a member the class owns (`this.Child.Event += h`, Child a + // `=> _owned` / `{ get; } = new()` property) is the SAME collectable self-cycle as the + // owned field directly. GATED on a `this`/bare access (`Channel` / `this.Channel`): + // `other.Channel.Event += h` reaches ANOTHER instance's property, which may outlive + // this subscriber and retain the handler, so it stays flagged (Codex P2). return recv is IPropertySymbol { IsStatic: false } p - && PropertyReturnsOwnedMember(p, owned, model); + && m.Expression is (IdentifierNameSyntax + or MemberAccessExpressionSyntax { Expression: ThisExpressionSyntax }) + && PropertyReturnsOwnedMember(p, owned, model, cls); } // A GET-ONLY property whose value the class OWNS: an auto-property initialised to @@ -506,7 +510,7 @@ static bool IsSelfOwnedSource(ExpressionSyntax left, IEventSymbol ev, // drop a real leak). Only the dominant owned-property shapes are recognised; anything // else (a computed getter, a getter returning a parameter/injected field) falls through. static bool PropertyReturnsOwnedMember(IPropertySymbol prop, HashSet owned, - SemanticModel model) + SemanticModel model, ISymbol? cls) { if (prop.SetMethod is not null) return false; @@ -528,7 +532,13 @@ static bool PropertyReturnsOwnedMember(IPropertySymbol prop, HashSet own continue; var pm = model.Compilation.GetSemanticModel(pd.SyntaxTree); var rsym = pm.GetSymbolInfo(returned).Symbol; - if (rsym is IFieldSymbol or IPropertySymbol && owned.Contains(rsym.Name)) + // The returned member must be one the ANALYZED class declares — compare the + // symbol's CONTAINING TYPE, not just its name, so a same-named field on another + // type cannot satisfy ownership (CodeRabbit). `owned` holds only this class's + // constructed-field names, so the name check + the containing-type check agree. + if (rsym is IFieldSymbol or IPropertySymbol + && owned.Contains(rsym.Name) + && (cls is null || SymbolEqualityComparer.Default.Equals(rsym.ContainingType, cls))) return true; } return false; @@ -2582,7 +2592,8 @@ static bool CallReleasesReceiver(IMethodSymbol? sym, SemanticModel model) if (args.Count != 2 || !args[0].RefKindKeyword.IsKind(SyntaxKind.RefKeyword) || !(args[1].Expression.IsKind(SyntaxKind.NullLiteralExpression) - || args[1].Expression.IsKind(SyntaxKind.DefaultLiteralExpression))) + || args[1].Expression.IsKind(SyntaxKind.DefaultLiteralExpression) + || args[1].Expression is DefaultExpressionSyntax)) // default(T) for a ref type is null too return null; return ThisFieldName(args[0].Expression) is { } f && model.GetSymbolInfo(args[0].Expression).Symbol is IFieldSymbol @@ -3296,7 +3307,7 @@ or ImplicitObjectCreationExpressionSyntax // intent, not a leak (mined: Npgsql PoolManager's `AppDomain.CurrentDomain. // ProcessExit += (_,_) => ClearAll()` shutdown hook). A handler that captures // instance state still pins it to the process, so it stays OWN014 (Codex). - if (!isTimer && (IsSelfOwnedSource(a.Left, ev, model, selfOwned) + if (!isTimer && (IsSelfOwnedSource(a.Left, ev, model, selfOwned, clsSymbol) || IsStaticHandler(a.Right, model) || (IsProcessLifetimeAppDomainEvent(ev) && HandlerRetainsNoInstance(a.Right, model)))) diff --git a/scripts/oracle_compare.py b/scripts/oracle_compare.py index 878ade45..9342befa 100644 --- a/scripts/oracle_compare.py +++ b/scripts/oracle_compare.py @@ -261,10 +261,13 @@ def _load_baseline(path: str) -> list[BaselineRule]: if not s or s.startswith("#"): continue parts = [p.strip() for p in s.split("|")] - if len(parts) < 4: + # need all four key fields, and a NON-EMPTY message-substring: an empty substr + # would make `substr in message` always true — a wildcard that a typo could turn + # into a blanket suppressor for that file/rule (CodeRabbit). Reason keeps any `|`. + if len(parts) < 4 or not parts[3]: continue repo, basename, rule, substr = parts[:4] - reason = parts[4] if len(parts) > 4 else "" + reason = " | ".join(parts[4:]).strip() if len(parts) > 4 else "" rules.append(BaselineRule(repo, basename, rule, substr, reason)) return rules @@ -769,14 +772,18 @@ def _selftest() -> int: # the loader: comments/blank lines ignored, <4 fields skipped, reason optional. import tempfile with tempfile.NamedTemporaryFile("w", suffix=".txt", delete=False) as tf: - tf.write("# comment\n\no/r | B.cs | OWN001 | local 'b' | a reason\n" - "too | few | fields\n") + tf.write("# comment\n\no/r | B.cs | OWN001 | local 'b' | a reason | with a pipe\n" + "too | few | fields\n" + "o/r | B.cs | OWN001 | | empty substr is a wildcard — must be skipped\n") tf.flush() loaded = _load_baseline(tf.name) - if len(loaded) != 1 or loaded[0].reason != "a reason": + # the <4-field line and the empty-substring line are both skipped; reason keeps its `|`. + if len(loaded) != 1 or loaded[0].reason != "a reason | with a pipe": fails.append(f"baseline loader wrong: {loaded}") + if any(r.substr == "" for r in loaded): + fails.append("baseline loader must reject an empty message-substring (wildcard)") - total = 30 + total = 31 for f in fails: print(f"ORACLE SELFTEST FAIL: {f}") print(f"oracle_compare selftest: {total - len(fails)}/{total} checks passed")