diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 5bbb4f80..bf24195c 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -235,6 +235,54 @@ static string SubscriptionSourceKind(ExpressionSyntax left, IEventSymbol ev, 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)..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.(...)`. 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 @@ -912,6 +960,10 @@ or ImplicitObjectCreationExpressionSyntax 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, }); // POOL001: an ArrayPool/MemoryPool buffer `Rent`ed but never `Return`ed, diff --git a/ownlang/ownir.py b/ownlang/ownir.py index 954c4fc4..9a455243 100644 --- a/ownlang/ownir.py +++ b/ownlang/ownir.py @@ -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 @@ -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 ` with # the source's region, so the lifetime engine reports OWN014 when the @@ -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 ` + the source's region) @@ -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)") diff --git a/tests/test_ownir.py b/tests/test_ownir.py index 69ac6726..300ad86c 100644 --- a/tests/test_ownir.py +++ b/tests/test_ownir.py @@ -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: