fix(#3321): step 3 — a disposed stream releases its hub by rbuergi · Pull Request #3427 · Systemorph/MeshWeaver

rbuergi · 2026-09-06T14:33:13Z

Step 3 of 3 for #3321 — the step that actually reclaims the leak. Steps 1 (#3380, make liveness
askable) and 2 (#3386, migrate the dereference sites) landed first, in that order, because doing
this one before them is the production NRE this issue is about.

Closes #3321.

🚨 `Dispose()` clearing the field would have reclaimed 11 of 1 496

The step is usually described as "SynchronizationStream.Dispose() clears Hub". Read against the
heap dump on the issue, that description is a sixth of the fix:

SynchronizationStream total=8461 disposed=11
   1485  <MeshNode>     hub RunLevel=Dead, stream NOT disposed
     11  <JsonElement>  hub RunLevel=Dead, stream DISPOSED

A sync/ hub is a HOSTED hub: its parent disposes it during the parent's own teardown — a Blazor
circuit ending, a DisposeRequest, a recycle — while the stream that created it is owned by a
workspace somewhere else and is never told. Dispose() never runs for those 1 485.

So the release happens at both ends, through one private ReleaseHub() (idempotent, CAS'd,
callable from any thread; it disposes nothing — it only drops a reference):

End	Where	Covers
the stream is disposed	`Dispose()`, after `Hub.Dispose()`	the 11
the hub dies underneath it	`syncHub.RegisterForDisposal(_ => ReleaseHub())`, from the constructor	the 1 485

The constructor's hook is registered after RegisterForDisposal(resyncSubscription), and that
ordering is load-bearing: that call is what hooks streamDisposables onto the hub, and a hub's
composite disposes in registration order — so the stream's own registrants run first and still see a
bound Hub, and the release runs after them. A hub already past accepting registrants disposes the
hook inline, so the constructor re-checks and refuses with HubDisposingException exactly as its
syncHub is null branch does.

No new state enters the liveness vocabulary: StreamLiveness.IsUsable has always treated
current.Hub is not { } hub as dead, so a released stream reports itself unusable through the
predicate every cache already consults.

The site classification — 29 found, 4 untouched, 25 resolved

A fresh sweep of src/ for every .Hub whose receiver resolves to an ISynchronizationStream
found 29 occurrences on 27 lines. Four stay untouched and must: two ARE the predicate
(StreamLiveness.IsUsable's chain walk, TryGetHub's own read), one is already stream?.Hub?.…,
one is Workspace's cache probe, which is itself a liveness check.

The other 25 are this step's work — the 20 step 2 named, plus five it did not:

the three LayoutAreaHost reads step 2 called "hub-turn confined". That rested on a hub
executing a turn is by construction alive — a statement about the HUB. Step 3 makes the FIELD
independently clearable, so a queued UpdateStreamRequest can now run after the release. The
premise expired with the change that needed it.
Workspace.EvictClientSubscriptions — step 2 placed it in "already answerable" for the right
reason (an IsUsable guard there would SKIP a disposal), but a null check is still needed.
WorkspaceExtensions.ApplyChanges — step 2's list missed it (stream!.Hub.Version, behind a
null-forgiving !).

Group	Sites	Resolution
Patch reducers — `StandardReducers` (10), `MeshDataSource.PatchMeshNode` (2)	12	`RequireHub()`, absorbed by their single gateway
creation-path `Subscribe` callback — `WorkspaceStreams` (2), `PartitionedHubDataSource` (2)	4	`HubIfHeld()` once at the top; skip the emission
inside a `stream.Update(…)` lambda — `LayoutAreaHost` (3), `VirtualDataSource` (1)	4	`HubIfHeld()`, then `return null` — `Update`'s documented no-op
`JsonSynchronizationStream`'s `reduced.Hub.Register`	2	`HubIfHeld()` once; don't register
`GenericUnpartitionedDataSource.Initialized`	1	a released stream contributes no task to wait for
`WorkspaceExtensions.ApplyChanges`	1	`RequireHub()` — same shape as a reducer
`Workspace.EvictClientSubscriptions`	1	`Hub is { }` — deliberately not a liveness check
	25

