From 8630b4ddea6895c7ef630e22c1683dc46976e002 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 11 Jul 2026 05:18:15 +0000 Subject: [PATCH 1/2] fix(extractor): exempt locals of user types with a provably-empty Dispose() (#225) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A local of a user-defined type whose `Dispose()` body is literally empty was false-flagged as an OWN001 leak. Such a type (e.g. an `IEnumerator` enumerator) implements IDisposable only to satisfy an interface, not because it holds a resource — never calling its empty Dispose cannot leak anything. The existing no-op exemption (IsNoOpDisposeWrapper / IsDisposeOptional) only reached a handful of NAMED BCL types. Confirmed FP in ClosedXML, 5 sites, single root cause (Slice.Enumerator locals); issue #201 oracle sweep, field-notes entry 19. Fix (extractor-only, no OwnIR change): HasEmptyDisposeBody recognises from SOURCE a type whose parameterless `Dispose()` is a block body of zero statements and whose base chain owns no Dispose to cascade to, and drops a LOCAL of it from both the flat (IsDisposableType) and --flow-locals (ImplementsIDisposable) paths. Deliberately scoped to LOCALS: a FIELD keeps its own disposal contract, so the OwnIgnoreSample `Handle` field stand-in (an empty-Dispose type used as a leak control in #209) is unaffected. Precision guards, pinned by EmptyDisposeSample.cs in both the wpf-extractor (flat) and flow-locals CI jobs: a non-empty Dispose (RealResource, LeakyReader) still leaks; an empty override over a base whose Dispose does real work (EmptyOverrideOverRealBase) still leaks; an expression-bodied / metadata-only Dispose we cannot read is not exempted (never a guessed drop). The empty enumerator (the ClosedXML shape) and an empty *Reader stay silent. Two existing fixtures had used an empty `Dispose()` as a modelling shortcut while their comments claimed a real resource; made faithful (a real Dispose body) so they stay genuine leak fixtures: UnitOfWork now owns+disposes a CancellationTokenSource; PixelOwner clears its backing. This keeps their existing by-name CI assertions ('uow'/'leakedOwner' still leak). Verified locally with the real extractor (.NET 8): EmptyDisposeSample goes silent on the two empty-Dispose locals, flags the three controls, in both flat and flow modes; a full existing-sample diff (both modes) is byte-identical after the fixture-fidelity fix (zero regression). Gates: run_tests.py exit 0, test_ownir 276/276, ruff clean, mypy --strict green. Closes #225 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Cg6DVXahkyY68Ruu7f76rG --- .github/workflows/ci.yml | 33 ++++++- docs/notes/field-notes-patterns.md | 18 ++++ frontend/roslyn/OwnSharp.Extractor/Program.cs | 36 ++++++- frontend/roslyn/samples/EmptyDisposeSample.cs | 99 +++++++++++++++++++ .../roslyn/samples/MemoryOwnerEscapeSample.cs | 2 +- .../roslyn/samples/UnitOfWorkFlowSample.cs | 6 +- 6 files changed, 188 insertions(+), 6 deletions(-) create mode 100644 frontend/roslyn/samples/EmptyDisposeSample.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bebe6e5e..5a90beea 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -211,6 +211,7 @@ jobs: frontend/roslyn/samples/SelfDetachingHandlerSample.cs \ frontend/roslyn/samples/UsingFieldAcquisitionSample.cs \ frontend/roslyn/samples/TemplatePartLocalCaptureSample.cs \ + frontend/roslyn/samples/EmptyDisposeSample.cs \ -o "$RUNNER_TEMP/facts.json" cat "$RUNNER_TEMP/facts.json" - name: Check facts through the core @@ -835,7 +836,17 @@ jobs: # ...while the same-named local in the OTHER method must still warn. echo "$out" | grep -qE "TemplatePartLocalCaptureSample\.cs:[0-9]+: warning: \[OWN001\].*'SameNameDifferentScopeSubscriber'" \ || { echo "FAIL: expected OWN001 on the same-named-but-unrelated local in a different method"; exit 1; } - echo "OK: real C# -> facts -> OWN001 (subscription + timer + field + Subscribe + pool + local) + OWN014 (static-event region escape) + DI001 (captive dependency) + DI002 (scoped captured weakly) + DI003 (transient IDisposable captured by a singleton) + DI004 (transient IDisposable service-located from the root provider) + DI005 (scoped service cached from a created scope) + [OwnIgnore] suppression (silent-but-counted, SARIF suppressions) + #218 DP old->new subscription rotation (silent; controls flagged) at the C# location" + # issue #225 — a user-defined type whose Dispose() body is provably EMPTY holds no resource, + # so a LOCAL of it never disposed is not a leak (extends the named-BCL no-op exemption to any + # type). Here the flat (non-flow) name path: a `*Reader`-named empty-Dispose local (ScratchReader) + # is silent, while a non-empty `*Reader` local (LeakyReader) still leaks. (The semantic enumerator + # form is exercised in the --flow-locals step below.) + if echo "$out" | grep -q "'ScratchReader'"; then + echo "FAIL: a local of a user type with a provably-empty Dispose() body was wrongly flagged (#225)"; exit 1 + fi + echo "$out" | grep -qE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'lr'.*'LeakyReader'" \ + || { echo "FAIL: a non-empty-Dispose local (LeakyReader) must still leak (#225 stays scoped)"; exit 1; } + echo "OK: real C# -> facts -> OWN001 (subscription + timer + field + Subscribe + pool + local) + OWN014 (static-event region escape) + DI001 (captive dependency) + DI002 (scoped captured weakly) + DI003 (transient IDisposable captured by a singleton) + DI004 (transient IDisposable service-located from the root provider) + DI005 (scoped service cached from a created scope) + [OwnIgnore] suppression (silent-but-counted, SARIF suppressions) + #218 DP old->new subscription rotation (silent; controls flagged) + #225 empty-Dispose local exemption (silent; controls flagged) at the C# location" - name: Flow-sensitive local IDisposables (--flow-locals, P-016 B0b/B2) run: | # Path-sensitive flow analysis of local IDisposables — bugs the flat D1 @@ -844,7 +855,8 @@ jobs: frontend/roslyn/samples/FlowLocalsSample.cs \ frontend/roslyn/samples/MemoryOwnerEscapeSample.cs \ frontend/roslyn/samples/FactoryLeakSample.cs \ - frontend/roslyn/samples/OverloadSigSample.cs --flow-locals -o "$RUNNER_TEMP/flow.json" + frontend/roslyn/samples/OverloadSigSample.cs \ + frontend/roslyn/samples/EmptyDisposeSample.cs --flow-locals -o "$RUNNER_TEMP/flow.json" out=$(python -m ownlang ownir "$RUNNER_TEMP/flow.json" || true) echo "$out" echo "$out" | grep -q "OWN002" || { echo "FAIL: expected OWN002 (use-after-dispose)"; exit 1; } @@ -1066,7 +1078,22 @@ jobs: for ok in opened probe; do if echo "$out" | grep -q "'$ok'"; then echo "FAIL: stage-2 silent case '$ok' was reported"; exit 1; fi done - echo "OK: flow-sensitive OWN001/002/003 on real C# (path-sensitive, loops via while/foreach/for, try/finally sequential, never-vs-every-path wording, dispose-optional exempt, beyond flat)" + # issue #225 (semantic flow path — the real ClosedXML Slice.Enumerator shape): a LOCAL of a + # user type whose Dispose() body is provably EMPTY holds no resource, so never disposing it + # cannot leak. The three controls STILL leak: a real IDisposable ('r'), a non-empty *Reader + # ('lr'), and an empty override over a base that owns a real Dispose ('d'). + echo "$out" | grep -qE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'r' is never disposed" \ + || { echo "FAIL: #225 flow: a real IDisposable local must still leak"; exit 1; } + echo "$out" | grep -qE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'lr' is never disposed" \ + || { echo "FAIL: #225 flow: a non-empty *Reader local must still leak"; exit 1; } + echo "$out" | grep -qE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'d' is never disposed" \ + || { echo "FAIL: #225 flow: an empty override over a real base Dispose must still leak"; exit 1; } + # exactly 3 EmptyDisposeSample findings -> the two empty-Dispose locals (the enumerator and + # ScratchReader) are SILENT; any leak would push the count past 3. + n225=$(echo "$out" | grep -cE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN00") + [ "$n225" = "3" ] \ + || { echo "FAIL: #225 flow: expected exactly 3 EmptyDisposeSample findings (controls only), got $n225"; exit 1; } + echo "OK: flow-sensitive OWN001/002/003 on real C# (path-sensitive, loops via while/foreach/for, try/finally sequential, never-vs-every-path wording, dispose-optional exempt, #225 empty-Dispose local exempt, beyond flat)" - name: Gallery C#-native bad/ok pairs (examples/gallery/cs/) run: | # C#-native mirror of examples/gallery/*.own, run through the real extractor -> diff --git a/docs/notes/field-notes-patterns.md b/docs/notes/field-notes-patterns.md index 4f36b739..30ede01f 100644 --- a/docs/notes/field-notes-patterns.md +++ b/docs/notes/field-notes-patterns.md @@ -644,6 +644,24 @@ type names, a `Dispose()` method whose body is provably empty (no statements) is safe to skip regardless of whose type it is — first-party or third-party, named allowlist or not.** +**Shipped (issue #225).** `HasEmptyDisposeBody` (extractor) recognises a type +whose parameterless `Dispose()` is declared in source with a **block body of zero +statements** *and* whose base chain carries no `Dispose` to cascade to (base is +`object` / not `IDisposable`), so nothing real is skipped. A **LOCAL** of such a +type never disposed is then dropped from both the flat (`IsDisposableType`) and the +`--flow-locals` (`ImplementsIDisposable`) local-disposable paths. Deliberately +**scoped to locals**: a FIELD keeps its own disposal contract (so the +`OwnIgnoreSample` `Handle` field stand-in and other field fixtures are untouched). +Precision guards, each pinned by `EmptyDisposeSample.cs`: a **non-empty** body +(`RealResource`, `LeakyReader`) still leaks; an **empty override over a base whose +`Dispose` does real work** (`EmptyOverrideOverRealBase`) still leaks; an +**expression-bodied / metadata-only** `Dispose` we cannot read as empty is not +exempted (never a guessed drop). The empty enumerator (`EmptyDisposeEnumerator`, +the ClosedXML shape) and an empty `*Reader` (`ScratchReader`) stay silent. Two +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. + --- ## The through-line diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 7cf7db2b..679b0805 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -1306,6 +1306,32 @@ static bool IsNoOpDisposeWrapper(BaseObjectCreationExpressionSyntax oce, Semanti return arg0 is not null && IsInMemoryDisposableBacking(model.GetTypeInfo(arg0).Type); } +// Issue #225 — a user-defined type whose parameterless `Dispose()` body is provably EMPTY holds no +// resource behind the interface: it implements IDisposable only to satisfy a contract (e.g. +// `IEnumerator`), so never calling Dispose() cannot leak. Recognised from SOURCE: the type +// declares `void Dispose()` with a BLOCK body of ZERO statements, and no base in its chain carries a +// Dispose to cascade to (the base is `object` / not IDisposable), so nothing real is skipped. A body +// we cannot read (third-party metadata-only, or expression-bodied doing work) is NOT provably empty +// and stays flagged — never a guessed drop. This generalises the named-BCL IsNoOpDisposeWrapper / +// IsDisposeOptional idea to any type. Deliberately scoped to LOCAL disposables (the #201-sweep +// evidence — ClosedXML's `Slice.Enumerator` locals): a FIELD keeps its own disposal contract, so an +// empty-Dispose field (the OwnIgnoreSample `Handle` stand-in) is unaffected. +static bool HasEmptyDisposeBody(ITypeSymbol? t) +{ + if (t is not INamedTypeSymbol nt) + return false; + // The base must not own a Dispose we'd be silently skipping — only `object` / a non-IDisposable base. + for (var b = nt.BaseType; b is not null && b.SpecialType != SpecialType.System_Object; b = b.BaseType) + if (ImplementsIDisposable(b)) + return false; + var dispose = nt.GetMembers("Dispose").OfType().FirstOrDefault( + m => m.Parameters.Length == 0 && m.ReturnsVoid && m.TypeParameters.Length == 0); + if (dispose is null || dispose.DeclaringSyntaxReferences.Length == 0) + return false; // no readable source Dispose (metadata-only) -> cannot prove empty + return dispose.DeclaringSyntaxReferences.All( + sref => sref.GetSyntax() is MethodDeclarationSyntax { Body.Statements.Count: 0 }); +} + // A type that is System.Windows.Forms.Form or derives from it (semantic, walks the // base chain). A modeless `Form.Show()` transfers ownership to the framework, which // disposes the form on close — EmitFlowExpr models that show as a release. @@ -4715,6 +4741,11 @@ when model.GetSymbolInfo(asg.Left).Symbol is IFieldSymbol }; if (ctype is null || !IsDisposableType(ctype)) continue; + // #225: a user type whose Dispose() body is provably empty holds no resource — + // never disposing a local of it cannot leak (mirrors the flow path's guard). + if (v.Initializer?.Value is { } newExpr + && HasEmptyDisposeBody(model.GetTypeInfo(newExpr).Type)) + continue; subs.Add(new { @event = name, @@ -4758,7 +4789,10 @@ when model.GetSymbolInfo(asg.Left).Symbol is IFieldSymbol if (v.Initializer is { Value: ObjectCreationExpressionSyntax or ImplicitObjectCreationExpressionSyntax } init && model.GetTypeInfo(init.Value).Type is { } dt - && ImplementsIDisposable(dt) && !IsDisposeOptional(dt)) + && ImplementsIDisposable(dt) && !IsDisposeOptional(dt) + // #225: a user type with a provably-empty Dispose() body holds no + // resource -> never disposing a local of it cannot leak. + && !HasEmptyDisposeBody(dt)) { candidates.Add(v.Identifier.Text); newedDisposables.Add(v.Identifier.Text); diff --git a/frontend/roslyn/samples/EmptyDisposeSample.cs b/frontend/roslyn/samples/EmptyDisposeSample.cs new file mode 100644 index 00000000..57f9835e --- /dev/null +++ b/frontend/roslyn/samples/EmptyDisposeSample.cs @@ -0,0 +1,99 @@ +using System; +using System.Collections; +using System.Collections.Generic; + +namespace Own.Samples; + +// Issue #225 — a user-defined type whose Dispose() body is provably EMPTY holds no resource behind +// the interface (it implements IDisposable only to satisfy a contract, e.g. IEnumerator), so a +// LOCAL of it that is never disposed cannot leak. Generalises the named-BCL no-op exemption +// (field-notes entry 9 / IsNoOpDisposeWrapper) to any type. Mirrors ClosedXML's Slice.Enumerator. +// Negative controls prove it does not over-widen. A FIELD keeps its own disposal contract (out of +// scope here), so the OwnIgnoreSample `Handle` field stand-in is unaffected. + +// EMPTY Dispose — implements IDisposable only because IEnumerator requires it (the ClosedXML +// shape). Its Dispose does literally nothing. +public sealed class EmptyDisposeEnumerator : IEnumerator +{ + private int _i; + public int Current => _i; + object IEnumerator.Current => Current; + public bool MoveNext() => ++_i <= 3; + public void Reset() => _i = 0; + public void Dispose() { } // literally empty -> no resource +} + +// EMPTY Dispose on a name that ALSO matches the flat (non-flow) name heuristic (`*Reader`), so the +// non-flow local-disposable path exercises the same exemption. +public sealed class ScratchReader : IDisposable +{ + public int Read() => -1; + public void Dispose() { } // empty -> no resource +} + +// NON-empty Dispose — a real owned resource released in Dispose. A local never disposed LEAKS. +public sealed class RealResource : IDisposable +{ + private bool _closed; + public void Touch() { } + public void Dispose() { _closed = true; } // has a statement -> not provably empty +} + +// NON-empty Dispose, name matches the flat heuristic — the flat-path control. +public sealed class LeakyReader : IDisposable +{ + public int Read() => 0; + public void Dispose() { GC.SuppressFinalize(this); } // real work -> stays flagged +} + +// Empty-bodied OVERRIDE, but the BASE owns a real Dispose — skipping this type's empty Dispose would +// skip the base's cascade, so a local of it must STAY flagged. +public class BaseWithRealDispose : IDisposable +{ + public void Ping() { } + public virtual void Dispose() { GC.SuppressFinalize(this); } +} +public sealed class EmptyOverrideOverRealBase : BaseWithRealDispose +{ + public override void Dispose() { } // empty, but base.Dispose does real work +} + +public sealed class EmptyDisposeConsumers +{ + // SILENT: an empty-Dispose enumerator local, iterated and never disposed (the ClosedXML shape). + public int CountEmpty() + { + var e = new EmptyDisposeEnumerator(); + var n = 0; + while (e.MoveNext()) n++; // never disposed -> Dispose is empty -> SILENT + return n; + } + + // SILENT: an empty-Dispose `*Reader` local (flat-path name match) never disposed. + public int UseScratch() + { + var s = new ScratchReader(); + return s.Read(); // never disposed -> Dispose is empty -> SILENT + } + + // FLAGGED (control): a real IDisposable local never disposed -> OWN001. + public void LeakReal() + { + var r = new RealResource(); + r.Touch(); // used, never disposed -> LEAK + } + + // FLAGGED (control): a non-empty `*Reader` local never disposed -> OWN001 (flat path). + public int LeakReader() + { + var lr = new LeakyReader(); + return lr.Read(); // never disposed -> LEAK + } + + // FLAGGED (control): empty override, but the base Dispose does real work -> OWN001. + public void LeakDerived() + { + var d = new EmptyOverrideOverRealBase(); + d.Ping(); // base.Dispose() is real -> LEAK + } +} diff --git a/frontend/roslyn/samples/MemoryOwnerEscapeSample.cs b/frontend/roslyn/samples/MemoryOwnerEscapeSample.cs index 38afad02..20f7002a 100644 --- a/frontend/roslyn/samples/MemoryOwnerEscapeSample.cs +++ b/frontend/roslyn/samples/MemoryOwnerEscapeSample.cs @@ -16,7 +16,7 @@ internal sealed class PixelOwner : IMemoryOwner public Memory Memory => this.data; - public void Dispose() { } + public void Dispose() { Array.Clear(this.data, 0, this.data.Length); } // real work (#225): not a no-op } internal static class MemoryOwnerEscape diff --git a/frontend/roslyn/samples/UnitOfWorkFlowSample.cs b/frontend/roslyn/samples/UnitOfWorkFlowSample.cs index 249f5b91..369eeb6b 100644 --- a/frontend/roslyn/samples/UnitOfWorkFlowSample.cs +++ b/frontend/roslyn/samples/UnitOfWorkFlowSample.cs @@ -99,11 +99,15 @@ public interface IUnitOfWork : IDisposable // optional) — which is what makes a leaked local matter. public sealed class UnitOfWork : IUnitOfWork { + // Owns a real IDisposable (a DbContext-like resource) released in Dispose, so a leaked + // local genuinely leaks — its Dispose() is NOT a no-op (keeps this fixture faithful under + // the #225 empty-Dispose exemption, which only skips a provably-empty body). + private readonly System.Threading.CancellationTokenSource _cts = new(); public UnitOfWork(bool isCatalogs) { } public IQueryable ProductCatalogs => Enumerable.Empty().AsQueryable(); public IQueryable TempProducts => Enumerable.Empty().AsQueryable(); public IGenericRepository GenericRepository() => new GenericRepository(); - public void Dispose() { } + public void Dispose() { _cts.Dispose(); } } public interface IGenericRepository From 447dee4a6d7df4019714f6586d7b62662f016898 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 11 Jul 2026 05:26:30 +0000 Subject: [PATCH 2/2] fix(extractor): don't exempt an empty sync Dispose when the type has a real DisposeAsync (#225) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review catch. HasEmptyDisposeBody proved only that the SYNCHRONOUS Dispose() body is empty; a type implementing both IDisposable and IAsyncDisposable can have an empty sync Dispose() as a compatibility no-op while DisposeAsync() does the real cleanup. The flow detector already treats DisposeAsync() as a release, so silencing such a local would drop a real leak. Now conservatively refuse to exempt any type that declares its own parameterless DisposeAsync. Pinned by a new AsyncReader control in EmptyDisposeSample.cs (empty sync Dispose, real DisposeAsync) — flagged in both the flat and --flow-locals CI jobs; the two provably-empty-Dispose locals stay silent. Verified locally: existing-sample diff still byte-identical (the guard only ever flags MORE, never fewer); gates green (test_ownir 276/276, ruff, mypy). Refs #225 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Cg6DVXahkyY68Ruu7f76rG --- .github/workflows/ci.yml | 15 ++++++++---- frontend/roslyn/OwnSharp.Extractor/Program.cs | 6 +++++ frontend/roslyn/samples/EmptyDisposeSample.cs | 24 +++++++++++++++++++ 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5a90beea..f63a4117 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -846,6 +846,9 @@ jobs: fi echo "$out" | grep -qE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'lr'.*'LeakyReader'" \ || { echo "FAIL: a non-empty-Dispose local (LeakyReader) must still leak (#225 stays scoped)"; exit 1; } + # Codex P2: an empty SYNC Dispose but a real DisposeAsync (a `*Reader` here) must still leak. + echo "$out" | grep -qE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'ar'.*'AsyncReader'" \ + || { echo "FAIL: a type with a real DisposeAsync (empty sync Dispose) must still leak (#225)"; exit 1; } echo "OK: real C# -> facts -> OWN001 (subscription + timer + field + Subscribe + pool + local) + OWN014 (static-event region escape) + DI001 (captive dependency) + DI002 (scoped captured weakly) + DI003 (transient IDisposable captured by a singleton) + DI004 (transient IDisposable service-located from the root provider) + DI005 (scoped service cached from a created scope) + [OwnIgnore] suppression (silent-but-counted, SARIF suppressions) + #218 DP old->new subscription rotation (silent; controls flagged) + #225 empty-Dispose local exemption (silent; controls flagged) at the C# location" - name: Flow-sensitive local IDisposables (--flow-locals, P-016 B0b/B2) run: | @@ -1088,11 +1091,15 @@ jobs: || { echo "FAIL: #225 flow: a non-empty *Reader local must still leak"; exit 1; } echo "$out" | grep -qE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'d' is never disposed" \ || { echo "FAIL: #225 flow: an empty override over a real base Dispose must still leak"; exit 1; } - # exactly 3 EmptyDisposeSample findings -> the two empty-Dispose locals (the enumerator and - # ScratchReader) are SILENT; any leak would push the count past 3. + # Codex P2: an empty SYNC Dispose() but a real DisposeAsync() (IDisposable + IAsyncDisposable) + # must NOT be exempted — the real cleanup is async and the flow detector treats it as a release. + echo "$out" | grep -qE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'ar' is never disposed" \ + || { echo "FAIL: #225 flow: a type with a real DisposeAsync (empty sync Dispose) must still leak"; exit 1; } + # exactly 4 EmptyDisposeSample findings -> the two provably-empty-Dispose locals (the enumerator + # and ScratchReader) are SILENT; any leak of them would push the count past 4. n225=$(echo "$out" | grep -cE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN00") - [ "$n225" = "3" ] \ - || { echo "FAIL: #225 flow: expected exactly 3 EmptyDisposeSample findings (controls only), got $n225"; exit 1; } + [ "$n225" = "4" ] \ + || { echo "FAIL: #225 flow: expected exactly 4 EmptyDisposeSample findings (controls only), got $n225"; exit 1; } echo "OK: flow-sensitive OWN001/002/003 on real C# (path-sensitive, loops via while/foreach/for, try/finally sequential, never-vs-every-path wording, dispose-optional exempt, #225 empty-Dispose local exempt, beyond flat)" - name: Gallery C#-native bad/ok pairs (examples/gallery/cs/) run: | diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 679b0805..0361adbb 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -1324,6 +1324,12 @@ static bool HasEmptyDisposeBody(ITypeSymbol? t) for (var b = nt.BaseType; b is not null && b.SpecialType != SpecialType.System_Object; b = b.BaseType) if (ImplementsIDisposable(b)) return false; + // A non-empty DisposeAsync() can do the REAL cleanup even when the sync Dispose() is an empty + // compatibility no-op (a type implementing both IDisposable AND IAsyncDisposable). The flow + // detector already treats DisposeAsync() as a release, so an undisposed local of such a type is a + // real leak — conservatively refuse to exempt any type that declares its own DisposeAsync (Codex). + if (nt.GetMembers("DisposeAsync").OfType().Any(m => m.Parameters.Length == 0)) + return false; var dispose = nt.GetMembers("Dispose").OfType().FirstOrDefault( m => m.Parameters.Length == 0 && m.ReturnsVoid && m.TypeParameters.Length == 0); if (dispose is null || dispose.DeclaringSyntaxReferences.Length == 0) diff --git a/frontend/roslyn/samples/EmptyDisposeSample.cs b/frontend/roslyn/samples/EmptyDisposeSample.cs index 57f9835e..0608b7c7 100644 --- a/frontend/roslyn/samples/EmptyDisposeSample.cs +++ b/frontend/roslyn/samples/EmptyDisposeSample.cs @@ -58,6 +58,22 @@ public sealed class EmptyOverrideOverRealBase : BaseWithRealDispose public override void Dispose() { } // empty, but base.Dispose does real work } +// NON-empty cleanup via DisposeAsync, with the sync Dispose() an empty COMPATIBILITY no-op (a type +// implementing both IDisposable and IAsyncDisposable). The empty sync body must NOT exempt it — the +// real cleanup lives in DisposeAsync, which the flow detector treats as a release, so an undisposed +// local still leaks (Codex P2). Name ends `Reader` so both detector paths see it. +public sealed class AsyncReader : IDisposable, IAsyncDisposable +{ + private readonly System.Threading.CancellationTokenSource _cts = new(); + public int Read() => 0; + public void Dispose() { } // empty sync compat no-op + public System.Threading.Tasks.ValueTask DisposeAsync() // REAL cleanup + { + _cts.Dispose(); + return default; + } +} + public sealed class EmptyDisposeConsumers { // SILENT: an empty-Dispose enumerator local, iterated and never disposed (the ClosedXML shape). @@ -96,4 +112,12 @@ public void LeakDerived() var d = new EmptyOverrideOverRealBase(); d.Ping(); // base.Dispose() is real -> LEAK } + + // FLAGGED (control, Codex P2): empty SYNC Dispose but a real DisposeAsync -> never disposing + // (sync or async) leaks; the empty sync body must not exempt it. + public int LeakAsync() + { + var ar = new AsyncReader(); + return ar.Read(); // never disposed (sync or async) -> LEAK + } }