Soundness: OWN001 treats any `-=` in the class as a release — a `-=` behind a flag, or in a method nobody calls, silently swallows a real leak (heap-proven on SectorTS)

[PhysShell]

Classification

• Change type: soundness fix (false negative), not a precision fix
• Priority: P1 — it silently swallows a leak class, and it does so on the codebase the heuristic was tuned against
• Found by: a runtime heap investigation of SectorTS (ClrMD retention-path walk), not by the oracle sweep
• Doctrine: this is the same failure mode as Soundness regression from #225: empty-Dispose exemption trusts source emptiness, but IL weaving (Janitor.Fody) adds real cleanup — 263 silent exemptions in ClosedXML (+ explicit-interface Dispose coverage gap) #238 — "the worst case of an exemption must be 'keeps today's honest warning', never 'silently swallows a leak class'"
• Related: Soundness regression from #225: empty-Dispose exemption trusts source emptiness, but IL weaving (Janitor.Fody) adds real cleanup — 263 silent exemptions in ClosedXML (+ explicit-interface Dispose coverage gap) #238 / fix(extractor): confine the empty-Dispose exemption to enumerators — source emptiness ≠ runtime no-op (Closes #238) #240 (empty-Dispose exemption), Runtime witness generation: turn supported OWN001/OWN014 evidence into red→green retention tests #270 (runtime witnesses), PhysShell/OwnAudit#13 (the runtime-only bucket, which is exactly what this is)

Problem — a -= that exists is not a -= that runs

The shipped rule (frontend/roslyn/OwnSharp.Extractor/Program.cs:13-14):

A subscription is "released" by a matching target -= handler in the class

Any matching -= anywhere in the class silences the finding. There is no check that the -= is
reachable, that the method holding it is ever called, or that it is not guarded away by a parameter.

The design documents specify the stricter rule, and the implementation is looser than its own spec:

	what it says
docs/proposals/P-004-wpf-lifetime-profile.md:33	source.Event += handler with no matching -= in Dispose/OnClosed/Unloaded
docs/proposals/P-001-csharp-extractor.md:51	with no matching -= in a Dispose/OnClosed/…
OwnSharp.Extractor/Program.cs:13	released by a matching -= in the class

The divergence is in the unsound direction, and it is not theoretical.

Evidence — a real leak, missed, with a heap-proven retention path

In STS_new/SectorTS, GTD's constructor subscribes to a static publisher (GTD.cs:5192-5193):

AppData.Properties.GBProperty.PropertyChanged += GBProperty_PropertyChanged;   // AppData.cs:232: public static readonly KernelProperty Properties

A matching -= exists (GTD.cs:5272) — so OWN001 pairs them and stays silent. But it lives inside a
method that is not a teardown, and behind a parameter that skips it:

public void UnregisterEventHandlers(bool UnregOnlyGoodys = false)
{
    if (!UnregOnlyGoodys)                    // <-- callers pass true; the block never runs
    {
        AppData.Properties.GBProperty.PropertyChanged -= GBProperty_PropertyChanged;
        ...
    }
    // only the goodys are detached here
}

Who actually calls it:

