From b30db58a2f952253c8f2d9a54750a771eaa4ede5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 22 Jun 2026 12:51:10 +0000 Subject: [PATCH 1/3] fix(extractor): exempt process-host AppDomain event subscriptions from OWN014 (mined Npgsql) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mining npgsql/npgsql surfaced this: PoolManager's static ctor subscribes `AppDomain.CurrentDomain.{DomainUnload,ProcessExit} += (_,_) => ClearAll()` — a deliberate "close idle connectors on appdomain unload (web-app redeployment)" hook (#491) — and the detector reported it as an OWN014 region escape. But subscribing to a process-host AppDomain event is never a leak: ProcessExit/DomainUnload run at shutdown, UnhandledException / FirstChanceException on every unhandled throw, and the AppDomain IS the process host — the handler is MEANT to live for the whole process. Promoting the subscriber to "the AppDomain's lifetime" is the intent, not a leak. IsProcessLifetimeAppDomainEvent exempts these four System.AppDomain events from the OWN014 region escape (alongside the self-owned-source and static-handler exemptions). Scoped to the event's resolved declaring type, so a project's own `AppDomain`-named type is not matched, and a non-AppDomain process-lived static event still escapes. Regression sample AppDomainShutdownSample.cs: ShutdownCleanup (ProcessExit/DomainUnload/ UnhandledException lambdas) stays silent; NonAppDomainSubscriber (a lambda on a non-AppDomain static event) still raises OWN014. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- .github/workflows/ci.yml | 10 +++++ frontend/roslyn/OwnSharp.Extractor/Program.cs | 20 +++++++++- .../roslyn/samples/AppDomainShutdownSample.cs | 40 +++++++++++++++++++ 3 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 frontend/roslyn/samples/AppDomainShutdownSample.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d17609aa..128149d3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -142,6 +142,7 @@ jobs: frontend/roslyn/samples/ResolvedDisposableSample.cs \ frontend/roslyn/samples/FieldReleaseSample.cs \ frontend/roslyn/samples/StaticClassEscapeSample.cs \ + frontend/roslyn/samples/AppDomainShutdownSample.cs \ -o "$RUNNER_TEMP/facts.json" cat "$RUNNER_TEMP/facts.json" - name: Check facts through the core @@ -317,6 +318,15 @@ jobs: || { echo "FAIL: expected OWN014 region escape on the static-event instance subscription"; exit 1; } echo "$out" | grep -q "region escape" \ || { echo "FAIL: expected the region-escape wording on the static-event capture"; exit 1; } + # P-004 process-lifetime AppDomain-event exemption (mined: Npgsql PoolManager): a + # subscription to a process-host AppDomain event (ProcessExit/DomainUnload/Unhandled- + # Exception) is a shutdown/diagnostics hook meant to live for the process -> NOT a + # region escape -> silent. A non-AppDomain static event with a lambda still escapes. + if echo "$out" | grep -q "ShutdownCleanup"; then + echo "FAIL: an AppDomain process-lifetime event subscription was wrongly reported as a region escape"; exit 1 + fi + echo "$out" | grep -qE "NonAppDomainSubscriber.*OWN014|OWN014.*NonAppDomainSubscriber" \ + || { echo "FAIL: a lambda on a non-AppDomain static event must still raise OWN014 (exemption stays scoped)"; exit 1; } # the unsubscribed variant (a matching `-=`, released capture) is mitigated # -> silent. Must NOT be reported. if echo "$out" | grep -q "CleanStaticEventViewModel"; then diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index a716e00c..7f58430d 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -203,6 +203,19 @@ static bool IsStaticHandler(ExpressionSyntax right, SemanticModel model) return cands.Length > 0 && cands.All(c => c is IMethodSymbol { IsStatic: true }); } +// P-004 process-lifetime exemption: a subscription to a PROCESS-HOST `System.AppDomain` +// event — ProcessExit / DomainUnload (shutdown cleanup hooks) or UnhandledException / +// FirstChanceException (process-wide diagnostics) — is never a region escape. The handler +// is MEANT to live for the whole process: it runs at shutdown, or on every unhandled throw, +// and the AppDomain IS the process host. Promoting the subscriber to "the AppDomain's +// lifetime" is therefore the intent, not a leak. Mined: Npgsql's PoolManager static ctor +// `AppDomain.CurrentDomain.{DomainUnload,ProcessExit} += (_,_) => ClearAll()` — a deliberate +// "close idle connectors on appdomain unload (web-app redeployment)" hook (#491). +static bool IsProcessLifetimeAppDomainEvent(IEventSymbol ev) => + ev.Name is "ProcessExit" or "DomainUnload" or "UnhandledException" or "FirstChanceException" + && ev.ContainingType is { Name: "AppDomain" } ct + && IsInNamespace(ct, "System"); + // P-004 process-lived-subscriber exemption: the WPF application object (`App`) is a // process-lived singleton — exactly one instance, created at startup, alive until // the process exits. Subscribing it to a process-lived static event @@ -2078,8 +2091,13 @@ or ImplicitObjectCreationExpressionSyntax // constructs) — the source<->this cycle is GC-collectable; // - static handler — a static method has a null delegate target, // so no instance is retained and nothing can leak. + // - a process-host AppDomain event (ProcessExit/DomainUnload/Unhandled- + // Exception/FirstChanceException) — the handler is meant to live for the + // whole process, so the "escape" is the intent, not a leak (mined: Npgsql + // PoolManager's `AppDomain.CurrentDomain.ProcessExit += …` shutdown hook). if (!isTimer && (IsSelfOwnedSource(a.Left, ev, model, selfOwned) - || IsStaticHandler(a.Right, model))) + || IsStaticHandler(a.Right, model) + || IsProcessLifetimeAppDomainEvent(ev))) continue; // P-004 tiering: a local-variable source is method-bounded — it // cannot outlive `this`, so it is not a heap leak; drop it (the same diff --git a/frontend/roslyn/samples/AppDomainShutdownSample.cs b/frontend/roslyn/samples/AppDomainShutdownSample.cs new file mode 100644 index 00000000..0a76aa4d --- /dev/null +++ b/frontend/roslyn/samples/AppDomainShutdownSample.cs @@ -0,0 +1,40 @@ +using System; + +namespace Own.Samples; + +// P-004 process-lifetime AppDomain-event exemption (mined: Npgsql PoolManager). Subscribing to +// AppDomain's process-host events — ProcessExit / DomainUnload (shutdown cleanup hooks) and +// UnhandledException / FirstChanceException (process-wide diagnostics) — is never a region +// escape: the handler is meant to live for the whole process. Must be SILENT. Contrast: +// NonAppDomainSubscriber below — a non-AppDomain static event with a lambda still raises OWN014, +// proving the exemption keys off the AppDomain source, not any process-lived static event. + +public sealed class ShutdownCleanup +{ + public ShutdownCleanup() + { + AppDomain.CurrentDomain.ProcessExit += (_, _) => Cleanup(); // shutdown hook -> SILENT + AppDomain.CurrentDomain.DomainUnload += (_, _) => Cleanup(); // shutdown hook -> SILENT + AppDomain.CurrentDomain.UnhandledException += (_, _) => Cleanup(); // process diagnostics -> SILENT + } + + private static void Cleanup() { } +} + +public static class SomeBus +{ + public static event EventHandler? Pinged; + + public static void Raise() => Pinged?.Invoke(null, EventArgs.Empty); +} + +// Control: a lambda on a NON-AppDomain process-lived (static) event -> region escape -> must WARN. +public sealed class NonAppDomainSubscriber +{ + public NonAppDomainSubscriber() + { + SomeBus.Pinged += (_, _) => Handle(); // static event, lambda, not AppDomain -> OWN014 + } + + private static void Handle() { } +} From 685fbf6e2e57973b76aeec2a280f456d2d4010ff Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 22 Jun 2026 12:59:35 +0000 Subject: [PATCH 2/3] fix(extractor): scope the AppDomain-event exemption to NON-CAPTURING handlers (Codex P1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- .github/workflows/ci.yml | 17 +++--- frontend/roslyn/OwnSharp.Extractor/Program.cs | 53 +++++++++++++++++-- .../roslyn/samples/AppDomainShutdownSample.cs | 20 +++++-- 3 files changed, 77 insertions(+), 13 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 128149d3..dd53d4d2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -319,14 +319,19 @@ jobs: echo "$out" | grep -q "region escape" \ || { echo "FAIL: expected the region-escape wording on the static-event capture"; exit 1; } # P-004 process-lifetime AppDomain-event exemption (mined: Npgsql PoolManager): a - # subscription to a process-host AppDomain event (ProcessExit/DomainUnload/Unhandled- - # Exception) is a shutdown/diagnostics hook meant to live for the process -> NOT a - # region escape -> silent. A non-AppDomain static event with a lambda still escapes. - if echo "$out" | grep -q "ShutdownCleanup"; then - echo "FAIL: an AppDomain process-lifetime event subscription was wrongly reported as a region escape"; exit 1 + # NON-CAPTURING handler on a process-host AppDomain event (ProcessExit/DomainUnload/ + # UnhandledException/FirstChanceException) is a shutdown/diagnostics hook meant to live + # for the process -> NOT a region escape -> silent. (Assert no OWN014 anywhere in the + # sample file, not just the class name, so a format change can't slip through.) + if echo "$out" | grep -qE "AppDomainShutdownSample\.cs:[0-9]+:.*\[OWN014\]"; then + echo "FAIL: a non-capturing AppDomain process-lifetime event subscription was wrongly reported as OWN014"; exit 1 fi - echo "$out" | grep -qE "NonAppDomainSubscriber.*OWN014|OWN014.*NonAppDomainSubscriber" \ + # scope guards: a lambda on a NON-AppDomain static event still escapes; and an AppDomain + # handler that CAPTURES instance state is still pinned to the process -> still OWN014 (Codex). + echo "$out" | grep -qE "OWN014.*NonAppDomainSubscriber" \ || { echo "FAIL: a lambda on a non-AppDomain static event must still raise OWN014 (exemption stays scoped)"; exit 1; } + echo "$out" | grep -qE "OWN014.*CapturingShutdownSubscriber" \ + || { echo "FAIL: an instance-capturing AppDomain handler must still raise OWN014 (exemption is non-capturing only)"; exit 1; } # the unsubscribed variant (a matching `-=`, released capture) is mitigated # -> silent. Must NOT be reported. if echo "$out" | grep -q "CleanStaticEventViewModel"; then diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 7f58430d..163a6944 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -216,6 +216,48 @@ ev.Name is "ProcessExit" or "DomainUnload" or "UnhandledException" or "FirstChan && ev.ContainingType is { Name: "AppDomain" } ct && IsInNamespace(ct, "System"); +// Does this handler retain NO subscriber instance? A static method group has a null delegate +// target; a lambda / anonymous method retains nothing only when it captures neither `this` +// (explicit, or implicit via an instance member) nor an enclosing local/parameter. Keeps the +// AppDomain process-lifetime exemption sound — a CAPTURING handler is still pinned to the +// process until shutdown and stays OWN014 (Codex); only a non-capturing one (Npgsql's +// `(_,_) => ClearAll()`, a static call) is safe to drop. +static bool HandlerRetainsNoInstance(ExpressionSyntax right, SemanticModel model) +{ + if (IsStaticHandler(right, model)) + return true; + if (right is not AnonymousFunctionExpressionSyntax lambda) + return false; // a delegate-typed value is opaque -> conservatively assume it captures + foreach (var node in lambda.DescendantNodes()) + { + if (node is ThisExpressionSyntax or BaseExpressionSyntax) + return false; + if (node is not IdentifierNameSyntax id + || (id.Parent is MemberAccessExpressionSyntax m && m.Name == id)) // `x.Member`: name resolved via x + continue; + var sym = model.GetSymbolInfo(id).Symbol; + if (sym is IFieldSymbol { IsStatic: false } or IPropertySymbol { IsStatic: false } + or IEventSymbol { IsStatic: false } + or IMethodSymbol { IsStatic: false, MethodKind: MethodKind.Ordinary }) + return false; // an instance member by SIMPLE name -> implicit `this` capture + if (sym is ILocalSymbol or IParameterSymbol && !DeclaredWithin(sym, lambda)) + return false; // an enclosing local / parameter -> captured + } + return true; +} + +// Is EVERY declaration of `sym` inside `scope`? A lambda's own parameters/locals are; an +// enclosing local/parameter is not — so a reference to the latter is a capture. +static bool DeclaredWithin(ISymbol sym, SyntaxNode scope) +{ + if (sym.DeclaringSyntaxReferences.Length == 0) + return false; + foreach (var r in sym.DeclaringSyntaxReferences) + if (!scope.FullSpan.Contains(r.Span)) + return false; + return true; +} + // P-004 process-lived-subscriber exemption: the WPF application object (`App`) is a // process-lived singleton — exactly one instance, created at startup, alive until // the process exits. Subscribing it to a process-lived static event @@ -2092,12 +2134,15 @@ or ImplicitObjectCreationExpressionSyntax // - static handler — a static method has a null delegate target, // so no instance is retained and nothing can leak. // - a process-host AppDomain event (ProcessExit/DomainUnload/Unhandled- - // Exception/FirstChanceException) — the handler is meant to live for the - // whole process, so the "escape" is the intent, not a leak (mined: Npgsql - // PoolManager's `AppDomain.CurrentDomain.ProcessExit += …` shutdown hook). + // Exception/FirstChanceException) whose handler retains NO instance — the + // handler is meant to live for the whole process, so the "escape" is the + // intent, not a leak (mined: Npgsql PoolManager's `AppDomain.CurrentDomain. + // ProcessExit += (_,_) => ClearAll()` shutdown hook). A handler that captures + // instance state still pins it to the process, so it stays OWN014 (Codex). if (!isTimer && (IsSelfOwnedSource(a.Left, ev, model, selfOwned) || IsStaticHandler(a.Right, model) - || IsProcessLifetimeAppDomainEvent(ev))) + || (IsProcessLifetimeAppDomainEvent(ev) + && HandlerRetainsNoInstance(a.Right, model)))) continue; // P-004 tiering: a local-variable source is method-bounded — it // cannot outlive `this`, so it is not a heap leak; drop it (the same diff --git a/frontend/roslyn/samples/AppDomainShutdownSample.cs b/frontend/roslyn/samples/AppDomainShutdownSample.cs index 0a76aa4d..a83529b3 100644 --- a/frontend/roslyn/samples/AppDomainShutdownSample.cs +++ b/frontend/roslyn/samples/AppDomainShutdownSample.cs @@ -13,14 +13,28 @@ public sealed class ShutdownCleanup { public ShutdownCleanup() { - AppDomain.CurrentDomain.ProcessExit += (_, _) => Cleanup(); // shutdown hook -> SILENT - AppDomain.CurrentDomain.DomainUnload += (_, _) => Cleanup(); // shutdown hook -> SILENT - AppDomain.CurrentDomain.UnhandledException += (_, _) => Cleanup(); // process diagnostics -> SILENT + AppDomain.CurrentDomain.ProcessExit += (_, _) => Cleanup(); // shutdown hook -> SILENT + AppDomain.CurrentDomain.DomainUnload += (_, _) => Cleanup(); // shutdown hook -> SILENT + AppDomain.CurrentDomain.UnhandledException += (_, _) => Cleanup(); // process diagnostics -> SILENT + AppDomain.CurrentDomain.FirstChanceException += (_, _) => Cleanup(); // process diagnostics -> SILENT } private static void Cleanup() { } } +// Codex control: a process-host AppDomain event is exempt only when the handler retains NO +// instance. A lambda that CAPTURES instance state (here `_count`) pins this subscriber to the +// process until shutdown — a real region escape that must STILL raise OWN014. +public sealed class CapturingShutdownSubscriber +{ + private int _count; + + public CapturingShutdownSubscriber() + { + AppDomain.CurrentDomain.ProcessExit += (_, _) => _count++; // captures `this` -> OWN014 + } +} + public static class SomeBus { public static event EventHandler? Pinged; From 63b0bc5e78b5e4a48d637a003c0be37bd2e4ec26 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 22 Jun 2026 13:01:36 +0000 Subject: [PATCH 3/3] ci: scope the AppDomain silent-assertion to ShutdownCleanup (the capturing control shares the file) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous assertion failed CI: it asserted NO OWN014 anywhere in AppDomainShutdownSample.cs, but CapturingShutdownSubscriber — added in the same file to prove the Codex non-capturing scope — correctly DOES raise OWN014. Scope the silent check to ShutdownCleanup (the exempt class); the capturing and non-AppDomain controls keep their own must-warn OWN014 assertions. Still OWN014-specific (CodeRabbit), just per-class. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- .github/workflows/ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dd53d4d2..9cf2c8c3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -321,10 +321,10 @@ jobs: # P-004 process-lifetime AppDomain-event exemption (mined: Npgsql PoolManager): a # NON-CAPTURING handler on a process-host AppDomain event (ProcessExit/DomainUnload/ # UnhandledException/FirstChanceException) is a shutdown/diagnostics hook meant to live - # for the process -> NOT a region escape -> silent. (Assert no OWN014 anywhere in the - # sample file, not just the class name, so a format change can't slip through.) - if echo "$out" | grep -qE "AppDomainShutdownSample\.cs:[0-9]+:.*\[OWN014\]"; then - echo "FAIL: a non-capturing AppDomain process-lifetime event subscription was wrongly reported as OWN014"; exit 1 + # for the process -> NOT a region escape -> silent. (Scoped to ShutdownCleanup, the + # exempt class — CapturingShutdownSubscriber in the same file MUST still raise OWN014.) + if echo "$out" | grep -qE "OWN014.*'ShutdownCleanup'"; then + echo "FAIL: ShutdownCleanup's non-capturing AppDomain subscriptions were wrongly reported as OWN014"; exit 1 fi # scope guards: a lambda on a NON-AppDomain static event still escapes; and an AppDomain # handler that CAPTURES instance state is still pinned to the process -> still OWN014 (Codex).