Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions docs/notes/oracle-known-fps.md
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,48 @@ entry and let the oracle re-confirm clean.
`docs-src/`) are handled per-entry in the baseline rather than by polluting the
generic `_is_test_path` with repo-specific directory names.

## Rejected approaches

### Static-class subscriber exemption (the CsvHelper `ConsoleHost` over-reach)

**Attempted in PR #157, reverted in `488d505` before merge. Do not retry.**

To clear the two CsvHelper `ConsoleHost` FPs (§3, root-cause 3 — a process-lived
host subscribing to a process-lived `AppDomain`/`Console` event), the tempting move
was to add `|| clsIsStatic` next to the existing `clsIsApp` exemption in the
extractor (`Program.cs`, the `if (!isTimer && source == "static" && clsIsApp)`
drop): "the subscriber's containing type is a `static class`, so there's no instance
to leak — drop the OWN014 the same way we drop it for the WPF `App` singleton."

**Why it is unsound.** A `static class` only rules out an instance `this` being
pinned. It says **nothing** about a lambda handler that captures a **local**. When
the source is a static/process-lived event, that captured local is pinned for the
whole process — a genuine leak. The exemption would silently swallow it:

```csharp
static class Foo {
void Attach(VM vm) =>
SystemEvents.UserPreferenceChanged += (_, _) => vm.Refresh(); // pins vm forever
}
```

`clsIsApp` is safe where `clsIsStatic` is not: the WPF `App` singleton *is* the
process-lived object, so promoting its own subscriptions to process lifetime changes
nothing; a static class is just a namespace for methods whose lambdas can still
capture and pin arbitrary shorter-lived state.

**Caught by:** Codex (P2) and CodeRabbit (Major) in review of #157, before merge.

**Why a sound narrowing still wouldn't help here.** A capture-gated version
("exempt only when the handler captures nothing") would be sound — but it would
**not** clear the motivating case: CsvHelper's `ConsoleHost` handlers capture `cts`
and `resetEvent`, so the capture-free guard would (correctly) keep firing. The clear
verdict is "this specific host is process-lived" (the subscriber's own lifetime),
which we have no reliable signal for. So those two findings stay in
`corpus/oracle-fp-baseline.txt` as baselined FPs rather than being suppressed by an
extractor rule. An in-code `ANTI-PATTERN` comment at the exemption site warns against
re-adding `|| clsIsStatic`.

## How the baseline stays honest

- **Matched by name, not line** — `(repo, file-basename, OWN code,
Expand Down
16 changes: 16 additions & 0 deletions frontend/roslyn/OwnSharp.Extractor/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3143,7 +3143,7 @@
.Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries)
.Where(p => p.EndsWith(".dll", StringComparison.OrdinalIgnoreCase))
.ToList();
var refNames = new HashSet<string>(tpa.Select(Path.GetFileName), StringComparer.OrdinalIgnoreCase);

Check warning on line 3146 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / own-check SARIF -> GitHub code scanning (dog-food)

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 3146 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / P-014 Tier B — external reference resolution (--ref-dir)

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 3146 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 3146 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / C# leak extractor (Roslyn) -> OwnIR -> core

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 3146 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / P-014 Tier B — external reference resolution (--ref-dir)

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 3146 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / own-check SARIF -> GitHub code scanning (dog-food)

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 3146 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 3146 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / C# leak extractor (Roslyn) -> OwnIR -> core

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.
var references = tpa.Select(p => (MetadataReference)MetadataReference.CreateFromFile(p)).ToList();
// P-004 WPF profile: widen the reference set with assemblies named by the
// OWN_EXTRA_REF_DIRS env var (colon-separated dirs) — e.g. the WindowsDesktop ref
Expand Down Expand Up @@ -3325,6 +3325,22 @@
// process — so the region escape (OWN014) is a false positive. Scoped
// to NON-timers: a timer is forced to source "static" above, but a
// never-stopped timer in `App` is still a real leak (CodeRabbit).
//
// ANTI-PATTERN — do NOT broaden this to `|| clsIsStatic` (subscriber is
// a static class). It looks symmetric with `clsIsApp` but is UNSOUND: a
// static class only rules out an instance `this` being pinned; it does
// NOTHING about a lambda handler that captures a *local*. A static-source
// (process-lived) event then pins that captured local for the whole
// process — a genuine leak this exemption would silently swallow, e.g.
// static class Foo {
// void Attach(VM vm) =>
// SystemEvents.UserPreferenceChanged += (_,_) => vm.Refresh(); // pins vm forever
// }
// This was tried in PR #157 and caught pre-merge by Codex (P2) + CodeRabbit
// (Major); reverted in 488d505. A capture-gated narrowing was considered but
// would NOT clear the motivating case (CsvHelper ConsoleHost captures cts/
// resetEvent), so it stays in corpus/oracle-fp-baseline.txt instead. Full
// write-up: docs/notes/oracle-known-fps.md → "Rejected approaches".
if (!isTimer && source == "static" && clsIsApp)
continue;
var released = unsub.Contains($"{a.Left}|{a.Right}")
Expand Down
Loading