fix(extractor): treat closure-captured locals as escaped (ShareX throttler FP)

.github/workflows/ci.yml

@@ -380,6 +380,18 @@ jobs:
  || { echo "FAIL: expected OWN001 on the undisposed local in a do-while loop"; exit 1; }
  echo "$out" | grep -qE "OWN001.*'swLeak'" \
  || { echo "FAIL: expected OWN001 on the switch default-branch leak"; exit 1; }
+ # closure-capture escape (precision): a SemaphoreSlim captured by a returned async
+ # lambda outlives the method, so it cannot be disposed at method scope -> escaped ->
+ # silent ('captured'). A SemaphoreSlim NOT captured and never disposed STILL leaks ->
+ # OWN001 ('semLeak'), proving the exemption is closure-capture, not a blanket
+ # SemaphoreSlim dispose-optional (reduced from a ShareX FP — Helpers.ForEachAsync).
+ echo "$out" | grep -qE "OWN001.*'semLeak' is never disposed" \
+ || { echo "FAIL: expected OWN001 on the non-captured SemaphoreSlim leak"; exit 1; }
+ # a `nameof(x)` operand inside a lambda is NOT a closure capture (it is a compile-time
+ # string) -> the local stays method-bounded and still leaks -> OWN001 (Codex review on
+ # #59: nameof must not masquerade as a capture/escape).
+ echo "$out" | grep -qE "OWN001.*'nofLeak' is never disposed" \
+ || { echo "FAIL: expected OWN001 on the nameof-in-lambda local (not a capture)"; exit 1; }
  # dispose-optional (Task), disposed/escaping locals, a `for` loop whose
  # disposable is disposed after it (`looped`, balanced), a balanced
  # acquire+dispose in a loop (`whileClean`), a try/finally dispose (`tfClean`,
@@ -401,7 +413,7 @@ jobs:
  # case disposes (no default) -> last case is the tail, no phantom no-match leak.
  # `ncf`: `ncf?.Dispose()` (null-conditional) in a threaded finally IS a release
  # (member-binding form), so it is disposed on the return path -> silent (Codex review).
- for ok in clean looped esc exemptTask whileClean asyncDisposed asyncDisposedCfg tfClean tfCatch tfRet tfNull cda daci cif ctorLater lamPrior other doClean swAll ncf; do
+ for ok in clean looped esc exemptTask whileClean asyncDisposed asyncDisposedCfg tfClean tfCatch tfRet tfNull cda daci cif ctorLater lamPrior other doClean swAll ncf captured; do
  if echo "$out" | grep -q "'$ok'"; then echo "FAIL: silent/exempt 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)"

docs/notes/closure-capture-escape-precision.md

