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

[PhysShell]

The false positive (the one FP from the ShareX triage)

Triaging the ShareX re-mine left six real/defensible local-disposable findings and one false positive — Helpers.ForEachAsync's SemaphoreSlim throttler:

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 flagged throttler as an undisposed IDisposable (OWN001). But it's captured by the async lambdas, which escape via the returned Task.WhenAll — the semaphore must stay alive until every task finishes, so it cannot be disposed at method scope. Not a method-local leak.

The fix — capture into a closure is an escape

The escape filter already untracks a local that escapes by return / out / 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 filter now also untracks a candidate whose reference is lexically inside a lambda / anonymous method / local function:

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; }

Purely syntactic (an ancestor walk to the method body) — crash-proof, no data-flow analysis. It doesn't require proving the closure escapes: a captured local may outlive the method, and the precision-first stance is to not flag what we can't prove leaks (a sound, bounded recall gap, like the argument-passing case).

Not a blanket SemaphoreSlim exemption. SemaphoreSlim is not dispose-optional — accessing AvailableWaitHandle allocates a handle Dispose() must release — so a method-bounded one must still be flagged. The bug is the capture/escape, not the type.

Pinned in CI

Two FlowLocalsSample.cs cases in the wpf-extractor --flow-locals step:

method	shape	verdict
ThrottlerCaptured	SemaphoreSlim captured by a returned async lambda	silent (the FP this removes)
SemaphoreLeaks	SemaphoreSlim not captured, never disposed	OWN001 (control — proves it's closure-capture, not a blanket exemption)

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 (verified).

Local validation: the semLeak control → OWN001 "is never disposed" through the core; the captured throttler emits no facts (escaped → skipped). The extractor's source→facts step is validated in CI. Writeup: docs/notes/closure-capture-escape-precision.md.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED

---

Generated by Claude Code

Summary by CodeRabbit

• Improvements

  • Enhanced flow-local analysis precision for closure-captured locals in async lambdas, including more accurate handling of SemaphoreSlim.
  • Corrected how nameof(...) expressions inside lambdas affect escape/capture detection.

• Documentation

  • Updated the closure-capture escape-precision write-up with a concrete C# example and explanation of the updated behavior.

• Tests

  • Added new sample cases covering captured semaphore scenarios and nameof inside lambdas to validate the improved detection.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0cebf332-85fd-48c5-9ce8-81a0a08af81b

📥 Commits

Reviewing files that changed from the base of the PR and between ac9f99c and 6e1b44a.

📒 Files selected for processing (4)

• .github/workflows/ci.yml
• docs/notes/closure-capture-escape-precision.md
• frontend/roslyn/OwnSharp.Extractor/Program.cs
• frontend/roslyn/samples/FlowLocalsSample.cs

🚧 Files skipped from review as they are similar to previous changes (3)

• frontend/roslyn/OwnSharp.Extractor/Program.cs
• .github/workflows/ci.yml
• docs/notes/closure-capture-escape-precision.md

---

📝 Walkthrough

Walkthrough

The --flow-locals escape filter in Program.cs is extended to detect when a tracked local is captured inside a lambda, anonymous method, or local function by walking ancestor syntax nodes; such captured locals are marked as escaped to suppress false OWN001 reports. The change includes special handling to skip nameof operands. Three new SemaphoreSlim sample methods are added to FlowLocalsSample.cs to demonstrate closure-capture (exempt), non-capture leak, and nameof-in-closure cases. CI assertions are updated to verify expected findings, and a new design document explains the precision trade-off rationale.

Changes

Closure-Capture Escape Precision for --flow-locals

Layer / File(s)	Summary
Closure-capture and nameof handling in escape detection frontend/roslyn/OwnSharp.Extractor/Program.cs	In the --flow-locals escape detection, occurrences of candidate tracked locals are now skipped when used as nameof operands; a new ancestor walk checks if any identifier occurrence is lexically inside an AnonymousFunctionExpressionSyntax or LocalFunctionStatementSyntax, and if so, adds the local to escapedLocals to suppress OWN001.
SemaphoreSlim sample test cases frontend/roslyn/samples/FlowLocalsSample.cs	Adds ThrottlerCaptured(int max) (closure-capture exempt: SemaphoreSlim captured by async lambda whose Task is returned), SemaphoreLeaks() (non-captured leak: SemaphoreSlim without disposal), and NameofInLambda() (nameof inside lambda body without actual closure capture).
CI assertions for closure-capture precision .github/workflows/ci.yml	Extends the flow-locals step to assert OWN001 for semLeak and nofLeak, and adds captured to the silent/exempt identifier loop so the closure-capture variant produces no finding.
Design rationale and precision trade-offs documentation docs/notes/closure-capture-escape-precision.md	Documents the original false-positive scenario, the implemented syntactic ancestor walk fix, rejection of a global SemaphoreSlim exemption due to lazy wait-handle allocation, the conservative recall gap, and CI-pinned test case descriptions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• PhysShell/Own.NET#15: Introduces the initial --flow-locals implementation and FlowLocalsSample with basic CI assertions; this PR refines the escape/capture logic in Program.cs and extends the sample methods and CI assertions to handle the new closure-capture precision cases.

Poem

🐇 A semaphore slim sat captured in a lambda's embrace,
Its owner could not see it escape to another place.
I walked up the syntax tree, ancestor by ancestor's name,
And silenced the false warning — precision was the aim!
Now leaks get their OWN001, and captured ones go free,
The rabbit hops along, proud of flow sensitivity. 🌿

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main fix: treating closure-captured locals as escaped to resolve a false positive with a ShareX throttler (SemaphoreSlim).
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/closure-capture-escape

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ac9f99c21f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".