A shutdown reject classified as Failed reads as terminal by rbuergi · Pull Request #1023 · Systemorph/MeshWeaver

rbuergi · 2026-08-09T15:48:31Z

MonolithRoutingService.PostNotFound classified a routing reject issued during mesh
shutdown as ErrorType.Failed. Two consumers match on ErrorType.ShuttingDown
explicitly and change behaviour on it:

JsonSynchronizationStream.cs:375 — the SubscribeRequest OnError arm returns early and
keeps the stream and its keep-alive alive for the change-feed resubscribe latch.
SynchronizationStream.cs:1019 — the DeliveryFailure handler returns Processed()
instead of calling OnError.

Failed reads as terminal to both: the stream faults, the resubscribe latch dies with it, and
every read of a mid-recycle NodeType waits to its timeout. That is CI 30003419841 /
NodeTypeCompileParkTest.RecycleRetry — the exact regression the ShuttingDown member exists
to prevent. Introduced by #1022 (3b1423208); a reviewer flagged it there and it merged past
the unresolved thread. This is the forward fix.

Scope. This is the monolith router: MonolithRoutingService is registered only by
UseMonolithMesh — Memex.Portal.Monolith, Memex.LocalMesh, Memex.Client (MAUI),
Memex.NodeType.Bake, and every MonolithMeshTestBase test in CI. It is not used by
Memex.Portal.Distributed, which routes through Orleans (UseOrleansMeshServer), so the AKS
portals were never affected.

What is in the PR

3d639265a — the one-line correction plus the comment saying why the member is load-bearing.

c2cb04e77 — the two pins that were missing. Reverting the fix left the whole suite green,
which is what made the regression possible in the first place. Each test was verified to fail
against the defect it guards:

Test	Guards	Negative control
`ShutdownRoutingRejectClassificationTest` (Hosting.Monolith.Test)	the router emits `ShuttingDown`	revert to `ErrorType.Failed` → RED (`Expected … ShuttingDown … but found Failed`), while the shutdown-message assertion still passes — proving it rode the branch under test, not an ordinary NotFound
`ShutdownFailureRideOutTest` (Data.Test)	the consumer rides it out and still faults on anything else	disable the `ShuttingDown` branch in `SynchronizationStream` → RED (`but found "transient-shutdown-reject"`)

