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
56 changes: 55 additions & 1 deletion docs/notes/field-notes-patterns.md
Original file line number Diff line number Diff line change
Expand Up @@ -551,12 +551,21 @@ exemption's real criterion is "does this object's lifetime start and end with th
subscriber's" — a base-class accessor to the attached object, or an item of an
owned collection, satisfies that just as well as a constructed field.**

**Status (2026-07):** shapes **(a)** and **(c)** shipped.
**Status (2026-07):** all three sub-shapes **(a)**, **(b)**, **(c)** shipped —
parent issue #221 closed once all three landed.
- **(a)** the `Behavior.AssociatedObject` self-owned source, shipped in #227 —
extractor `IsAssociatedObjectSource`, gated on the `Behavior` base
(`IsBehaviorSubscriber`) plus a same-class assignment-chain resolving to
`this.AssociatedObject` (`ResolvesToAssociatedObject`); pinned by
`frontend/roslyn/samples/AssociatedObjectSourceSample.cs`.
- **(b)** the curated app-scoped source for an `Application`-derived
subscriber, shipped in #228 (PR #232) — extractor keeps the existing
`clsIsApp` subscriber gate byte-for-byte and only loosens the *source*
check to a curated resolver allowlist (`PaletteHelper.GetThemeManager`),
method-group handlers only (no lambdas); see
`docs/notes/oracle-known-fps.md` "What DID ship next to it (issue #228)"
for why this is not the rejected `clsIsStatic` broadening. Pinned by
`frontend/roslyn/samples/AppScopedSourceSample.cs`.
- **(c)** the element of a self-populated collection, shipped in #229 — extractor
`IsOwnedCollectionElementSource`: a `foreach` loop variable whose collection is a
this-owned field/property populated ONLY from own construction / an own factory
Expand Down Expand Up @@ -694,6 +703,51 @@ existing fixtures that had used an empty `Dispose()` as a modelling shortcut
(`UnitOfWork`, `PixelOwner`) were made faithful (a real Dispose body) so they
remain genuine leak fixtures.

**Superseded by a soundness regression (issue #238) — gate narrowed to
enumerators (issue #240 / PR #240).** The re-measure in
`docs/notes/precision-remeasure-2026-07-11.md` (PR #235) ran the shipped \#225
exemption above against real ClosedXML and found it unsound:
`XLWorkbook.Dispose()` is empty **in source only** — `Janitor.Fody` (an IL
weaver declared in ClosedXML's `FodyWeavers.xml`) rewrites the body at
compile time to call real cleanup (`DisposeManaged()` →
`Worksheets.ForEach(w => (w as XLWorksheet).Cleanup())`). `HasEmptyDisposeBody`
read the source-level emptiness as proof of "nothing to release" and
silently exempted 263 unrelated `XLWorkbook` locals across
`ClosedXML.Examples` — a real leak class going quiet, not a false-positive
fix. This inverts the analyzer's core doctrine (an exemption's worst case
must stay "keeps today's honest warning," never "silently swallows a leak
class"), so it was treated as a bug, not a precision tweak.

The fix (#240) **narrowed** the gate rather than special-casing weavers:
`HasEmptyDisposeBody`'s exemption now applies **only** to types that
implement the *generic* `System.Collections.Generic.IEnumerator<T>` — the
one interface that *forces* a (frequently no-op) `Dispose()` implementation
via `IEnumerator<T> : IEnumerator, IDisposable`, which was the entire
motivating shape for #225 in the first place. The non-generic
`System.Collections.IEnumerator` does **not** extend `IDisposable`, so it
does not qualify. `XLWorkbook` and every other weaver-augmented domain type
fall outside the gate and keep the honest warning. Defense in depth: a
`FodyWeavers.xml` found above the source (or above the owning project, for
linked sources) disables the exemption even for a qualifying enumerator.
Explicit-interface `Dispose()` is now recognized as empty too (closing the
coverage gap that left 3 of 5 `Slice.cs` sites unexempted) — `DisposeAsync()`
is the opposite of an exemption: ANY `IAsyncDisposable` (declared or
inherited) or bare `DisposeAsync()` method (declared, explicit-impl, or
inherited from a base) unconditionally **disqualifies** the exemption, since
async disposal means the type is no longer the "simple enumerator holding
nothing" the rule is scoped to. Two review rounds closed further soundness
holes: inherited `IAsyncDisposable` (via `AllInterfaces`), inherited bare
`DisposeAsync` (via base-chain walk), a sticky static-registry cache, and a
fail-open weaver-detection path (`File.GetAttributes` instead of
`File.Exists`, which silently disabled the guard when the check itself
errored). Confirmed on real ClosedXML (HEAD `4e89dce`, `--flow-locals`):
121 of the swallowed findings restored, 0 removed. See issue #238 and PR \#240
for the full record — this is a deliberate historical lesson, not
cleaned up: **source-level emptiness is not a runtime no-op once a
compile-time IL weaver is in play, and a broad "any empty Dispose" gate
cannot rule that out — only a gate scoped to a specific
compiler/language-forced shape (here: `IEnumerator<T>`) can.**

---

## The through-line
Expand Down
22 changes: 20 additions & 2 deletions docs/notes/precision-remeasure-2026-07-11.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ OSS repos: #218-#225. Four follow-up PRs have since shipped fixes:

- [#230](https://github.com/PhysShell/Own.NET/pull/230) — Closes **#218**
(not #219 — #219 is the separate WinForms `Controls`/`IContainer` disposal
channel gap, still open). DependencyProperty/property-changed old→new
subscription-rotation recognition.
channel gap, still open **as of this note**; shipped later via PR #236,
see `docs/notes/field-notes-patterns.md` entry 12/13). DependencyProperty/
property-changed old→new subscription-rotation recognition.
- [#231](https://github.com/PhysShell/Own.NET/pull/231) — four smaller gaps
in one PR: `CommandManager.RequerySuggested` allowlist (#223),
`using (field = new T())` release (#220), template-part local/pattern-var
Expand Down Expand Up @@ -213,3 +214,20 @@ This note's own instructions were read-only with respect to the analyzer —
no code was changed to investigate or produce the breakdown above; the
breakdown is entirely from comparing old/new SARIF output and reading the
target repos' real source.

## Resolution addendum (2026-07-11 / 2026-07-12)

Both follow-ups identified above were filed and shipped as a single unit:
issue [#238](https://github.com/PhysShell/Own.NET/issues/238) (soundness
regression — item 2 above, the ClosedXML 263-finding over-exemption) with
the explicit-interface coverage gap (item 1 above) folded into the same
fix, closed by [PR #240](https://github.com/PhysShell/Own.NET/pull/240).
The shipped fix took the narrowing direction (confine the exemption to
types implementing the generic `System.Collections.Generic.IEnumerator<T>`,
not the non-generic `IEnumerator`) rather than the weaver-convention-sniffing
heuristic sketched in item 2 — see
`docs/notes/field-notes-patterns.md` entry 19 for the full account,
including why the original "−5 Slice.cs points, 263 restored" acceptance
target from #238 was itself corrected (ClosedXML's own `FodyWeavers.xml`
covers `Slice.cs` too, so the sound outcome is "fully restored, nothing
silently dropped," not a fixed `−5`).
Loading