@@ -0,0 +1,89 @@
+# Closure-capture escape — a local captured by a lambda is not method-bounded
+
+A precision fix driven by triaging the **re-mine of ShareX** (after the WinForms
+modeless-`Form` fix, #57): of the seven local-disposable findings that survived, six
+were real or defensible and **one was a false positive** — `Helpers.ForEachAsync`'s
+`SemaphoreSlim throttler`:
+
+```csharp
+public static Task ForEachAsync<T>(IEnumerable<T> items, Func<T, Task> body, int max)
+{
+ SemaphoreSlim throttler = new SemaphoreSlim(max, max);
+
+ IEnumerable<Task> tasks = items.Select(async input =>
+ {
+ await throttler.WaitAsync(); // throttler used INSIDE the lambda
+ try { await body(input); } finally { throttler.Release(); }
+ });
+
+ return Task.WhenAll(tasks); // the lambdas (and throttler) escape
+}
+```
+
+The flow detector saw `throttler = new SemaphoreSlim(...)` (an undisposed `IDisposable`
+local) and flagged OWN001. But `throttler` is **captured by the async lambdas**, and
+those lambdas escape the method — they run while the returned `Task.WhenAll(tasks)` is
+awaited by the caller. The semaphore must stay alive until every task finishes, so it
+*cannot* be disposed at method scope. It is not a method-local leak.
+
+## The fix — capture into a closure is an escape
+
+The flow detector already untracks a local that escapes by **return**, **out/ref**, or
+being **passed as an argument** (an ambiguous ownership transfer). A capture into a
+closure is the same kind of escape — the closure can be stored, returned, or run async,
+so the local outlives the method frame. The escape filter now also untracks a candidate
+local when any reference to it is **lexically inside a lambda / anonymous method / local
+function** body:
+
+```csharp
+// in the --flow-locals escape filter, before the return/out/arg checks:
+var capturedInClosure = false;
+for (var a = idn.Parent; a is not null && a != mbody; a = a.Parent)
+ if (a is AnonymousFunctionExpressionSyntax or LocalFunctionStatementSyntax)
+ { capturedInClosure = true; break; }
+if (capturedInClosure) { escapedLocals.Add(nm); continue; }
+```
+
+It is purely syntactic (an ancestor walk to the method body), so it is crash-proof and
+needs no escape/data-flow analysis of where the closure goes. It deliberately does **not**
+require proving the closure escapes: a captured local *may* outlive the method, and the
+precision-first stance is to not flag what we cannot prove leaks.
+
+One syntactic exception: a **`nameof(x)`** operand is skipped before the escape checks. It
+exposes an `IdentifierNameSyntax` under the closure (or as an argument), but `nameof` is a
+compile-time string that captures and transfers nothing — so it must not be mistaken for a
+capture (nor, on the argument path, an ownership transfer), or a still-leaking method-bounded
+local would be wrongly untracked (Codex review). Pinned by the `NameofInLambda` sample case.
+
+### Why not just exempt `SemaphoreSlim`?
+
+`SemaphoreSlim` is **not** unconditionally dispose-optional: accessing
+`AvailableWaitHandle` lazily allocates a wait handle that `Dispose()` must release (CA2000
+flags an undisposed one). Blanket-exempting it (the way `Task`/`DataTable` are exempt in
+`IsDisposeOptional`) would be unsound — it would hide a real method-local semaphore leak.
+The bug here is the **capture/escape**, not the type, so the fix targets the capture.
+
+## The recall trade-off (sound, bounded)
+
+The rule is conservative: a local captured by a closure that does **not** escape, and is
+never disposed, is now silenced too (e.g. `var s = new MemoryStream(); Action a = () =>
+s.Use(); a(); /* never disposed */`). Proving such a closure stays method-local is exactly
+the data-flow analysis the syntactic rule avoids, so this is an accepted recall gap, never
+a false positive — the same precision-over-recall trade the escape filter already makes for
+argument-passing.
+
+## Pinned in CI
+
+`frontend/roslyn/samples/FlowLocalsSample.cs` gains two cases, asserted in the
+`wpf-extractor` `--flow-locals` step:
+
+- `ThrottlerCaptured` — a `SemaphoreSlim` captured by a returned async lambda → **silent**
+ (the FP this removes);
+- `SemaphoreLeaks` — a `SemaphoreSlim` **not** captured by any closure and never disposed →
+ **OWN001**, proving the exemption is closure-capture, not a blanket `SemaphoreSlim`
+ dispose-optional.
+
+The existing `UnitOfWorkFlowSample` OWN001 is unaffected: its `uow` is always the
+*receiver* of `uow.Member` (outside the `.Where(p => …)` lambda bodies, which reference
+`p`), and the `join … in uow.TempProducts` is query syntax, not an
+`AnonymousFunctionExpressionSyntax` — so the ancestor walk never marks `uow` captured.

frontend/roslyn/OwnSharp.Extractor/Program.cs

@@ -1081,7 +1081,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);
 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
@@ -1450,6 +1450,34 @@
  var nm = idn.Identifier.Text;
  if (!candidates.Contains(nm))
  continue;
