Skip to content

Commit 2ea4d93

Browse files
committed
feat(extractor): self-owned subscription exemption now covers ref/out-built fields and template parts
The WPF mining run (docs/notes/real-world-mining.md) showed that loading the WPF reference pack jumps OWN001 from 8 to 123, dominated by FALSE positives on self-owned controls: a class subscribing to a control it owns is a collectable cycle, not a leak. The existing exemption only recognised fields built by a direct `field = new ...`, so two real ownership shapes slipped through: * indirect construction — `BuildCorner(ref _thumb, ...)`: a helper the class calls populates the field (the adorner pattern); * template parts — `_part = GetTemplateChild("PART_x") as T` / FindName: a control owns the parts of its own template. Fold both into a separate `selfOwned` set used by the subscription exemption only. It is deliberately kept OUT of `constructed`, so the WPF003 disposal detector keeps demanding disposal of `new`'d fields only (you do not dispose a borrowed-by-ref value or a template part). Detection is AST-based — the ref/out argument keyword, and a GetTemplateChild/FindName call name behind an optional cast / `as` — in the spirit of the rest of the extractor. Verification: new sample SelfOwnedControlParts.cs exercises both shapes; the ci.yml wpf-extractor job asserts it produces no finding, alongside the existing self-owned silence checks.
1 parent 7a2e879 commit 2ea4d93

3 files changed

Lines changed: 112 additions & 9 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ jobs:
124124
frontend/roslyn/samples/PooledBufferSample.cs \
125125
frontend/roslyn/samples/LocalDisposableSample.cs \
126126
frontend/roslyn/samples/SelfOwnedViewModel.cs \
127+
frontend/roslyn/samples/SelfOwnedControlParts.cs \
127128
frontend/roslyn/samples/StaticHandlerViewModel.cs \
128129
frontend/roslyn/samples/SampleTypes.cs \
129130
-o "$RUNNER_TEMP/facts.json"
@@ -215,6 +216,13 @@ jobs:
215216
if echo "$out" | grep -q "SelfOwnedViewModel.cs"; then
216217
echo "FAIL: a self-owned subscription was wrongly reported"; exit 1
217218
fi
219+
# P-004 self-owned (extended): a field built indirectly via a `ref`/`out`
220+
# helper, or fetched as one of the control's own template parts
221+
# (GetTemplateChild), is owned just like a `new`'d field — both
222+
# subscriptions in SelfOwnedControlParts are collectable cycles -> silent.
223+
if echo "$out" | grep -q "SelfOwnedControlParts.cs"; then
224+
echo "FAIL: a self-owned (ref-built / template-part) subscription was wrongly reported"; exit 1
225+
fi
218226
# P-004 static-handler exemption: a static-method handler has a null
219227
# delegate target — no instance retained, so not a leak — silent.
220228
if echo "$out" | grep -q "StaticHandlerViewModel.cs"; then

‎frontend/roslyn/OwnSharp.Extractor/Program.cs‎

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -140,21 +140,44 @@ left is MemberAccessExpressionSyntax m
140140

141141
// P-004 self-owned exemption: is the event SOURCE owned by (and so never longer-
142142
// lived than) the subscriber? True for a bare instance event on `this`, or a
143-
// receiver that resolves to a field/local the class constructs (`new`s). Such a
144-
// `source <-> this` reference cycle is GC-collectable, so the subscription is not
145-
// a leak. The receiver is resolved to a SYMBOL (not matched by text), and the
146-
// `constructed` set is AST-based (ObjectCreationExpressionSyntax) — not a regex.
147-
// NOTE: callers must exclude timers — a *running* timer is rooted by the
148-
// dispatcher regardless of who owns the field.
143+
// receiver that resolves to a field/local the class OWNS. "Owns" is the `owned`
144+
// set the caller computes: fields the class constructs directly (`new`), builds
145+
// indirectly through a `ref`/`out` helper, or fetches as one of its own template
146+
// parts. Such a `source <-> this` reference cycle is GC-collectable, so the
147+
// subscription is not a leak. The receiver is resolved to a SYMBOL (not matched by
148+
// text), and `owned` is AST-based — not a regex. NOTE: callers must exclude timers
149+
// — a *running* timer is rooted by the dispatcher regardless of who owns the field.
149150
static bool IsSelfOwnedSource(ExpressionSyntax left, IEventSymbol ev,
150-
SemanticModel model, HashSet<string> constructed)
151+
SemanticModel model, HashSet<string> owned)
151152
{
152153
if (left is not MemberAccessExpressionSyntax m)
153154
return !ev.IsStatic; // bare event => an instance event on `this`
154155
if (m.Expression is ThisExpressionSyntax)
155156
return true;
156157
var recv = model.GetSymbolInfo(m.Expression).Symbol;
157-
return (recv is IFieldSymbol or ILocalSymbol) && constructed.Contains(recv.Name);
158+
return (recv is IFieldSymbol or ILocalSymbol) && owned.Contains(recv.Name);
159+
}
160+
161+
// P-004 (ext): a control fetching one of its OWN template parts —
162+
// `GetTemplateChild("PART_x")` or `[Template.]FindName(...)`, optionally behind a
163+
// cast or `as` — owns the result (it lives inside the control's own template /
164+
// visual tree). AST-only (matched by call name), in the spirit of the rest of the
165+
// file; used to fold template-part fields into the self-owned exemption.
166+
static bool IsTemplatePartFetch(ExpressionSyntax? expr)
167+
{
168+
expr = expr switch
169+
{
170+
CastExpressionSyntax c => c.Expression,
171+
BinaryExpressionSyntax b when b.IsKind(SyntaxKind.AsExpression) => b.Left,
172+
_ => expr,
173+
};
174+
return expr is InvocationExpressionSyntax inv
175+
&& (inv.Expression switch
176+
{
177+
MemberAccessExpressionSyntax ma => ma.Name.Identifier.Text,
178+
IdentifierNameSyntax id => id.Identifier.Text,
179+
_ => null,
180+
}) is "GetTemplateChild" or "FindName";
158181
}
159182

