fix(rx): an owned connection's refusal goes on the release lane — refusal vs release deadlocked a render leaf in teardown - #5681
Conversation
…usal vs release deadlocked a render leaf in teardown MeshWeaver.Plugins scheduled run 36090164894 (job 107933985813, set 3.0.0-ci.9321): SendDocumentIdentityPanelTest.StampingARecheckOnTheDraft_ReprobesAndReplacesThePanel passed its body in 442 ms, then teardown reported DIRTY after the whole 38 s drain with one Layout leaf (Defer<EntityStoreAndUpdates>, the layout render's pooled subscribe) that never finished. Reproduced in a Linux container on 2 CPUs (1 in 400 runs of that class) and captured with dotnet-stack: the render leaf was subscribing the permission fold (a SelectMany over Zips of MeshNodeStreamCache queries) as the mesh tore down. - the RELEASE LANE was delivering one query's release: it held that inner's Zip gate and was entering the SelectMany gate; - the RENDER thread was subscribing the next inner, whose query was already released and REFUSED with a synchronous Observable.Throw: it held the SelectMany gate forwarding that error and was disposing the first Zip, which takes the Zip gate. #5660 serialised release against release; the refusal was the same terminal delivered off the lane. Every terminal a released owner produces now goes on the lane: - ReleaseLane.Refuse<T>: a cold sequence that errors on the lane, behind earlier releases; - AutoConnectOwnedBy refuses through it, and its release signal refuses a subscriber that attaches after it fired on the lane too (a terminated Subject replays its error synchronously); - MeshNodeStreamCache's disposed-cache query guards refuse through the mesh lane. Also: the lane observed on TaskPoolScheduler.Default, whose ISchedulerLongRunning made ObserveOn park one dedicated thread per lane forever (the lane is never disposed) — one leaked thread per torn-down mesh. It now drains on pooled work items. Tests (OwnedConnectionTest): - ARefusalAndAReleaseMeetingInOneConsumer_NeverDeadlock parks the lane inside the first Zip's gate and lets the subscriber meet it in the order the CI stacks show. Negative control run by hand: with the refusal restored to Observable.Throw the subscriber thread never returns (red after the bound); green with the fix. - ASubscriptionAfterTheRelease_IsRefusedNamingTheOwner_OnTheLane, AReleaseRunsOnAPooledThread_NeverOnADedicatedOne. MeshWeaver.Messaging.Hub.Test 523/523, MeshWeaver.Hosting.Test 959/960 (1 skipped). Doc: HubDisposalModel.md → "A refusal is a release terminal too" and the pooled-drain note. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The regression tests need safer thread-delivery assertions and explicit escape, cleanup, and worker-join handling to prevent wedged teardown.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
What changed in this PR
Moves owned-connection refusals onto the serialized release lane and uses pooled draining to prevent teardown deadlocks and dedicated-thread leaks.
Changes:
- Adds cancellable, lane-delivered refusals.
- Updates owned connections and disposed-cache query handling.
- Adds concurrency tests and documents the lifecycle behavior.
| File | Summary |
|---|---|
test/MeshWeaver.Messaging.Hub.Test/OwnedConnectionTest.cs |
Adds refusal, deadlock, and scheduler tests. |
src/MeshWeaver.Messaging.Hub/ReleaseLane.cs |
Adds lane-based refusals and pooled draining. |
src/MeshWeaver.Messaging.Hub/OwnedConnectionExtensions.cs |
Routes release and refusal terminals through the lane. |
src/MeshWeaver.Hosting/MeshNodeStreamCache.cs |
Routes disposed-cache failures through the mesh lane. |
src/MeshWeaver.Documentation/Data/Architecture/HubDisposalModel.md |
Documents refusal serialization and pooled draining. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } | ||
| finally | ||
| { | ||
| Volatile.Write(ref subscriberReturned, 1); | ||
| } |
There was a problem hiding this comment.
Agreed on the teardown half — fixed in 128a29e. The connection is no longer a using: it is disposed (after joining the worker) only on the green path. On the red path the subscriberReturned assertion fails first and nothing walks back into the gates the two deadlocked threads hold, so a regression reports as a named assertion failure, not a hung teardown.
On "release the parked callbacks": both parks are already bounded SpinWait.SpinUntil(…, parkBudget), so no callback of this test is ever left waiting on a flag. What remains in the regression case is the lock-order deadlock itself — two threads blocked in Monitor.Enter on each other's gate — and nothing outside can release that; it is left on background threads (the worker is IsBackground, the lane runs on pool threads). I ran exactly that negative control by hand (refusal restored to Observable.Throw): the test failed at the subscriberReturned assertion after the bound and the host exited normally.
Test Results (shard 3)430 tests 430 ✅ 1m 13s ⏱️ Results for commit 128a29e. ♻️ This comment has been updated with latest results. |
Test Results (shard 0) 1 files 1 suites 2m 32s ⏱️ Results for commit 128a29e. ♻️ This comment has been updated with latest results. |
Test Results (shard 1)550 tests 550 ✅ 1m 7s ⏱️ Results for commit 128a29e. ♻️ This comment has been updated with latest results. |
…egression would leave wedged On the green path the worker is joined and the connection disposed; on the red path the assertion fails first and nothing walks back into the two gates the deadlocked threads hold. Both parks stay bounded, so no callback is left waiting on a flag. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Test Results (shard 4) 2 files 2 suites 6m 2s ⏱️ Results for commit 128a29e. |
Test Results (shard 2)1 625 tests 1 625 ✅ 9m 12s ⏱️ Results for commit 128a29e. |
Test Results (shard 5) 5 files 5 suites 15m 22s ⏱️ Results for commit 128a29e. |
Test Results 13 files 13 suites 35m 30s ⏱️ Results for commit 128a29e. |
The failure
MeshWeaver.Plugins scheduled run 36090164894 (job 107933985813, platform set 3.0.0-ci.9321, core at 651ee2d):
SendDocumentIdentityPanelTest.StampingARecheckOnTheDraft_ReprobesAndReplacesThePanelpassed its body in 442 ms, then teardown reportedafter the whole 38 s drain. The leaf is
LayoutAreaHost's top-level render subscribe on the Layout pool.Root cause (captured, not inferred)
Reproduced in a Linux container on 2 CPUs (1 failure in 400 runs of that class, the exact CI signature) and captured with
dotnet-stackon the hung host. The render leaf was subscribing the permission fold (aSelectManyoverZips ofMeshNodeStreamCachequeries) when the mesh tore down:ReleaseLane.Run → ReleaseSignal → TakeUntil → … → Zip.SecondObserver.OnError) held that innerZip's gate and was blocked enteringSelectMany.InnerObserver.OnError(theSelectManygate);… Zip.Run → Defer → ThrowImmediate, the owned connection's synchronous refusal) held theSelectManygate forwarding that error and was blocked inZip._.Dispose(bool)disposing the first inner.#5660 serialised release against release on one lane; the refusal of a subscriber arriving after the release was the same terminal, still delivered synchronously on the subscriber's thread — so it raced the lane into the same gates.
Fix
Every terminal a released owner produces is delivered on the lane:
ReleaseLane.Refuse<T>(Func<Exception>)— a cold sequence that errors on the lane, behind earlier releases, cancellable by disposal.AutoConnectOwnedByrefuses through it; its release signal refuses a subscriber that attaches after it fired on the lane too (a terminatedSubjectreplays its error synchronously — the same race one line later).MeshNodeStreamCache's disposed-cache query guards refuse through the mesh lane instead ofObservable.Throw.Also in
ReleaseLane:ObserveOn(TaskPoolScheduler.Default)took Rx's long-running path and parked one dedicated thread per lane forever (the lane is never disposed) — one leaked thread per torn-down mesh, visible in every stack capture. It now drains on pooled work items (DisableOptimizations(typeof(ISchedulerLongRunning))), same serial order.Tests
OwnedConnectionTest:ARefusalAndAReleaseMeetingInOneConsumer_NeverDeadlock— deterministic: parks the lane inside the firstZip's gate and lets the subscriber meet it in the order the CI stacks show. Negative control (run by hand): with the refusal restored toObservable.Throw, the subscriber thread never returns (red after the bound, threads deadlocked on the captured frames); green with the fix.ASubscriptionAfterTheRelease_IsRefusedNamingTheOwner_OnTheLane(was the synchronous-refusal assertion),AHubOwnedConnection_IsReleasedInTheHubsShutDownnow awaits the refusal.AReleaseRunsOnAPooledThread_NeverOnADedicatedOne.Local:
MeshWeaver.Messaging.Hub.Test523/523,MeshWeaver.Hosting.Test959/960 (1 skipped). Release-warnaserrorclean for Messaging.Hub.Test, Hosting.Test, Documentation.Doc:
HubDisposalModel.md→ "A refusal is a release terminal too, so it goes on the same lane" and the pooled-drain paragraph.Pairs-with: none — no public type or member removed (one public method added to a sealed class).
Refs: the incident is Plugins run 36090164894; intermittent, so it closes on the Plugins daily run against a set carrying this commit, not on this merge.
🤖 Generated with Claude Code