+ // A `nameof(x)` operand is a compile-time string, not a real reference: it
+ // neither uses, captures, nor transfers the local. Skip it so it triggers no
+ // escape rule — otherwise `nameof(s)` would look like an argument (the arg
+ // rule below) or, inside a lambda, a closure capture, and wrongly untrack a
+ // still-leaking method-bounded local (Codex review on PR #59).
+ if (idn.Parent is ArgumentSyntax { Parent: ArgumentListSyntax
+ { Parent: InvocationExpressionSyntax ninv } }
+ && ninv.Expression is IdentifierNameSyntax { Identifier.Text: "nameof" })
+ continue;
+ // Captured into a CLOSURE (lambda / anonymous method / local function):
+ // the closure can outlive the method — stored, returned, or run async —
+ // so the local is no longer method-bounded and cannot be disposed at
+ // method scope. Conservatively treat the capture as an escape (don't
+ // flag it), the same way a returned/out-passed local is untracked.
+ // Reduced from a ShareX false positive: a SemaphoreSlim throttler captured
+ // by the async lambdas of a returned `Task.WhenAll(...)` (Helpers.ForEachAsync).
+ var capturedInClosure = false;
+ for (var a = idn.Parent; a is not null && a != mbody; a = a.Parent)
+ if (a is AnonymousFunctionExpressionSyntax or LocalFunctionStatementSyntax)
+ {
+ capturedInClosure = true;
+ break;
+ }
+ if (capturedInClosure)
+ {
+ escapedLocals.Add(nm);
+ continue;
+ }
  // ... unless it is handed to a CONSUMER (a first-party method that
  // disposes a by-value IDisposable param) as a bare `Consume(s);`
  // statement: that is a handoff RELEASED at the call site, not an escape

frontend/roslyn/samples/FlowLocalsSample.cs

@@ -11,7 +11,7 @@
  // OWN002: used after Dispose()
  public void UseAfterDispose()
  {
  var uad = new MemoryStream();
  uad.WriteByte(1);
  uad.Dispose();
  uad.WriteByte(2);
@@ -20,7 +20,7 @@
  // OWN001: disposed only on the `then` path -> leaks on the else path
  public void LeakOnElse(bool c)
  {
  var leak = new MemoryStream();
  if (c)
  {
  leak.Dispose();
@@ -30,7 +30,7 @@
  // OWN003: disposed twice
  public void DoubleDispose()
  {
  var dbl = new MemoryStream();
  dbl.Dispose();
  dbl.Dispose();
  }
@@ -60,7 +60,7 @@
  {
  while (n > 0)
  {
  var whileLeak = new MemoryStream();
  whileLeak.WriteByte(1);
  n = n - 1;
  }
@@ -71,7 +71,7 @@
  {
  foreach (var it in items)
  {
  var foreachLeak = new MemoryStream();
  foreachLeak.WriteByte((byte)it);
  }
  }
@@ -83,7 +83,7 @@
  {
  for (int i = 0; i < n; i++)
  {
  var forLeak = new MemoryStream();
  forLeak.WriteByte((byte)i);
  }
  }
@@ -403,6 +403,44 @@
  asyncDisposedCfg.WriteByte(1);
  await asyncDisposedCfg.DisposeAsync().ConfigureAwait(false);
  }
+
+ // NOT a leak (precision): a SemaphoreSlim captured into an async lambda whose Task is
+ // returned outlives the method — the caller awaits it, so it cannot be disposed at
+ // method scope. A local referenced inside a closure (lambda / anonymous method / local
+ // function) is treated as escaped, exactly like a returned/out-passed local. Reduced
+ // from a ShareX false positive (Helpers.ForEachAsync's `throttler` captured by the async
+ // lambdas of a returned Task.WhenAll). Silent.
+ public Task ThrottlerCaptured(int max)
+ {
+ var captured = new SemaphoreSlim(max, max);
+ Func<Task> run = async () =>
+ {
+ await captured.WaitAsync();
+ captured.Release();
+ };
+ return run();
+ }
+
+ // control (must still leak): a SemaphoreSlim NOT captured by any closure and never
+ // disposed -> OWN001. Proves the exemption is about CLOSURE CAPTURE (escape), not
+ // SemaphoreSlim being blanket dispose-optional — it is not (accessing AvailableWaitHandle
+ // allocates a handle Dispose must release), so it must stay tracked when method-bounded.
+ public void SemaphoreLeaks()
+ {
+ var semLeak = new SemaphoreSlim(1, 1);
+ semLeak.Wait();
+ }
+
+ // OWN001 (Codex review on #59): a `nameof(x)` operand inside a lambda exposes an
+ // identifier under the closure, but `nameof` is a compile-time string — it captures
+ // nothing. `nofLeak` is only mentioned via nameof, so it is still method-bounded and
+ // never disposed -> a real leak. The nameof operand must not be mistaken for a capture.
+ public void NameofInLambda()
+ {
+ var nofLeak = new MemoryStream();
+ Action log = () => System.Console.WriteLine(nameof(nofLeak));
+ log();
+ }
 }
 
 // A domain exception type literally named `Exception`, in a non-System namespace — the