fix(dispose): every rooted Rx connection in src/ is owned and released with its owner — the #3737 shape as the one spelling, ratcheted by rbuergi · Pull Request #3746 · Systemorph/MeshWeaver

rbuergi · 2026-09-08T17:57:08Z

The dispose chain, made structurally correct for every rooted Rx connection in `src/`

Follows #3737 (fix(MeshNodeStreamCache): release synced-query connections on Dispose), which fixed ONE
site of a defect class. This makes the fix the only spelling and ratchets the class to zero.

The defect class. A multicast chain (Replay(1), Publish()) whose Connect() handle nobody owns:
.AutoConnect(1) keeps it inside the operator, a dropped .Connect() keeps it nowhere. The owner's
Dispose() cannot reach the upstream — and the connect is frequently NOT synchronous:
Defer(…).SubscribeOn(TaskPoolScheduler) only queues the upstream subscribe, and no teardown phase joins a
pool-queued Rx subscribe (DisposalCompleted covers action blocks, IoPoolRegistry.DrainAll covers
IIoPool leaves, AsyncDisposeQueue covers enqueued cleanup). It ran whenever the pool reached it, against
a closed scope: 11 ObjectDisposedException stragglers in Plugins run 34222933802, all one Defer.

The shape (one helper, MeshWeaver.Messaging.OwnedConnectionExtensions).
.Replay(1).AutoConnectOwnedBy(hub | CompositeDisposable, nameof(Owner)) / .Publish().ConnectOwnedBy(owner):
the handle is registered with the owner on connect; the owner's disposal unsubscribes a live upstream and
cancels a still-queued connect (a registration landing in a disposed CompositeDisposable is disposed on
the spot); a chain that terminates drops its handle (the owner tracks LIVE connections only); a subscriber
arriving after the release is refused with ObjectDisposedException(ownerName) instead of being parked on a
replay nothing will feed.

Per-site table (sweep: `git grep -n -E "AutoConnect$|\.Connect\($|RefCount\(" -- src`, code hits only)

Site	Owner	Verdict before	What changed
`Hosting/MeshNodeStreamCache.cs` `GetQueryRaw`	cache (`_queryConnections`)	fixed inline by #3737	migrated to `AutoConnectOwnedBy(_queryConnections, …)`; the hand-written `ReleaseQueryConnection` fault-arm release is now the helper's terminal release. #3737's `QueryConnectionsReleasedOnTeardownTest` still passes (2/2)
`ContentCollections/ContentService.cs` `InitializeCollection`	hub-scoped (`AddScoped<IContentService>`)	FAILS — bare `AutoConnect(1)` on a promise whose first subscriber can be a pool-landed render; after teardown `GetCollection` REPLAYED the disposed collection	`AutoConnectOwnedBy(hub, nameof(ContentService))`; new `ContentCollectionConnectionOwnedByHubTest`
`Compiler.Pipeline/NodeTypeCompilationHelpers.cs` `RunCompile`	NodeType hub	FAILS — the compile pipeline subscribes on the pool (`SubscribeOn`), so the first subscriber's connect could run the activity `CreateNode` off a closed scope, `Catch` swallowed it as `null`, and Roslyn ran for a dead hub	`AutoConnectOwnedBy(hub, $"RunCompile({hubPath})")`; a late subscribe now faults into the compile subscription's existing error arm
`GitSync/GitHubRepoIdentityResolver.cs` `Lookup`	DI singleton, no hub	FAILS — promise in a `PromiseCache`, bare `AutoConnect(1)`; a lookup in flight at host stop completed against the disposed container	class is now `IDisposable` with a `CompositeDisposable lookups`; `AutoConnectOwnedBy(lookups, …)`
`Hosting.AspNetCore/Portal/UserIdentityCache.cs` ctor	DI singleton + mesh hub	partial — `Publish()+Connect()` handle WAS stored and disposed in `Dispose()`, but only at container disposal, i.e. AFTER the mesh's scopes closed	`hub.RegisterForDisposal(_connections)` + `ConnectOwnedBy(_connections)`: released in the hub's ShutDown, and by `Dispose()` (idempotent)
`Mesh.Contract/ActivityTracker.cs` ctor	self (DI singleton)	OK — handle stored, disposed in `Dispose()`, upstream is its own Subject+EventLoopScheduler	`ConnectOwnedBy(connections)` so the guard sees ownership; behaviour unchanged
`Hosting.Orleans/RoutingQuiescence.cs` ctor	self (DI singleton)	OK — same as ActivityTracker	same
`Data/VirtualDataSource.cs` `StreamUpdates`	—	`RefCount()`: subscriber-owned, subscription `stream.RegisterForDisposal`'d	inventoried with reason
`Graph/Configuration/PartitionRegistry.cs` `GetPartition`	—	`RefCount()`: subscriber-owned; mesh service resolved at CALL time	inventoried
`Graph/DeckSlidesCache.cs` `GetOrderedSlides`	—	`RefCount()`: subscriber-owned; deliberately no disconnect delay	inventoried
`Graph/MeshNodeTypeSource.cs` ctor	—	`RefCount()`: own subscription `hub.RegisterForDisposal`'d	inventoried
`Hosting/Persistence/PartitionStorage/RoutingProxyAdapter.cs` ctor	—	`RefCount()`: completing one-shot probe, `Defer`'d	inventoried
`Hosting/Persistence/PersistenceService.cs` ctor	—	`RefCount()`: completing one-shot probe	inventoried

