Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions frontend/roslyn/OwnSharp.Extractor/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,54 @@
static bool IsLambdaHandler(ExpressionSyntax right) =>
right is AnonymousFunctionExpressionSyntax;

// P-004 source-lifetime tier for an ignored `.Subscribe()` chain (WPF004). A
// self-rooted `this.WhenAnyValue(p => p.SelfProp).<self-preserving ops>.Subscribe`
// watches the component's OWN property: the observable, its handler and `this`
// form one cycle the GC collects together, so it is NOT a leak. We classify ONLY
// this unambiguous self-cycle (the bridge drops a `source: "self"` subscribe).
// Anything else stays a flagged leak (conservative). Purely syntactic.

// A single-source operator whose arguments are only funcs / schedulers / scalars,
// so it cannot mix in an EXTERNAL observable. A combinator NOT listed here
// (CombineLatest, Merge, SelectMany, WithLatestFrom, Zip, Switch, Concat, ...), an
// operator with an observable-taking overload (Throttle/Buffer/Sample/TakeUntil/
// Window), or any unknown operator is treated as possibly external -> the chain
// stays flagged. (Conservative: an unrecognised op never silences a real leak.)
static bool IsSelfPreservingOp(string name) =>
name is "Select" or "Where" or "Do" or "Skip" or "Take" or "SkipWhile"
or "TakeWhile" or "ObserveOn" or "SubscribeOn" or "DistinctUntilChanged"
or "WhereNotNull" or "Cast" or "OfType" or "StartWith" or "Scan"
or "Finally" or "AsObservable" or "Synchronize" or "Timestamp";

static bool IsSelfRootedWhenAny(ExpressionSyntax chain)
{
// Walk the fluent chain leftwards. The HEAD must be `this.WhenAnyValue(...)`;
// EVERY downstream operator must be self-preserving (no external observable),
// else a later `.CombineLatest(_bus.X)` / `.SelectMany(_ => _bus.Y)` roots the
// subscription externally and it must stay flagged (codex P1).
var e = chain;
while (e is InvocationExpressionSyntax iv
&& iv.Expression is MemberAccessExpressionSyntax ma)
{
if (ma.Expression is ThisExpressionSyntax)
{
// Head: `this.<op>(...)`. A self-cycle requires WhenAnyValue with a
// single-hop self-member lambda (`p => p.Member`, not `p => p.A.B`,
// not multi-arg).
return ma.Name.Identifier.Text == "WhenAnyValue"
&& iv.ArgumentList.Arguments.Count == 1
&& iv.ArgumentList.Arguments[0].Expression is SimpleLambdaExpressionSyntax lam
&& lam.Body is MemberAccessExpressionSyntax body
&& body.Expression is IdentifierNameSyntax pid
&& pid.Identifier.Text == lam.Parameter.Identifier.Text;
}
if (!IsSelfPreservingOp(ma.Name.Identifier.Text))
return false; // a combinator / unknown op -> possibly external
e = ma.Expression;
}
return false; // not a `this.WhenAnyValue(...)`-headed chain
}

// --- P-016 B0b/B2: flow lowering for local IDisposables (experimental) ---

// A type that implements System.IDisposable (semantic) — the flow lowering tracks
Expand Down Expand Up @@ -697,7 +745,7 @@
.Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries)
.Where(p => p.EndsWith(".dll", StringComparison.OrdinalIgnoreCase))
.ToList();
var refNames = new HashSet<string>(tpa.Select(Path.GetFileName), StringComparer.OrdinalIgnoreCase);

Check warning on line 748 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / C# leak extractor (Roslyn) -> OwnIR -> core

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 748 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / C# leak extractor (Roslyn) -> OwnIR -> core

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 748 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 748 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.
var references = tpa.Select(p => (MetadataReference)MetadataReference.CreateFromFile(p)).ToList();
// P-004 WPF profile: widen the reference set with assemblies named by the
// OWN_EXTRA_REF_DIRS env var (colon-separated dirs) — e.g. the WindowsDesktop ref
Expand Down Expand Up @@ -912,6 +960,10 @@
line = LineOf(inv),
released = false,
resource = "subscribe",
// A self-rooted `this.WhenAnyValue(p => p.SelfProp)` chain is a
// GC-collectible self-cycle: the bridge drops `source: "self"`.
// Any other (external) source stays a flagged leak (null source).
source = IsSelfRootedWhenAny(m.Expression) ? "self" : null,
Comment thread
PhysShell marked this conversation as resolved.
});