160183
// P-004 static-handler exemption: a `+= StaticMethod` stores a delegate whose
@@ -486,6 +509,27 @@ or ImplicitObjectCreationExpressionSyntax
486509
&& FieldName(a.Left) is { } fn)
487510
constructed.Add(fn);
488511

512+
// P-004 (extended) self-owned set for the SUBSCRIPTION exemption ONLY. A
513+
// field can be owned without a direct `_f = new ...`; fold in two more
514+
// shapes the WPF mining run surfaced. Kept OUT of `constructed` so the
515+
// WPF003 disposal detector keeps demanding disposal of `new`'d fields only
516+
// (you don't dispose a borrowed-by-ref value or a template part):
517+
// * ref/out construction — `BuildCorner(ref _thumb, ...)`: a helper this
518+
// class calls populates the field, so the class owns it;
519+
// * template parts — `_part = GetTemplateChild("PART_x") as T`: a control
520+
// owns the parts of its own template (collectable part<->control cycle).
521+
var selfOwned = new HashSet<string>(constructed);
522+
foreach (var arg in cls.DescendantNodes().OfType<ArgumentSyntax>())
523+
if ((arg.RefKindKeyword.IsKind(SyntaxKind.RefKeyword)
524+
|| arg.RefKindKeyword.IsKind(SyntaxKind.OutKeyword))
525+
&& FieldName(arg.Expression) is { } rf)
526+
selfOwned.Add(rf);
527+
foreach (var a in assigns)
528+
if (a.IsKind(SyntaxKind.SimpleAssignmentExpression)
529+
&& FieldName(a.Left) is { } tf
530+
&& IsTemplatePartFetch(a.Right))
531+
selfOwned.Add(tf);
532+
489533
var subs = new List<object>();
490534
foreach (var a in assigns)
491535
{
@@ -507,7 +551,7 @@ or ImplicitObjectCreationExpressionSyntax
507551
// constructs) — the source<->this cycle is GC-collectable;
508552
// - static handler — a static method has a null delegate target,
509553
// so no instance is retained and nothing can leak.
510-
if (!isTimer && (IsSelfOwnedSource(a.Left, ev, model, constructed)
554+
if (!isTimer && (IsSelfOwnedSource(a.Left, ev, model, selfOwned)
511555
|| IsStaticHandler(a.Right, model)))
512556
continue;
513557
// P-004 tiering: a local-variable source is method-bounded — it
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
using System;
2+
3+
// P-004 self-owned exemption (EXTENDED) — both subscriptions below must stay
4+
// SILENT. A class can OWN its event source without a direct `_f = new ...`; these
5+
// are the two shapes the WPF mining run on ScreenToGif surfaced, each a
6+
// GC-collectable source<->this cycle rather than a leak:
7+
// * `_thumb` is constructed INDIRECTLY — handed by `ref` to a helper that `new`s
8+
// it (the adorner `BuildCorner(ref _thumb, ...)` shape);
9+
// * `_upButton` is one of this control's OWN template parts, fetched with
10+
// `GetTemplateChild(...)` (the templated-control shape).
11+
// Contrast CustomerViewModel, whose source is an injected bus (a warning). Timers
12+
// remain the exception (a running timer is dispatcher-rooted) — see TimerViewModel.
13+
//
14+
// Stand-in types (Tier A: no WPF reference set) mirror the real surface: a Thumb
15+
// with DragDelta, a RepeatButton with Click, and a control with the protected
16+
// GetTemplateChild(string) lookup. Namespaced so they cannot collide with the
17+
// other samples compiled alongside this file.
18+
namespace OwnSamples.SelfOwnedParts
19+
{
20+
public sealed class SelfOwnedControlParts : TemplatedControlStub
21+
{
22+
private Thumb _thumb = null!; // built indirectly, through a `ref` helper
23+
private RepeatButton? _upButton; // fetched as one of our own template parts
24+
25+
public SelfOwnedControlParts()
26+
{
27+
BuildCorner(ref _thumb);
28+
_thumb.DragDelta += OnDrag; // self-owned (ref-constructed) -> not a leak
29+
}
30+
31+
public override void OnApplyTemplate()
32+
{
33+
_upButton = GetTemplateChild("PART_Up") as RepeatButton;
34+
_upButton!.Click += OnUp; // self-owned (template part) -> not a leak
35+
}
36+
37+
private static void BuildCorner(ref Thumb t) => t = new Thumb();
38+
39+
private void OnDrag(object? sender, EventArgs e) { }
40+
private void OnUp(object? sender, EventArgs e) { }
41+
}
42+
43+
public sealed class Thumb { public event EventHandler? DragDelta; }
44+
public sealed class RepeatButton { public event EventHandler? Click; }
45+
46+
public abstract class TemplatedControlStub
47+
{
48+
protected object? GetTemplateChild(string name) => null;
49+
public virtual void OnApplyTemplate() { }
50+
}
51+
}

0 commit comments

Comments
 (0)