1b40c1bbe — raises the quiesce budget for that one test class. The fixture's 500 ms budget
is a leaked-callback detector that assumes no test has a request in flight when Quiescing starts;
this one does by construction (the probe's pending callback is what holds the window open), so on
a loaded 2-core runner the detector could fire before the assertion and fail the whole class with
a leak report — a flake in whichever shard drew it. The probe's own Rx timeout was lowered to 10 s
so it speaks first: an unanswered request still fails here, loudly, on the right assertion.

Both windows are constructed, not waited for — no sleeps, no polling:

The routing reject only exists while IsDisposing is true and RunLevel < DisposeHostedHubs; outside that, routing short-circuits and nothing is NACKed at all. The
mesh action block is stalled with a gated handler (the TeardownHubCreationFreezeTest
pattern) and the probe is queued ahead of Dispose()'s ShutdownRequest. Because
Observe pre-registers its callback before posting, the pending callback that Quiescing
waits to drain is the probe — Quiescing cannot race past the reject, and the NACK drains
it. Runs in ~0.5 s with a clean teardown, no quiesce timeout.
The ride-out test delivers both failures back-to-back to the stream hub's single-threaded
action block, so FIFO decides which branch ran. The terminal half is asserted in the same
observation, so the ride-out cannot be widened into a blanket swallow of DeliveryFailure.

Sibling routing paths — checked, no second instance

Every other DeliveryFailure construction on a routing path was reviewed. None has the same
shutdown-vs-terminal misclassification, because none of them has a shutdown signal in scope:

RoutingServiceBase.PostNotFound (:325) — NotFound, and it is only reached when the path
genuinely does not resolve. Correct.
RoutingGrain :182 NotFound (unresolvable path — mirrors the monolith), :210 Failed
(resolution faulted/timed out), :270/:288 Failed (grain could not service the delivery,
after DeliverToGrainWithRetry has already retried the transient Orleans reject), :112
Failed (memory-stream forward fault). All terminal answers for that delivery, none gated on
a disposal flag.
MeshNodeStreamCache.cs:1127 documents the Orleans mid-deactivation reject as
ErrorType.Failed deliberately — it is matched by message content
(IsTransientOwnerFailure), not by type. Left alone, as intended.

One adjacent smell noted but not changed here (no shutdown misclassification, out of scope):
OrleansRoutingService.SendDeliveryFailure (:255) hard-codes ErrorType.Failed for every
failure it relays, discarding whatever classification the grain decided and keeping only the
message text. The grain's own PostFailureToSender carries the correct type on the live path,
so this is a latent flattening rather than an active defect.

Note on the `FutuRe.Test` exit=139 seen on an earlier run of this PR

That SIGSEGV is an upstream CoreCLR background-GC fault, not this change:
WKS::gc_heap::background_sweep() dereferencing a swept object whose MethodTable pointer is
exactly 0x0 (si_code=SEGV_MAPERR, si_addr=0x0). It is characterised at instruction
granularity across three dumps, it survived #996, #967 and 74bc3a07c, and #613 is closed as
external. The crashing test's name carries no information for this class. Note this is not
the ALC use-after-unload story in the older RCA — that faults on a non-null unmapped address,
never on 0x0 — so the "stragglers re-enter hub construction over an unloading ALC" mechanism
does not apply here.

Stated at its honest strength, and no stronger: keeping sync streams alive rather than faulting
them could in principle increase exposure to a concurrent background sweep. That is exposure,
not causation, and the fault itself is upstream and closed.

What's New

Skipped, deliberately. #1022 introduced this defect at 15:20Z today and this corrects it the
same day — there is no delta a user could observe between the last shipped state and this one,
so there is nothing to announce as fixed. #1022 itself correctly shipped no entry for the same
reason: this is messaging-layer classification, internal to the routing/sync plumbing.

Refs #981

🤖 Generated with Claude Code

#1022 added the errorType branch in MonolithRoutingService.PostNotFound and picked ErrorType.Failed for the shutting-down case. Consumers with their own recovery machinery key on ErrorType.ShuttingDown to ride a reject out instead of tearing down — JsonSynchronizationStream and SynchronizationStream both match on that member explicitly. So the shutdown reject reached them looking terminal. That is the CI 30003419841 wedge the ShuttingDown member exists to prevent: erroring the sync stream on this NACK killed the resubscribe latch and wedged every read of a mid-recycle NodeType. The reviewer caught this on #1022 and I merged past the thread. Fixing forward. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The one-line fix in 3d63926 is only as durable as what pins it, and nothing did: reverting ErrorType.ShuttingDown to ErrorType.Failed left the whole suite green. Two tests now make each half of the contract impossible to reintroduce, and each was verified to FAIL against the defect it guards. ShutdownRoutingRejectClassificationTest pins the producing side — a routing reject issued while the mesh is shutting down carries ShuttingDown. That reject only exists in a narrow window (IsDisposing true, RunLevel still below DisposeHostedHubs; outside it routing short-circuits and nothing is NACKed), so the window is constructed rather than waited for: the mesh action block is stalled with a gated handler and the probe is queued ahead of Dispose()'s ShutdownRequest. Observe pre-registers the probe's callback before posting, so the pending callback that Quiescing waits to drain is the probe itself — Quiescing cannot race past the reject, and the NACK drains it. No sleep, no polling for a state. Reverted, it fails on the ErrorType while still asserting the shutdown message, which is what proves it rode the branch under test rather than an ordinary NotFound. ShutdownFailureRideOutTest pins the consequence, which is the half that actually costs something. A correctly-labelled reject that the consumer still treats as terminal reproduces the wedge exactly, and the SynchronizationStream ride-out branch had no test at all. Both failures go to the stream hub's single-threaded action block back to back, so FIFO decides which branch ran: fault on the terminal one behind it and the shutdown reject was ridden out; fault on the shutdown reject and the transient branch is gone. One observation, no timing dependency, and the terminal half is asserted in the same breath so the ride-out cannot widen into a blanket swallow of DeliveryFailure. Refs #981 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…322c2f5f880

The fixture's 500 ms quiesce budget exists to catch a leaked Observe subscription, and it assumes no test has a request in flight when Quiescing starts. This one does by construction — the probe's pending callback IS what holds the shutdown window open. Its answer crosses two thread-pool hops, so on a loaded 2-core runner the budget can expire first and fail the CLASS with a leak report instead of the assertion, which reads as a flake in whichever shard drew it. Budget raised for this class only, and the probe's Rx timeout lowered to 10 s so it fires FIRST. A genuinely unanswered request therefore still fails here, loudly, on the right assertion — the detector is not weakened, it is just no longer the thing that speaks first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Fixes routing failure classification during mesh shutdown so downstream consumers that explicitly key off ErrorType.ShuttingDown treat the condition as transient (ride out) rather than terminal (fault stream), preventing sync-stream wedge/timeouts during mid-recycle.

Changes:

Correct MonolithRoutingService.PostNotFound to emit ErrorType.ShuttingDown (not Failed) when rejecting due to mesh shutdown.
Add a monolith integration test pinning the router’s shutdown-reject classification.
Add a data-layer test pinning SynchronizationStream behavior: ride out ShuttingDown, but still fault on other failures.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/MeshWeaver.Hosting.Monolith/MonolithRoutingService.cs	Classifies shutdown routing rejects as `ErrorType.ShuttingDown` to preserve transient handling semantics.
test/MeshWeaver.Hosting.Monolith.Test/ShutdownRoutingRejectClassificationTest.cs	Deterministically reproduces the shutdown window and asserts the router emits `ShuttingDown`.
test/MeshWeaver.Data.Test/ShutdownFailureRideOutTest.cs	Asserts `SynchronizationStream` rides out `ShuttingDown` but remains terminal for other failures.

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

Copilot review: SynchronizationStream<T> owns a hosted sub-hub, and the test left it to the host's teardown cascade. That works, but it is not deterministic — and on the failing path the assertion throws before any cleanup at all. `using` releases it on both paths. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions · 2026-08-09T16:08:24Z