Skip to content

Commit 685fbf6

Browse files
committed
fix(extractor): scope the AppDomain-event exemption to NON-CAPTURING handlers (Codex P1)
Codex: the first cut keyed only off the EVENT (the AppDomain source), ignoring the handler — so it also dropped a real region escape, e.g. `AppDomain.CurrentDomain.ProcessExit += (_,_) => _field++` or an instance-method handler, whose delegate target is the subscriber and is pinned to the process until shutdown. The Npgsql case is safe only because its lambda is non-capturing (a static `ClearAll()` call). HandlerRetainsNoInstance now gates the exemption: a static method group (null target) or a lambda that captures neither `this` (explicit, or implicit via an instance member) nor an enclosing local/parameter. A capturing handler stays OWN014. Sample: ShutdownCleanup gains the 4th event (FirstChanceException, CodeRabbit) and a new CapturingShutdownSubscriber (`(_,_) => _count++`) that must STILL warn. CI: assert no OWN014 anywhere in the sample file (format-insensitive, CodeRabbit) for the exempt cases, and OWN014 for both the non-AppDomain and the capturing controls. (Rebased onto current main — #86/#87 landed; resolved the ci.yml sample-list conflict.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
1 parent b30db58 commit 685fbf6

3 files changed

Lines changed: 77 additions & 13 deletions

File tree

.github/workflows/ci.yml

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -319,14 +319,19 @@ jobs:
319319
echo "$out" | grep -q "region escape" \
320320
|| { echo "FAIL: expected the region-escape wording on the static-event capture"; exit 1; }
321321
# P-004 process-lifetime AppDomain-event exemption (mined: Npgsql PoolManager): a
322-
# subscription to a process-host AppDomain event (ProcessExit/DomainUnload/Unhandled-
323-
# Exception) is a shutdown/diagnostics hook meant to live for the process -> NOT a
324-
# region escape -> silent. A non-AppDomain static event with a lambda still escapes.
325-
if echo "$out" | grep -q "ShutdownCleanup"; then
326-
echo "FAIL: an AppDomain process-lifetime event subscription was wrongly reported as a region escape"; exit 1
322+
# NON-CAPTURING handler on a process-host AppDomain event (ProcessExit/DomainUnload/
323+
# UnhandledException/FirstChanceException) is a shutdown/diagnostics hook meant to live
324+
# for the process -> NOT a region escape -> silent. (Assert no OWN014 anywhere in the
325+
# sample file, not just the class name, so a format change can't slip through.)
326+
if echo "$out" | grep -qE "AppDomainShutdownSample\.cs:[0-9]+:.*\[OWN014\]"; then
327+
echo "FAIL: a non-capturing AppDomain process-lifetime event subscription was wrongly reported as OWN014"; exit 1
327328
fi
328-
echo "$out" | grep -qE "NonAppDomainSubscriber.*OWN014|OWN014.*NonAppDomainSubscriber" \
329+
# scope guards: a lambda on a NON-AppDomain static event still escapes; and an AppDomain
330+
# handler that CAPTURES instance state is still pinned to the process -> still OWN014 (Codex).
331+
echo "$out" | grep -qE "OWN014.*NonAppDomainSubscriber" \
329332
|| { echo "FAIL: a lambda on a non-AppDomain static event must still raise OWN014 (exemption stays scoped)"; exit 1; }
333+
echo "$out" | grep -qE "OWN014.*CapturingShutdownSubscriber" \
334+
|| { echo "FAIL: an instance-capturing AppDomain handler must still raise OWN014 (exemption is non-capturing only)"; exit 1; }
330335
# the unsubscribed variant (a matching `-=`, released capture) is mitigated
331336
# -> silent. Must NOT be reported.
332337
if echo "$out" | grep -q "CleanStaticEventViewModel"; then

frontend/roslyn/OwnSharp.Extractor/Program.cs

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,48 @@ ev.Name is "ProcessExit" or "DomainUnload" or "UnhandledException" or "FirstChan
216216
&& ev.ContainingType is { Name: "AppDomain" } ct
217217
&& IsInNamespace(ct, "System");
218218

219+
// Does this handler retain NO subscriber instance? A static method group has a null delegate
220+
// target; a lambda / anonymous method retains nothing only when it captures neither `this`
221+
// (explicit, or implicit via an instance member) nor an enclosing local/parameter. Keeps the
222+
// AppDomain process-lifetime exemption sound — a CAPTURING handler is still pinned to the
223+
// process until shutdown and stays OWN014 (Codex); only a non-capturing one (Npgsql's
224+
// `(_,_) => ClearAll()`, a static call) is safe to drop.
225+
static bool HandlerRetainsNoInstance(ExpressionSyntax right, SemanticModel model)
226+
{
227+
if (IsStaticHandler(right, model))
228+
return true;
229+
if (right is not AnonymousFunctionExpressionSyntax lambda)
230+
return false; // a delegate-typed value is opaque -> conservatively assume it captures
231+
foreach (var node in lambda.DescendantNodes())
232+
{
233+
if (node is ThisExpressionSyntax or BaseExpressionSyntax)
234+
return false;
235+
if (node is not IdentifierNameSyntax id
236+
|| (id.Parent is MemberAccessExpressionSyntax m && m.Name == id)) // `x.Member`: name resolved via x
237+
continue;
238+
var sym = model.GetSymbolInfo(id).Symbol;
239+
if (sym is IFieldSymbol { IsStatic: false } or IPropertySymbol { IsStatic: false }
240+
or IEventSymbol { IsStatic: false }
241+
or IMethodSymbol { IsStatic: false, MethodKind: MethodKind.Ordinary })
242+
return false; // an instance member by SIMPLE name -> implicit `this` capture
243+
if (sym is ILocalSymbol or IParameterSymbol && !DeclaredWithin(sym, lambda))
244+
return false; // an enclosing local / parameter -> captured
245+
}
246+
return true;
247+
}
248+
249+
// Is EVERY declaration of `sym` inside `scope`? A lambda's own parameters/locals are; an
250+
// enclosing local/parameter is not — so a reference to the latter is a capture.
251+
static bool DeclaredWithin(ISymbol sym, SyntaxNode scope)
252+
{
253+
if (sym.DeclaringSyntaxReferences.Length == 0)
254+
return false;
255+
foreach (var r in sym.DeclaringSyntaxReferences)
256+
if (!scope.FullSpan.Contains(r.Span))
257+
return false;
258+
return true;
259+
}
260+
219261
// P-004 process-lived-subscriber exemption: the WPF application object (`App`) is a
220262
// process-lived singleton — exactly one instance, created at startup, alive until
221263
// the process exits. Subscribing it to a process-lived static event
@@ -2092,12 +2134,15 @@ or ImplicitObjectCreationExpressionSyntax
20922134
// - static handler — a static method has a null delegate target,
20932135
// so no instance is retained and nothing can leak.
20942136
// - a process-host AppDomain event (ProcessExit/DomainUnload/Unhandled-
2095-
// Exception/FirstChanceException) — the handler is meant to live for the
2096-
// whole process, so the "escape" is the intent, not a leak (mined: Npgsql
2097-
// PoolManager's `AppDomain.CurrentDomain.ProcessExit += …` shutdown hook).
2137+
// Exception/FirstChanceException) whose handler retains NO instance — the
2138+
// handler is meant to live for the whole process, so the "escape" is the
2139+
// intent, not a leak (mined: Npgsql PoolManager's `AppDomain.CurrentDomain.
2140+
// ProcessExit += (_,_) => ClearAll()` shutdown hook). A handler that captures
2141+
// instance state still pins it to the process, so it stays OWN014 (Codex).
20982142
if (!isTimer && (IsSelfOwnedSource(a.Left, ev, model, selfOwned)
20992143
|| IsStaticHandler(a.Right, model)
2100-
|| IsProcessLifetimeAppDomainEvent(ev)))
2144+
|| (IsProcessLifetimeAppDomainEvent(ev)
2145+
&& HandlerRetainsNoInstance(a.Right, model))))
21012146
continue;
21022147
// P-004 tiering: a local-variable source is method-bounded — it
21032148
// cannot outlive `this`, so it is not a heap leak; drop it (the same