Mesh.Contract/Threading/PromiseCache.cs only mentions AutoConnect in prose (its Do, not a Subscribe, is
exactly so it never connects a chain). tools/MeshWeaver.PluginTester/Cascade.cs (PublishLast().AutoConnect())
is a run harness whose Finally completes the queue — outside src/, not touched.

The guard — `RootedRxConnectionRatchetGuard` (Documentation.Test) + `test/RootedRxConnectionSites.allow`

Scans src/ (comments/strings masked) for .AutoConnect( / .Connect() / .RefCount(, attributes each hit to
its enclosing member, and requires a path<TAB>member<TAB>reason line. The rooted budget is zero — a bare
AutoConnect(/Connect() is red with no allow-list escape (convert to the helper). RefCount() sites are
inventoried with the reason their subscriptions are owned; growing that list also needs RefCountBudget raised,
so it is a reviewed decision. A stale line is a failure. Two non-vacuity tests: the classifier on synthetic
source (bare vs owned vs prose vs expression-bodied member attribution), and the walk (≥ 7 helper call sites
in src/, ≥ 1 RefCount site).

Verification (all Release `-warnaserror`, one project per invocation, `0 Error(s)` read for each)

Built: Messaging.Hub, Mesh.Contract, ContentCollections, Compiler.Pipeline, Hosting, GitSync, Hosting.Orleans,
Hosting.AspNetCore, Messaging.Hub.Test, ContentCollections.Test, Hosting.Test, Documentation.Test — and every
built DLL checked to carry the new symbols (no up-to-date no-op).

OwnedConnectionTest (Messaging.Hub.Test) 8/8: a connect queued on a TestScheduler is cancelled by the
owner's release (control arm: the registration count reads 1 first); live connection released; late subscriber
refused naming the owner; terminated/faulted chain drops its handle; eager ConnectOwnedBy; registration on a
disposed owner disposed on the spot; hub-owned connection released in ShutDown.
ContentCollectionConnectionOwnedByHubTest + ContentCollectionDiesWithItsHubTest 2/2.
QueryConnectionsReleasedOnTeardownTest (fix(MeshNodeStreamCache): release synced-query connections on Dispose — sightings #13/#14 and the teardown chain walked (Plugins#1527) #3737's, on the migrated site) 2/2.
Guard 3/3 + DocumentationLinkIntegrityTest; full runs of Documentation.Test, ContentCollections.Test,
Messaging.Hub.Test green.
Negative controls, run and restored (md5 identical before/after): ContentService back to bare
AutoConnect(1) → the site test fails at Expected value to be OnError … but found OnNext (the disposed
collection replayed); the helper reduced to source.AutoConnect(minObservers) → 6 of 8 helper tests fail at
their control arms (Expected 1 … found 0) and release assertions.

Docs

HubDisposalModel.md → new section "Rooted Rx connections: what a Dispose() must release, and the one
spelling for it"; /async skill rule; What's New Category: Fix.

Was stacked on #3737's head; #3737 merged (9567a2e) while this was in progress and the branch is rebased onto
origin/main. Pairs-with: none — no public type or member is removed
(GitHubRepoIdentityResolver GAINS IDisposable; every other public surface is additive).

🤖 Generated with Claude Code

…d with its owner Follows #3737, which released the synced-query connections of MeshNodeStreamCache. This makes that fix the ONE spelling — OwnedConnectionExtensions.AutoConnectOwnedBy / ConnectOwnedBy — and converts every other bare AutoConnect(1) / dropped Connect() in src/: ContentService (hub-owned collection promise), NodeTypeCompilationHelpers.RunCompile (hub-owned activity create), GitHubRepoIdentityResolver (now IDisposable, owns its in-flight lookups), UserIdentityCache (released in the hub's ShutDown, not only at container disposal), ActivityTracker and RoutingQuiescence (already owned; now visibly so), MeshNodeStreamCache (migrated to the helper). The helper registers the connection on connect, releases it with the owner (a still-queued pool connect is cancelled before it runs), drops a terminated chain's handle, and refuses a subscriber arriving after the release with ObjectDisposedException naming the owner. RootedRxConnectionRatchetGuard keeps src/ at ZERO bare rooted sites (no allow-list escape) and inventories every RefCount() site with the reason its subscriptions are owned (test/RootedRxConnectionSites.allow). Pinned by OwnedConnectionTest (a TestScheduler-queued connect is cancelled by the owner's release, with a control arm) and ContentCollectionConnectionOwnedByHubTest; negative controls run at both. Docs: HubDisposalModel → "Rooted Rx connections"; /async skill; What's New (Fix). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Copilot

🟢 Approval recommended

The changes consistently apply a single owned-connection pattern across all identified rooted Rx sites and add guard + tests + docs to prevent regressions.

Pull request overview

This PR standardizes and enforces correct ownership/lifetime of “rooted” Rx multicast connections in src/ by introducing a single helper spelling (AutoConnectOwnedBy / ConnectOwnedBy), migrating all existing rooted sites to it, and adding a test ratchet to prevent regressions. It closes a disposal-chain gap where a pool-queued Connect() could execute after teardown and resolve services from a disposed scope.

Changes:

Added OwnedConnectionExtensions and migrated rooted connection sites to owner-tracked connections that are released on owner disposal and refuse late subscribers.
Added a governance ratchet (RootedRxConnectionRatchetGuard) plus an allow-list inventory for RefCount() sites.
Added targeted tests + docs/skill updates and a What’s New entry describing the behavioral fix.

File summaries

File	Description
test/RootedRxConnectionSites.allow	Inventory of `RefCount()` sites (with reasons) consumed by the new ratchet guard.
test/MeshWeaver.Messaging.Hub.Test/OwnedConnectionTest.cs	Verifies owned rooted connections are registered, released/cancelled on disposal, and refuse late subscribers.
test/MeshWeaver.Messaging.Hub.Test/MeshWeaver.Messaging.Hub.Test.csproj	Adds `Microsoft.Reactive.Testing` dependency for deterministic scheduler-based tests.
test/MeshWeaver.Documentation.Test/RootedRxConnectionRatchetGuard.cs	New guard that scans `src/` for rooted connection spellings and enforces budgets/inventory.
test/MeshWeaver.ContentCollections.Test/ContentCollectionConnectionOwnedByHubTest.cs	Regression test: post-teardown collection requests are refused (not replayed from a dead promise).
src/MeshWeaver.Messaging.Hub/OwnedConnectionExtensions.cs	Introduces the “one spelling” helper for rooted connections (`AutoConnectOwnedBy` / `ConnectOwnedBy`).
src/MeshWeaver.Mesh.Contract/ActivityTracker.cs	Migrates eager `Connect()` ownership to `ConnectOwnedBy` so disposal deterministically releases the connection.
src/MeshWeaver.Hosting/MeshNodeStreamCache.cs	Replaces ad-hoc connection tracking with `AutoConnectOwnedBy` for synced-query streams; removes bespoke release helper.
src/MeshWeaver.Hosting.Orleans/RoutingQuiescence.cs	Migrates eager `Connect()` ownership to `ConnectOwnedBy` for deterministic disposal.
src/MeshWeaver.Hosting.AspNetCore/Portal/UserIdentityCache.cs	Moves publish/connect lifetime under a hub-registered `CompositeDisposable` and uses `ConnectOwnedBy`.
src/MeshWeaver.GitSync/GitHubRepoIdentityResolver.cs	Makes lookup promises owned/disposable via a `CompositeDisposable` and `AutoConnectOwnedBy`; implements `IDisposable`.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-08-a-shutdown-releases-every-shared-subscription-it-owns.md	User-facing release note describing the fix and the observable behavioral change.
src/MeshWeaver.Documentation/Data/Architecture/HubDisposalModel.md	Documents the rooted-connection defect class and mandates the helper spelling.
src/MeshWeaver.ContentCollections/ContentService.cs	Converts collection initialization promises to hub-owned connections via `AutoConnectOwnedBy`.
src/MeshWeaver.Compiler.Pipeline/NodeTypeCompilationHelpers.cs	Converts compile pipeline promise to hub-owned connection via `AutoConnectOwnedBy`.
.claude/skills/async/SKILL.md	Updates async skill guidance to include the “rooted connections must be owned” rule and ratchet reference.

Review details

Files reviewed: 16/16 changed files
Comments generated: 0
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

github-actions · 2026-09-08T18:04:10Z