The reducers were the design problem, and their single gateway is the answer. A reducer's
signature is … → ChangeItem<T>: it MUST return a change, so it has no absent value to hand back.
But every patch reducer in the codebase is invoked from exactly ONE place —
JsonSynchronizationStream.ToChangeItem, the only call site of ReduceManager.PatchFunction — and
null is what ToChangeItem already returns for a stream with no PatchFunction registered. So
the reducers read the hub once through RequireHub(), which refuses with the TRANSIENT
HubDisposingException (an ObjectDisposedException → ErrorType.ShuttingDown, the "ask again"
answer, never the terminal DeliveryFailure the predecessor's raw NRE produced), and ToChangeItem
catches only that type and answers null — which both callers already handle
(BuildChangeItem falls through to its type-registry path; UpdateStream falls back to
deserializing through Host.JsonSerializerOptions). Not a swallow, and not a
catch (NullReferenceException): every other exception a reducer throws still propagates.

🚨 PRESENCE is not LIVENESS — and the wrong one is a behaviour change, measured

The first draft wrote every owner-side guard as TryGetHub(), on the reasoning that step 2's rule
was "migrate onto TryGetHub()". That is wrong, and one existing test proved it before the change
left the branch.

DataSource.Initialized is Task.WhenAll(streams.Select(s => s.Hub.Started)). Guarded with
TryGetHub(), a stream whose initial load faulted is excluded — because it is not IsUsable,
which is exactly what IsFaulted is for. So the WhenAll no longer held the task carrying the fault,
Initialized completed successfully, and a hub whose data source had thrown went on answering
requests as though nothing had happened. DataContextFaultedInitBeforeStreamHubBoundTest went red
on precisely that — the #2625 wedge, re-introduced by a guard that looked like a tightening of
safety.

So there are two accessors for two questions:

Accessor	Question	Where
`TryGetHub()` (`IsUsable`)	should I use this stream? — chain walk, faulted store, winding-down hub	a consumer, a cache, a read — step 2's 28 sites
`HubIfHeld()` / `RequireHub()`	is the reference still there? — one field read	an owner writing into its own stream

The rule for an owner-side guard is: match the guard the site's own gate already applies. Each of
them ends in stream.OnNext or stream.Update, whose refusal is isDisposed || Hub is null, so a
presence check refuses in exactly the cases the write would have been dropped anyway and never one
more. TryGetHub() there is a silent policy change smuggled in under a null-safety fix — the shape
step 2 already declined at Workspace's cache probe. RequireHub() is written in terms of
HubIfHeld(), so there is one definition of presence and one of liveness, never a fourth
hand-copied predicate (#1455).

And every read inside the stream now resolves the field once

Step 2 excluded SynchronizationStream's own bare Hub reads as "already guarded". They were —
against a field that could not change. Once the field can be nulled, a guard followed by a re-read
is check-then-act across threads, so every read below a guard now uses the local the guard resolved:
OwnerVersion(hub), BuildChangeItem(hub, …), BuildFullChangeItem(hub, …), and the locals in
RegisterForDisposal / DeliverMessage / OnNext / OnError.

One log level moved for a reason, not for debugging: OnError's deliberately LOUD "the stream has no
Hub bound" LogError (#2625) gained a second, benign way to be reached — the hub was released
because it died — and that arm logs Debug instead. The original arm, for a stream whose hub was never
bound, is unchanged.

Tests, and the measurement that says they can fail

test/MeshWeaver.Data.Test/StreamReleasesItsHubTest.cs, 6 cases. The flagship assertion is a
WeakReference, not Hub is null: "the field reads null" is a statement about one field;
"the hub is unreachable" is the statement this issue is about, and it is the only one that goes
red on the defect for the right reason. The hub is created and dropped inside a non-inlined helper
so no live local on the test's frame can root it. It passes.

Falsified by disabling only the two ReleaseHub() call sites — everything else, RequireHub and
every guard included, left in place, so the experiment isolates exactly what step 3 does — and
re-running both suites unchanged:

Suite	release disabled	release enabled
`StreamReleasesItsHubTest` (`MeshWeaver.Data.Test`)	4 failed, 1 passed	5 passed
`TornDownStreamCallSitesTest` (`MeshWeaver.Layout.Test`)	4 failed, 1 passed	5 passed

(Measured before the sixth case was added.) The live control passed in both runs of both suites — a
release that fired unconditionally, or a guard that answered "dead" always, would satisfy every
negative at once, and the controls are what rule that out. The four pre-change failures name four
different facts: Hub not null after Dispose(), Hub not null after the hub died underneath,
RequireHub not throwing, and ToChangeItem still deriving a change off the corpse.

A full src/ revert does not compile the tests, because RequireHub is itself part of step 3 —
which is why the falsification reverts the release rather than the whole change.

Two existing assertions INVERT here, deliberately kept rather than deleted, because they are the
lines that pin which contract is in force:
TornDownStreamCallSitesTest and StreamLivenessIsConsumerReachableTest both asserted
stream.Hub.Should().NotBeNull() after disposal — "the non-nullable contract still holds, which is
precisely why a consumer needed a separate way to ask". Step 3 is what makes that stop holding.

Verification

Local, -c Release -warnaserror, one project per invocation:

	Result
whole solution (64 projects)	0 Warning(s), 0 Error(s)

Suite	Result
`MeshWeaver.Data.Test`	484 passed, 0 failed
`MeshWeaver.Layout.Test`	450 passed, 0 failed
`MeshWeaver.Graph.Test`	1366 passed, 0 failed
`MeshWeaver.Documentation.Test`	279 passed, 0 failed

Work products conserved

Doc/Architecture/StreamLivenessAndTheHubReference gains "What step 3 did" — the 11-vs-1 485 split,
the two-ended release with the registration-order constraint, the 29/4/25 site table with the reason
per group, the measured PRESENCE-vs-LIVENESS finding, and the falsification numbers. Plus a What's
New entry (Category: Fix).

i18n

No new user-visible strings. HubDisposingException's message is existing framework text.

Cross-repo

Pairs-with: Systemorph/MeshWeaver.Plugins#1405

🚨 Not because this removes public surface — it removes none (zero public types, zero public
members; the gate does not fire). This is shape 7: ISynchronizationStream.Hub is a public
member of a public interface, and step 3 changes what it ANSWERS behind an unchanged signature,
which no surface detector can see by construction.

A by-hand sweep of every satellite checkout, on the RECEIVER type and including in-mesh .json
nodes, found the blast radius is one repo and one cluster: 11 production dereferences in
MeshWeaver.Plugins's Blazor view layer, all on the inherited
ISynchronizationStream<JsonElement>? Stream. The ! in Stream!.Hub.Something is on Stream,
not on .Hub, so the compiler stays silent; one of them sits inside a bare catch { }, so it would
have been silent at runtime too. The four content satellites are clean structurally
(ISynchronizationStream appears in zero files in four of them).

The dependent landed FIRST, and this PR was held as a draft until it did — its guards are no-ops
on a live stream, so they are safe against the old core, while core's release is not safe against an
unguarded dependent. The full table is in the doc page under
"The dependent half — and why no gate could have told you".

A SynchronizationStream held its sync sub-hub for the process's whole life. A production heap dump named it as the sole retainer of 1 496 MessageHubs at RunLevel=Dead — ~390 KB apiece, ~580 MB of a 3.5 GB live heap, and the reason 267 gen2 collections freed nothing. The release happens at BOTH ends, through one idempotent ReleaseHub(): Dispose() (11 of the 1 496), and a hook on the sub-hub's own disposal for the far more common case where the PARENT tears it down while nobody ever disposes the stream (the other 1 485). The constructor registers that hook AFTER streamDisposables is hooked onto the hub, so the stream's own registrants still see a bound Hub. Nulling the field was tried once before and cost a production NRE in LayoutAreaHost's constructor that reached the subscriber as a TERMINAL DeliveryFailure. What makes it safe now is steps 1 (#3380) and 2 (#3386) plus the 25 sites resolved here: - 12 patch reducers -> RequireHub(), whose TRANSIENT HubDisposingException their single gateway (ToChangeItem) absorbs into the `null` its own signature already models - 8 owner-side write paths -> HubIfHeld(), then the no-op each already models (skip the emission / return null from the Update transform) - 2 handler registrations -> don't register - Initialized, ApplyChanges, EvictClientSubscriptions -> one each PRESENCE is not LIVENESS: guarding DataSource.Initialized with TryGetHub() (IsUsable) excluded a FAULTED stream from its WhenAll, so a hub whose data source had thrown reported successful initialization — DataContextFaultedInitBeforeStreamHubBoundTest caught it. Owner-side guards therefore use HubIfHeld()/RequireHub(), which match the `isDisposed || Hub is null` refusal their own gate already applies. Every read inside SynchronizationStream now resolves the field once into a local rather than re-reading it after its own guard. Falsified by disabling only the two ReleaseHub() call sites: 4 failed / 1 passed in both new-and-existing suites, 5 passed each with the release in place; the live controls passed both ways. Doc/Architecture/StreamLivenessAndTheHubReference carries the design; What's New carries the user-facing entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

rbuergi · 2026-09-06T14:35:12Z

🚨 HELD AS DRAFT — the dependent half has to land first

A read-only sweep of every satellite checkout on this machine for .Hub on a stream receiver
found the break this change would cause outside core. It is shape 7 of the seven — a method's
BEHAVIOUR changing behind an unchanged signature — which no surface detector can see by
construction, and which the Cross-repo pair (public surface) gate therefore does not fire on
(measured: this diff removes zero public types and zero public members).

What the sweep found

Repo	stream-receiver `.Hub`	Raw `.Hub` lines classified
MeshWeaver.Plugins	22 (11 production, 11 test)	2 051
education	0	483
MeshWeaver.Education	0	11
MeshWeaver.Reinsurance	0	152
MeshWeaver.SocialMedia	0	73
MeshWeaver.Manufacturing	0	15

The four content satellites are clean structurally, not just by token match:
ISynchronizationStream appears in ZERO files in four of them and in exactly one file in
Reinsurance, where the stream is only .Update(…)d. Every in-mesh carrier was swept too — the 27
.json nodes carrying .Hub inside a C# configuration string all resolve to
IWorkspace.Hub / LayoutAreaHost.Hub / an IMessageHub local.

The 11 that would NRE

All in MeshWeaver.Plugins, all on one inherited property —
BlazorView.razor.cs:62, ISynchronizationStream<JsonElement>? Stream:

BlazorView.OnClick → Stream.Hub.Post(new ClickedEvent(…))
FormComponentBase.OnBlur → Stream.Hub.Post(new BlurEvent(…))
DialogView.HandleClose / HandleDialogResult → Stream!.Hub.Post(new CloseDialogEvent(…))
ViewModelExtensions.GetControl, FormComponentBase.ConversionToValue,
DataGridView.GetPropertyExpression, RadzenChartView.BindData ×2,
RadzenPivotGridView.BindData, RadzenChartView.razor:171 →
Stream!.Hub.JsonSerializerOptions

🚨 The ! is on Stream, not on .Hub — so after this change the NRE moves one level to the
right, past the operator that currently suppresses the warning, and the compiler stays silent.
And RadzenChartView.razor:171 sits inside a bare catch { }, so there it would be invisible at
runtime too: a wrong chart instead of an exception.

That is precisely the incident this issue exists to avoid, relocated one repo over — an NRE in a
Blazor view during a recycle window. Shipping core first would ship it.

Also flagged, not an NRE but a silent test regression:
MeshWeaver.Hosting.Monolith.Test/StreamCacheLivenessTest.cs:146-151 is already null-tolerant
(as + ?.), but its .Where predicate becomes permanently false once Hub can be null, so it
would stop observing the cascade and hang to its 60 s timeout rather than fail an assertion.

Order

Plugins first, core second. Un-drafting this once the Plugins counterpart is merged.

Pairs-with: will be filled in with that PR before this leaves draft.

Copilot

🟢 Approval recommended

The changes are cohesive, well-justified, and include targeted tests/docs; the only noted issue is a minor doc-comment mismatch in a new test helper.

Pull request overview

This PR completes step 3/3 of #3321 by ensuring SynchronizationStream releases its hosted sync-hub reference both when the stream is disposed and when the hosted hub is torn down underneath an undisposed stream, allowing the dead hub’s resolved graph to be garbage collected.

Changes:

Add an idempotent “release hub” mechanism in SynchronizationStream (dispose-path + hub-disposal hook) and adjust internal reads to avoid check-then-act races by resolving the hub once into a local.
Introduce/propagate “presence vs liveness” accessors (HubIfHeld() / RequireHub()) and migrate owner-side .Hub dereferences to use them or to no-op/skip work safely.
Add/adjust tests and documentation/What’s New to lock in the new contract and verify the hub becomes unreachable.

File summaries

File	Description
test/MeshWeaver.Layout.Test/TornDownStreamCallSitesTest.cs	Inverts the assertion to pin the new “disposed stream releases hub” contract.
test/MeshWeaver.Data.Test/StreamReleasesItsHubTest.cs	New test suite asserting hub release/collectability and presence-vs-liveness behavior.
test/MeshWeaver.Data.Test/StreamLivenessIsConsumerReachableTest.cs	Updates the step-3 contract assertion for `Hub` after disposal.
src/MeshWeaver.Layout/Composition/LayoutAreaHost.cs	Uses `HubIfHeld()` inside update lambdas and returns `null` (no-op) when hub was released.
src/MeshWeaver.Graph/MeshDataSource.cs	Switches patch reducer to a single `RequireHub()` read to avoid NRE and enable transient refusal.
src/MeshWeaver.Data/WorkspaceStreams.cs	Guards subscription callback with `HubIfHeld()` and uses the resolved hub once for version stamping.
src/MeshWeaver.Data/WorkspaceExtensions.cs	Uses `RequireHub().Version` to stamp changes where an absent value isn’t modeled.
src/MeshWeaver.Data/Workspace.cs	Avoids skipping disposal by using a pure null check before disposing a stream hub.
src/MeshWeaver.Data/VirtualDataSource.cs	Adds `HubIfHeld()` guard inside update transform and no-ops when the hub reference is released.
src/MeshWeaver.Data/SynchronizationStreamLiveness.cs	Adds the presence accessors (`HubIfHeld`, `RequireHub`) alongside consumer liveness (`TryGetHub`).
src/MeshWeaver.Data/StandardReducers.cs	Threads a single resolved hub through reducers and helper methods to prevent concurrent-release NREs.
src/MeshWeaver.Data/Serialization/SynchronizationStream.cs	Implements `ReleaseHub()`, hooks it into hub disposal + `Dispose()`, and resolves hub once inside guarded paths.
src/MeshWeaver.Data/Serialization/JsonSynchronizationStream.cs	Resolves `reducedHub` once; skips owner-protocol registrations if the hub was already released; maps `HubDisposingException` to modelled `null`.
src/MeshWeaver.Data/Persistence/PartitionedHubDataSource.cs	Mirrors the subscription-callback guard pattern with `HubIfHeld()` and reuses resolved hub for stamping.
src/MeshWeaver.Data/ISynchronizationStream.cs	Updates the contract documentation for `Hub` to reflect the step-3 reality and the safe access patterns.
src/MeshWeaver.Data/GenericUnpartitionedDataSource.cs	Updates `Initialized` to await only hubs still held (presence), avoiding the prior liveness/policy bug.
src/MeshWeaver.Documentation/Data/Architecture/StreamLivenessAndTheHubReference.md	Documents what step 3 changed, the two-ended release, and the presence-vs-liveness distinction.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-06-a-closed-page-gives-its-memory-back.md	Adds a user-facing What’s New entry describing the reclaimed memory behavior.

Review details

Files reviewed: 18/18 changed files
Comments generated: 1
Review effort level: Lite

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

+    /// Forces the collection the reachability assertion reads. Two passes with a finalizer drain
+    /// between them: the first may only queue the object for finalization, and a hub holds
+    /// finalizable state.


github-actions · 2026-09-06T14:38:01Z