// POOL001: an ArrayPool/MemoryPool buffer `Rent`ed but never `Return`ed,
Expand Down
36 changes: 32 additions & 4 deletions ownlang/ownir.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@
(optional `type` names the field's declared type); tag
`[resource: disposable field]`.
- "subscribe": a `X.Subscribe(...)` whose `IDisposable` result is ignored (a
bare statement, not captured/disposed) — always a leak; tag
bare statement, not captured/disposed). Tiered by `source` like a `+=`: a
self-rooted `this.WhenAnyValue(x => x.SelfProp)` chain (`source: "self"`) is a
GC-collectible self-cycle, not a leak (silent); an `injected` source is a
warning (unknown lifetime); a `static`/external/unknown source is a leak. Tag
`[resource: subscription token]`.
- "capture": a *tokenless* strong subscription (`event += handler` with no
token to release) whose event SOURCE provably outlives the subscriber. This
Expand Down Expand Up @@ -437,6 +440,13 @@ def to_own(facts: dict[str, Any]) -> tuple[str, dict[str, dict[str, Any]]]:
# an advisory OWN050 note by _unresolved_findings instead.
if rkind == "unresolved-subscription":
continue
# A `subscribe` (ignored `.Subscribe()` result) whose SOURCE is `self`
# is a self-rooted `this.WhenAnyValue(x => x.SelfProp)` cycle — the
# observable, its handler and `this` form one cycle the GC collects
# together, so it is NOT a leak. Skip it (silent); only an EXTERNAL
# source holds the component from a longer-lived root. Mirrors to_module.
if rkind == "subscribe" and sub.get("source") == "self":
continue
# A `capture` is the tokenless region-escape shape: it does NOT acquire
# a token (no OWN001); it lowers to `subscribe self to <source>` with
# the source's region, so the lifetime engine reports OWN014 when the
Expand Down Expand Up @@ -564,6 +574,12 @@ def to_module(facts: dict[str, Any]) -> tuple[Module, dict[str, dict[str, Any]]]
# surfaced as an advisory OWN050 note, never lowered (see to_own).
if rkind == "unresolved-subscription":
continue
# A self-rooted `subscribe` (ignored `.Subscribe()` on a
# `this.WhenAnyValue(x => x.SelfProp)` chain) is a GC-collectible
# self-cycle, not a leak — skip it (silent), like a released
# subscription. Only an EXTERNAL source holds the component.
if rkind == "subscribe" and sub.get("source") == "self":
continue
# P-004 region escape: a `capture` is a tokenless strong subscription
# whose source provably outlives the subscriber. Lower it to the
# lifetime engine (`subscribe self to <source>` + the source's region)
Expand Down Expand Up @@ -1042,9 +1058,21 @@ def check_facts(facts: dict[str, Any]) -> list[Finding]:
message = (f"local IDisposable '{event}'{of_type} is created but "
f"never disposed (leak)")
elif rkind == "subscribe":
message = (f"the result of '{event}' is ignored — the IDisposable "
f"subscription is never disposed, leaking "
f"'{component}' (leak)")
# source-lifetime tiering (P-004), mirroring the `+=` else branch: a
# self-rooted subscribe is already dropped in to_module (silent); an
# injected source has UNKNOWN lifetime -> warning ("may outlive"); a
# static/external/unknown source stays a provable leak (error).
if sub.get("source") == "injected":
fsev = "warning"
message = (f"the result of '{event}' is ignored — its IDisposable "
f"subscription is never disposed; the source is an "
f"injected dependency whose lifetime is unknown, so it "
f"may outlive and keep '{component}' alive (possible "
f"leak)")
else:
message = (f"the result of '{event}' is ignored — the IDisposable "
f"subscription is never disposed, leaking "
f"'{component}' (leak)")
elif rkind == "pool":
message = (f"pooled buffer '{event}' is rented but never returned "
f"to the pool (leak)")
Expand Down
35 changes: 35 additions & 0 deletions tests/test_ownir.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,41 @@ def _one(source: str, lambda_: bool = False) -> Finding:
if "inline lambda" not in lam.message or "-=" not in lam.message:
fails.append(f"lambda handler should note the missing -= handle: {lam.message!r}")

# --- P-004 source-lifetime tiering for `subscribe` (ignored `.Subscribe()`
# result) — the WalletWasabi precision win. A SELF-rooted subscribe
# (`this.WhenAnyValue(x => x.SelfProp)`) is a GC-collectible self-cycle ->
# silent (not a leak); an `injected` source -> warning (unknown lifetime);
# a `static`/external/unannotated source -> a leak (error). Mirrors `+=`.
def _sub(source: str | None) -> list[Finding]:
s: dict[str, object] = {
"event": "this.WhenAnyValue(x => x.Foo).Subscribe", "handler": "",
"line": 7, "released": False, "resource": "subscribe"}
if source is not None:
s["source"] = source
return check_facts({"module": "M", "components": [
{"name": "Vm", "file": "Vm.cs", "subscriptions": [s]}]})

# self-rooted -> silent (the cycle the GC collects); the 118->real win.
checks += 1
if _sub("self"):
fails.append(f"a self-rooted subscribe must be silent (self-cycle), got "
f"{[(x.code, x.severity) for x in _sub('self')]}")
# injected source -> OWN001 WARNING (unknown lifetime, may outlive).
checks += 1
si = _sub("injected")
if [(x.code, x.severity) for x in si] != [("OWN001", "warning")]:
fails.append(f"injected subscribe should be an OWN001 warning, got "
f"{[(x.code, x.severity) for x in si]}")
elif "may outlive" not in si[0].message:
fails.append(f"injected subscribe message missing wording: {si[0].message!r}")
# external (static) and UNANNOTATED -> OWN001 error (unchanged — no regression).
checks += 1
for src in ("static", None):
se = _sub(src)
if [(x.code, x.severity) for x in se] != [("OWN001", None)]:
fails.append(f"subscribe source={src!r} should stay an OWN001 error, "
f"got {[(x.code, x.severity) for x in se]}")

# the fixture carries the current schema version (the contract is stamped).
checks += 1
if facts.get("ownir_version") != OWNIR_VERSION:
Expand Down
Loading