frontend/roslyn/samples/AppDomainShutdownSample.cs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,28 @@ public sealed class ShutdownCleanup
1313
{
1414
public ShutdownCleanup()
1515
{
16-
AppDomain.CurrentDomain.ProcessExit += (_, _) => Cleanup(); // shutdown hook -> SILENT
17-
AppDomain.CurrentDomain.DomainUnload += (_, _) => Cleanup(); // shutdown hook -> SILENT
18-
AppDomain.CurrentDomain.UnhandledException += (_, _) => Cleanup(); // process diagnostics -> SILENT
16+
AppDomain.CurrentDomain.ProcessExit += (_, _) => Cleanup(); // shutdown hook -> SILENT
17+
AppDomain.CurrentDomain.DomainUnload += (_, _) => Cleanup(); // shutdown hook -> SILENT
18+
AppDomain.CurrentDomain.UnhandledException += (_, _) => Cleanup(); // process diagnostics -> SILENT
19+
AppDomain.CurrentDomain.FirstChanceException += (_, _) => Cleanup(); // process diagnostics -> SILENT
1920
}
2021

2122
private static void Cleanup() { }
2223
}
2324

25+
// Codex control: a process-host AppDomain event is exempt only when the handler retains NO
26+
// instance. A lambda that CAPTURES instance state (here `_count`) pins this subscriber to the
27+
// process until shutdown — a real region escape that must STILL raise OWN014.
28+
public sealed class CapturingShutdownSubscriber
29+
{
30+
private int _count;
31+
32+
public CapturingShutdownSubscriber()
33+
{
34+
AppDomain.CurrentDomain.ProcessExit += (_, _) => _count++; // captures `this` -> OWN014
35+
}
36+
}
37+
2438
public static class SomeBus
2539
{
2640
public static event EventHandler? Pinged;

0 commit comments

Comments
 (0)