From 8e80d668ce271c45edc590f21656175a2df52bda Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 22 Jun 2026 11:18:43 +0000 Subject: [PATCH 1/2] fix(extractor): suppress OWN014 region-escape from a static class (mined ImageSharp FP #4/4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `static class` has no instance, so a static-source subscription from it cannot promote an instance to the source's lifetime — the OWN014 region escape is vacuous. Mined: SixLabors/ImageSharp's MemoryAllocatorValidator, a `static class` whose static ctor hooks the static `MemoryDiagnostics.MemoryAllocated`/`MemoryReleased` events, was wrongly reported as "promotes MemoryAllocatorValidator to process lifetime" — there is no instance to promote. The static-source escape skip now also fires when the enclosing type is a static class (next to the existing process-lived `App` case). Scoped to the OWN014 escape only — OWN001 token leaks are untouched — and to non-timers, like the App case. (The two MemoryDiagnosticsTests OWN014s — lambdas in `static void RunTest` local functions of an INSTANCE class — are left honest: no `this` is captured so the named-instance claim is wrong, but the closure is retained, so this is the murky static-context territory, not a clean static-class drop.) Regression sample StaticClassEscapeSample.cs: a static class subscribing a LAMBDA (not covered by the static-method-handler exemption) to a static event stays silent; the existing StaticEventEscapeViewModel proves an INSTANCE class on the same shape still raises OWN014. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- .github/workflows/ci.yml | 8 +++++ frontend/roslyn/OwnSharp.Extractor/Program.cs | 18 +++++++---- .../roslyn/samples/StaticClassEscapeSample.cs | 30 +++++++++++++++++++ 3 files changed, 50 insertions(+), 6 deletions(-) create mode 100644 frontend/roslyn/samples/StaticClassEscapeSample.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 56a91db2..19f595c4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -141,6 +141,7 @@ jobs: frontend/roslyn/samples/InjectedDcViewSample.xaml.cs \ frontend/roslyn/samples/ResolvedDisposableSample.cs \ frontend/roslyn/samples/FieldReleaseSample.cs \ + frontend/roslyn/samples/StaticClassEscapeSample.cs \ -o "$RUNNER_TEMP/facts.json" cat "$RUNNER_TEMP/facts.json" - name: Check facts through the core @@ -321,6 +322,13 @@ jobs: if echo "$out" | grep -q "CleanStaticEventViewModel"; then echo "FAIL: an unsubscribed (released) static-event subscription was wrongly reported"; exit 1 fi + # P-004 static-class escape exemption (mined: ImageSharp MemoryAllocatorValidator): a + # `static class` has no instance, so a static-source subscription (even a lambda) from it + # cannot be a region escape -> OWN014 must NOT fire. (StaticEventEscapeViewModel above + # proves an INSTANCE class on the same shape still escapes, so this stays scoped.) + if echo "$out" | grep -q "StaticAllocationCounter"; then + echo "FAIL: a static-source subscription from a static class was wrongly reported as a region escape"; exit 1 + fi # P-004 process-lived-subscriber exemption (mined: ScreenToGif App + # Translator): the WPF `App` singleton hooking the process-lived # AppDomain.UnhandledException promotes nothing, so the static-source region diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index a94e544e..11530c07 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -2043,6 +2043,11 @@ or ImplicitObjectCreationExpressionSyntax // Is this class the process-lived WPF application object? Used to drop the // static-source region escape (OWN014) — `App` cannot be over-promoted. var clsIsApp = IsProcessLivedApplication(cls); + // A `static class` has NO instance, so a static-source subscription from it cannot + // promote an instance to the source's lifetime — the OWN014 escape is vacuous. (Mined: + // ImageSharp MemoryAllocatorValidator, a static class whose static ctor hooks the static + // MemoryDiagnostics events.) Drops only the static-source escape, not OWN001 token leaks. + var clsIsStaticClass = cls.Modifiers.Any(m => m.IsKind(SyntaxKind.StaticKeyword)); var subs = new List(); foreach (var a in assigns) @@ -2076,12 +2081,13 @@ or ImplicitObjectCreationExpressionSyntax : SubscriptionSourceKind(a.Left, ev, model); if (source == "local") continue; - // Process-lived subscriber (the WPF `App` singleton): a static-source - // subscription promotes nothing — `App` already lives for the whole - // process — so the region escape (OWN014) is a false positive. Scoped - // to NON-timers: a timer is forced to source "static" above, but a - // never-stopped timer in `App` is still a real leak (CodeRabbit). - if (!isTimer && source == "static" && clsIsApp) + // A static-source subscription whose SUBSCRIBER cannot be over-promoted is not a + // region escape (OWN014): the process-lived WPF `App` singleton (its lifetime + // already equals the process), or a `static class` (no instance exists at all — + // mined: ImageSharp MemoryAllocatorValidator). Scoped to NON-timers: a timer is + // forced to source "static" above, but a never-stopped timer is still a real + // leak (CodeRabbit). + if (!isTimer && source == "static" && (clsIsApp || clsIsStaticClass)) continue; var released = unsub.Contains($"{a.Left}|{a.Right}") || (isTimer && Receiver(a.Left) is { } recv && stopped.Contains(recv)); diff --git a/frontend/roslyn/samples/StaticClassEscapeSample.cs b/frontend/roslyn/samples/StaticClassEscapeSample.cs new file mode 100644 index 00000000..732b3449 --- /dev/null +++ b/frontend/roslyn/samples/StaticClassEscapeSample.cs @@ -0,0 +1,30 @@ +using System; + +namespace Own.Samples; + +// P-004 static-class region-escape exemption (mined: ImageSharp MemoryAllocatorValidator). +// +// A `static class` has NO instance, so subscribing to a process-lived STATIC event from its +// static ctor cannot promote an instance to the source's lifetime — OWN014 must NOT fire. A +// LAMBDA handler is used on purpose: it is NOT covered by the static-method-handler exemption, +// so silence here exercises the static-class drop itself, not that exemption. Contrast: +// StaticEventEscapeViewModel (an INSTANCE class on the same shape) must STILL raise OWN014. + +public static class StaticDiagnosticsBus +{ + public static event EventHandler? Allocated; + + public static void Raise() => Allocated?.Invoke(null, EventArgs.Empty); +} + +public static class StaticAllocationCounter +{ + private static int count; + + static StaticAllocationCounter() + { + StaticDiagnosticsBus.Allocated += (_, _) => count++; // lambda + static event + static class -> SILENT + } + + public static int Count => count; +} From 0cc0d5b1110cc944d1917b32792b820426a331f4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 22 Jun 2026 11:26:06 +0000 Subject: [PATCH 2/2] fix(extractor): make IsStaticHandler resolve member-group symbols; drop static-class escape suppression (Codex) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex P2: blanket-suppressing OWN014 for ALL static-source subscriptions in a static class is unsound — a CAPTURING lambda still retains its compiler-generated display-class instance for the process, even with no `this`. Codex's pointer is right: the mined case (MemoryAllocatorValidator) uses static-METHOD handlers, which the existing IsStaticHandler is meant to exempt; it missed them because a method group's symbol can surface as a member group (Symbol == null, CandidateSymbols populated). So: make IsStaticHandler fall back to CandidateSymbols (requiring ALL candidates static, so a mixed overload set is not wrongly exempted), and revert the static-class suppression entirely. A static-method handler in a static class is now exempted by IsStaticHandler (null target, no instance), while a capturing lambda there still escapes — sound. Sample now uses a static-METHOD handler on a static event from a static class's static ctor (replicating MemoryAllocatorValidator) and must stay silent; StaticEventEscapeViewModel still proves an instance handler on a static event escapes. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- .github/workflows/ci.yml | 11 +++--- frontend/roslyn/OwnSharp.Extractor/Program.cs | 39 +++++++++++-------- .../roslyn/samples/StaticClassEscapeSample.cs | 19 +++++---- 3 files changed, 38 insertions(+), 31 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 19f595c4..5474ee1b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -322,12 +322,13 @@ jobs: if echo "$out" | grep -q "CleanStaticEventViewModel"; then echo "FAIL: an unsubscribed (released) static-event subscription was wrongly reported"; exit 1 fi - # P-004 static-class escape exemption (mined: ImageSharp MemoryAllocatorValidator): a - # `static class` has no instance, so a static-source subscription (even a lambda) from it - # cannot be a region escape -> OWN014 must NOT fire. (StaticEventEscapeViewModel above - # proves an INSTANCE class on the same shape still escapes, so this stays scoped.) + # P-004 robust static-handler exemption (mined: ImageSharp MemoryAllocatorValidator): a + # static-METHOD handler on a static event stores a null-target delegate -> no instance is + # retained -> OWN014 must NOT fire, even when the method-group symbol surfaces as a member + # group (now resolved via CandidateSymbols). (StaticEventEscapeViewModel above proves an + # INSTANCE handler on the same static event still escapes, so this stays scoped.) if echo "$out" | grep -q "StaticAllocationCounter"; then - echo "FAIL: a static-source subscription from a static class was wrongly reported as a region escape"; exit 1 + echo "FAIL: a static-method handler on a static event was wrongly reported as a region escape"; exit 1 fi # P-004 process-lived-subscriber exemption (mined: ScreenToGif App + # Translator): the WPF `App` singleton hooking the process-lived diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 11530c07..51fb0c5a 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -185,10 +185,23 @@ BinaryExpressionSyntax b when b.IsKind(SyntaxKind.AsExpression) => b.Left, // Target is null, so no instance is retained — the subscription cannot leak a // subscriber, however long-lived the source. Only method-group handlers // (identifier / member access) are judged; lambdas and delegate-typed values may -// capture state and are left as leak candidates. -static bool IsStaticHandler(ExpressionSyntax right, SemanticModel model) => - IsHandler(right) - && model.GetSymbolInfo(right).Symbol is IMethodSymbol { IsStatic: true }; +// capture state and are left as leak candidates. A method group's symbol can surface +// as a MEMBER GROUP (Symbol == null, CandidateSymbols populated) instead of the bound +// method, so fall back to the candidates — else a genuinely static-method handler is +// missed and a static-source subscription that retains NO instance is mis-reported as +// a region escape (mined: ImageSharp MemoryAllocatorValidator's static MemoryDiagnostics +// handlers). When falling back, require ALL candidates static so an overload set that +// mixes a static and an instance method is not wrongly exempted. +static bool IsStaticHandler(ExpressionSyntax right, SemanticModel model) +{ + if (!IsHandler(right)) + return false; + var info = model.GetSymbolInfo(right); + if (info.Symbol is { } s) + return s is IMethodSymbol { IsStatic: true }; + var cands = info.CandidateSymbols; + return cands.Length > 0 && cands.All(c => c is IMethodSymbol { IsStatic: 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 @@ -2043,11 +2056,6 @@ or ImplicitObjectCreationExpressionSyntax // Is this class the process-lived WPF application object? Used to drop the // static-source region escape (OWN014) — `App` cannot be over-promoted. var clsIsApp = IsProcessLivedApplication(cls); - // A `static class` has NO instance, so a static-source subscription from it cannot - // promote an instance to the source's lifetime — the OWN014 escape is vacuous. (Mined: - // ImageSharp MemoryAllocatorValidator, a static class whose static ctor hooks the static - // MemoryDiagnostics events.) Drops only the static-source escape, not OWN001 token leaks. - var clsIsStaticClass = cls.Modifiers.Any(m => m.IsKind(SyntaxKind.StaticKeyword)); var subs = new List(); foreach (var a in assigns) @@ -2081,13 +2089,12 @@ or ImplicitObjectCreationExpressionSyntax : SubscriptionSourceKind(a.Left, ev, model); if (source == "local") continue; - // A static-source subscription whose SUBSCRIBER cannot be over-promoted is not a - // region escape (OWN014): the process-lived WPF `App` singleton (its lifetime - // already equals the process), or a `static class` (no instance exists at all — - // mined: ImageSharp MemoryAllocatorValidator). Scoped to NON-timers: a timer is - // forced to source "static" above, but a never-stopped timer is still a real - // leak (CodeRabbit). - if (!isTimer && source == "static" && (clsIsApp || clsIsStaticClass)) + // Process-lived subscriber (the WPF `App` singleton): a static-source + // subscription promotes nothing — `App` already lives for the whole + // process — so the region escape (OWN014) is a false positive. Scoped + // to NON-timers: a timer is forced to source "static" above, but a + // never-stopped timer in `App` is still a real leak (CodeRabbit). + if (!isTimer && source == "static" && clsIsApp) continue; var released = unsub.Contains($"{a.Left}|{a.Right}") || (isTimer && Receiver(a.Left) is { } recv && stopped.Contains(recv)); diff --git a/frontend/roslyn/samples/StaticClassEscapeSample.cs b/frontend/roslyn/samples/StaticClassEscapeSample.cs index 732b3449..e31cab4f 100644 --- a/frontend/roslyn/samples/StaticClassEscapeSample.cs +++ b/frontend/roslyn/samples/StaticClassEscapeSample.cs @@ -2,13 +2,14 @@ namespace Own.Samples; -// P-004 static-class region-escape exemption (mined: ImageSharp MemoryAllocatorValidator). +// P-004 static-handler exemption, robustly (mined: ImageSharp MemoryAllocatorValidator). // -// A `static class` has NO instance, so subscribing to a process-lived STATIC event from its -// static ctor cannot promote an instance to the source's lifetime — OWN014 must NOT fire. A -// LAMBDA handler is used on purpose: it is NOT covered by the static-method-handler exemption, -// so silence here exercises the static-class drop itself, not that exemption. Contrast: -// StaticEventEscapeViewModel (an INSTANCE class on the same shape) must STILL raise OWN014. +// A static class whose static ctor hooks a process-lived STATIC event with a STATIC METHOD +// handler stores a delegate whose Target is null — no instance is retained, so OWN014 must NOT +// fire. The mined case slipped through because the method-group symbol can surface as a member +// group (Symbol == null), which IsStaticHandler now resolves via CandidateSymbols. Contrast: +// StaticEventEscapeViewModel — an INSTANCE handler on a static event — must still raise OWN014 +// (capturing lambdas in a static class likewise still escape: the closure is retained). public static class StaticDiagnosticsBus { @@ -19,12 +20,10 @@ public static class StaticDiagnosticsBus public static class StaticAllocationCounter { - private static int count; - static StaticAllocationCounter() { - StaticDiagnosticsBus.Allocated += (_, _) => count++; // lambda + static event + static class -> SILENT + StaticDiagnosticsBus.Allocated += OnAllocated; // static-method handler -> null target -> SILENT } - public static int Count => count; + private static void OnAllocated(object? sender, EventArgs e) { } }