caller	call	released?
MainWindow.xaml.cs (~30 sites), reports	UnregisterEventHandlers()	yes
Service/GTDService.cs:139, 327, 1757, 2252, 2388	UnregisterEventHandlers(**true**)	no
BrokerDataClasses/DocCloud/** — DocCloudService_v1.cs:584, _v2.cs:710, and 8+ AutoMapper .ConstructUsing(x => new GTD(null, null)) profiles	never calls it	no

The AutoMapper profiles construct a GTD per mapping operation, and every one of them pins itself
to the static event for the life of the process.

Runtime proof. Attaching to a headless process that deserializes GTDs, after 31 documents:

roots                :       209 objects
on the heap          : 1 685 951 objects   223 MB
REACHABLE from roots : 1 569 072 objects   148 MB
uncollected garbage  :   116 879 objects    75 MB
>>> 66.3% of the heap is genuinely RETAINED

Retention path from a GC root to the document's goods list:

[PinnedHandle] System.Object[]
  BrokerDataClasses.Property.KernelProperty            <- AppData.Properties (static)
    BrokerDataClasses.Property.GBProperty
      System.ComponentModel.PropertyChangedEventHandler
        System.Object[]                                 <- the delegate's invocation list
          System.ComponentModel.PropertyChangedEventHandler
            BrokerDataClasses.GTD                       <- the whole document graph
              BindingList<BrokerDataClasses.GTDGoody>

And the analyzer half-caught it. OwnAudit/sts_audit/ownsharp-sectorts.txt contains:

KDT.cs:88: [OWN001] event 'AppData.Properties.GBProperty.PropertyChanged' is subscribed
    but never unsubscribed; ... may outlive and keep 'KDT' alive (possible leak)

Same static publisher, correctly flagged — because KDT has no -= at all. GTD.cs:5192 is the same
leak against the same publisher and is not flagged, purely because a -= exists somewhere in the
class.

Note the irony: Program.cs:459-460 says the delegate-unwrapping heuristic was tuned on "the dominant
SectorTS idiom". The pairing assumption was calibrated on the very codebase where it fails.

Fix direction

Primary — honour the spec. A -= releases a subscription only when it is in a teardown context:
Dispose/DisposeAsync, OnClosed/Closed, Unloaded, or a method the type's own disposal path
calls. A -= anywhere else is not a release; at most it is a mitigation candidate and should degrade
to a warning, not to silence. This restores P-004/P-001 as written.

Secondary — a guarded -= is not a -=. If the -= is inside a branch whose condition is a
parameter of the enclosing method (if (!flag) { ... -= ... }), it cannot be proven to run from the
subscription site. Treat it as unreleased, or emit OWN050 ("analysis skipped") rather than silence —
consistent with the existing doctrine of never guessing in favour of "no leak".

Optional, if the call graph is available (D5 / MOS): a -= in method M releases only if M is
reachable from a teardown of the subscribing type. Under the current modular-interprocedural model this
may be out of budget; the two rules above do not need it.

Worst case of each rule is a kept warning, never a swallowed leak — which is the doctrine #238
established.

Acceptance

• GTD.cs:5192 (and PGC.cs:81) are flagged OWN001 on the SectorTS corpus; KDT.cs:88 continues to be.
• The oracle sweep shows no new FPs of the "-= genuinely in Dispose" shape — those must stay silent.
• A minimal repro lands in corpus/wpf/ — subscribe in ctor, -= in a non-teardown method behind a
  bool parameter, caller passes true — as a bad case with a matching ok case that detaches in
  Dispose.
• Re-measure the ClosedXML / 5-repo sweep: the delta should be additive (new true positives), and any
  new finding of this shape should be triageable to a real un-run -=.

Notes

This is precisely the runtime-only bucket that PhysShell/OwnAudit#13 predicted — "retention with no
static finding — the analyzer's blind spot" — and it is the first real instance of it with a proven
retention path. Worth wiring back: a runtime-only finding is not just a report, it is a rule
request.

The retention path above was produced with a small ClrMD tool (attach → mark from GC roots → BFS to the
target type). If it is useful, it is also the collector OwnAudit/docs/runtime-contract.md sketches but
never implemented; happy to contribute it.

[PhysShell]

Prior art — this gap is known, but it was scoped too narrowly and never filed

Credit where it is due: the non-flow-sensitive release model is already written down, in
corpus/wpf/subscription-explicit-delegate-release/notes.md:28-41 (Codex P2 on #163):

after.cs … deliberately avoids the setter-rebind idiom … because own-check's release model is
not flow-sensitive — it treats any matching -= in the class as releasing the subscription. Under
a rebinding setter that model would call the subscription released even though the -= detached only
the old source … That soundness gap is pre-existing … tracking the last-rebound subscription
would need flow-sensitive release analysis, a separate change.

So the model was understood. What was not: the blast radius. It was framed as a rebinding-setter
problem — a narrow, arguably exotic shape — deferred, and never opened as an issue. This is the first
one.

The real failure surface of "any -= in the class = released" is wider, and each of these is ordinary
code, not an exotic idiom:

shape	is the -= proven to run?
rebinding setter (the known one)	no — it detached the old source
-= guarded by a parameter — void Teardown(bool skip) { if (!skip) { … -= … } }, callers pass true	no
-= in a method an entire subsystem never calls	no
-= in a method that is not a teardown at all (not Dispose/OnClosed/Unloaded)	no

The SectorTS leak in the issue body hits three of the four at once — UnregisterEventHandlers(bool)
is not a teardown, its -= is behind the flag, and DocCloud never calls it. It is not a corner case;
it is the default path for cloud import.

The same released flag powers the timer rule, so WPF002 likely has the identical hole

ownlang/ownir.py:34-35 defines the timer resource as:

a started DispatcherTimer/Timer whose Tick/Elapsed handler is never -='d or Stop()ped

and OwnSharp.Extractor/Program.cs:14-16 says the same:

a Tick/Elapsed handler is tagged resource=timer (WPF002) and is released if the timer's
receiver also has a .Stop() call

Same shape as the -= rule: a call somewhere in the class concludes "released". A .Stop() behind a
flag, or in a method nobody calls, should therefore silence WPF002 exactly the way our -= silenced
OWN001.

I have not read the code that computes this — I looked in the extractor and did not find the
Stop()-collection site, so treat this as worth confirming, not as a second confirmed bug. But if it
holds, the fix should be designed as an invariant over all release-concluding exemptions, not as a
patch to the -= rule alone.

The invariant worth adopting

Both this and #238 (empty-Dispose silenced by IL weaving) are the same failure:

an exemption concluded "released" from the existence of a release, without establishing that the
release runs.

#238 already gave the doctrine — "the worst case of an exemption must be 'keeps today's honest
warning', never 'silently swallows a leak class'". Making that a checked invariant over every
release-concluding path (-=, .Stop(), empty-Dispose, and any future one) is worth more than fixing
either instance:

• a release in a teardown context counts;
• a release under a parameter/flag guard does not (→ unreleased, or OWN050 "skipped" — never silence);
• a release in a method with no path from a teardown does not.

That is three rules, none of which needs whole-program points-to. The rebinding-setter case from #163
falls out of the same machinery once release becomes flow-aware.