Repository navigation
fix(#4023): the mesh router carries a reply to its live recipient during DisposeHostedHubs - #4036
Merged
meshweaver-cloud[bot] merged 2 commits intoSep 11, 2026
Conversation
…ing DisposeHostedHubs RoutingServiceBase.RouteInMesh dropped EVERY delivery once the mesh reached DisposeHostedHubs — no NACK, no log, Forwarded returned — because "recipients are likely also disposing". That is the #2778 assumption ("nobody is waiting") at its third site, after the owner's disposal NACK (#2868) and HierarchicalRouting's route-up refusal (#3303). A hosted hub whose parent is in that phase is Quiescing, waiting for exactly the replies it is owed. Measured: NackReachesTheWaiterDuringTeardownTest's two CI failures and 4 of 3,600 stressed local iterations. The owner COMMITTED the patch and posted its ack while the mesh was Quiescing; the mesh routed it one turn after its own DisposeHostedHubs turn, here, where it vanished. The caller's hub sat Quiescing with the callback pending for its whole 2 s budget; its response wait expired first and left the late watch armed (still armed 5 s later), so the writer reports OwnerUnreachable after 31 s for an applied write. The ShutDown-phase registrant ran with claimed=False every time: no OwnerDisposing NACK was ever minted, so #2868's fix never failed — #3291 moved the test off it. Fix, scoped as #3303 scoped its seam: at that run level a delivery carrying PostOptions.RequestId is still delivered to an ALREADY-EXISTING hosted recipient (HostedHubCreation.Never); everything else keeps the historical drop. The recipient's own intake gate still decides admission. Monolith only — OrleansRoutingService has no such guard. Controls (ReplyRoutedDuringMeshTeardownReachesItsWaiterTest), both falsified: the positive fails on the unfixed router; the negative (same payload, no correlation id) fails on an over-broad fix. Docs: DisposalStallVerdicts "Resolved 2026-09-11"; TeardownVerdictsAreCausal correction (Dead is when the owner could ANSWER, not when the answer ARRIVED, on the ordinary route). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Contributor
Contributor
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate issues affect delivery ordering, stream recipients, and regression-test error handling.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes loss of correlated replies during monolith mesh teardown while preserving rejection of uncorrelated traffic.
Changes:
- Adds teardown-aware reply routing and regression tests.
- Updates architecture documentation and release notes.
File summaries
| File | Summary |
|---|---|
test/MeshWeaver.Graph.Test/ReplyRoutedDuringMeshTeardownReachesItsWaiterTest.cs |
Adds positive and negative routing controls. Moderate finding (1 vote): route errors should reach the assertion. |
src/MeshWeaver.Hosting/RoutingServiceBase.cs |
Routes correlated teardown replies. Moderate findings: preserve activation FIFO ordering (3 votes) and support or explicitly exclude registered stream callbacks (1 vote). |
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-11-a-saved-change-is-confirmed-even-during-shutdown.md |
Adds the user-facing fix note. |
src/MeshWeaver.Documentation/Data/Architecture/TeardownVerdictsAreCausal.md |
Corrects teardown timing guidance. |
src/MeshWeaver.Documentation/Data/Architecture/DisposalStallVerdicts.md |
Documents the incident and resolution. |
Review details
Suppressed comments (2)
src/MeshWeaver.Hosting/RoutingServiceBase.cs:177
- This path only probes hosted hubs and never consults the Monolith router's registered stream callbacks.
IRoutingService.RegisterStream(Address, callback)is a supported recipient that can exist without a hosted hub (for example,ChangeFeedResetReleasesUpstreamTestregisters one directly), so a correlated response to that still-registered callback is classified asREPLY_UNROUTABLE_DURING_MESH_TEARDOWNand dropped. The teardown reply route also needs a Monolith stream lookup/delivery hook, or those stream recipients must be explicitly excluded and covered by the contract.
var address = GetHostAddress(delivery.Target);
if (Mesh.GetHostedHub(address, HostedHubCreation.Never) is { } recipient)
{
Mesh.NoteRequestStage(requestId,
$"REPLY_ROUTED_DURING_MESH_TEARDOWN to={recipient.Address} meshRunLevel={Mesh.RunLevel}");
recipient.DeliverMessage(delivery);
test/MeshWeaver.Graph.Test/ReplyRoutedDuringMeshTeardownReachesItsWaiterTest.cs:215
- The error callback only writes to test output, so the negative control can pass if
DeliverMessagethrows instead of refusing the uncorrelated delivery:arrivedremains unchanged and the assertion still succeeds. Surface route errors to the assertion (or assert the returned delivery state) so this control distinguishes a deliberate drop from a broken router.
private void Deliver(IRoutingService routing, IMessageDelivery delivery)
=> routing.DeliverMessage(delivery.Package(Mesh.JsonSerializerOptions))
.Subscribe(_ => { }, ex => Output.WriteLine($"[route] delivery errored: {ex.Message}"));
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Contributor
Contributor
Contributor
…stream recipients, errors asserted Addresses the Copilot review on #4036: - FIFO parity. The teardown answer path returned before the activation-serializer check, so an answer could overtake deliveries still queued for an address that had just been published. It now joins a draining serializer exactly as the live path does (#1145); the serializer's RouteOne, which discarded everything at DisposeHostedHubs, now carries a correlated answer to its live recipient at that run level. One shared method (DeliverReplyToLiveRecipient) serves both sites. - Registered-stream recipients. A recipient registered with IRoutingService.RegisterStream can exist with no hosted hub (portal and session hubs do). New protected hook TryDeliverToRegisteredStream, implemented by MonolithRoutingService, consulted after the hosted-hub lookup — the live path's order. - The negative control can no longer pass on a throwing router: route errors are recorded and asserted empty in every fact. New fact: AReplyToARegisteredStreamRecipient_ReachesItDuringDisposeHostedHubs, which also asserts its own precondition (no hosted hub). Falsified locally: the unfixed router fails both answer facts; an over-broad fix fails the uncorrelated control. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4023.
Summary
NackReachesTheWaiterDuringTeardownTestfailed in CI twice (core run 34513634943 shard 4; Plugins run 34581527040 shard 2) naming the #2778 defect. It was not #2778. The answer the caller was waiting for was dropped by the mesh's router:RoutingServiceBase.RouteInMeshthrew away every delivery once the mesh reachedDisposeHostedHubs(no NACK, no log,Forwardedreturned) because "recipients are likely also disposing". That is the #2778 assumption ("nobody is waiting") at its third site, after the owner's disposal NACK (#2868) andHierarchicalRouting's route-up refusal (#3303).Why #2778's fix "stopped holding": it did not — the test moved off it
Dispatchnever failed. Every iteration in which the ShutDown-phase registrant claimed the ack gate dispatched and the watch was consumed (41 of 200 unstressed, 42 of 400 stressed).f92305ae1, 2026-09-04) moved the test onto another path. The parked merge turn releases onowner.IsShuttingDown, which flips at the first instant ofMesh.Dispose()while the owner is stillStarted. The merge then COMMITS, the live ack claims the gate, and the registrant stands down withclaimed=False. In every failing iteration noOwnerDisposingNACK was minted, so the assertion's premise was false every time it failed.Error during shutdown, the registrant is on every failing trail atrunLevel=ShutDown), but it proves nothing about a NACK: readclaimed=.How the answer reached the armed watch, over 400 stressed iterations: 347 via 97a0284's route-up sink, 42 via #2868's registrant, 10 via #3196's refused-post fallthrough, 1 FAIL on the ordinary route (owner → mesh → caller's hub).
The failing trail (two identical captures, i=335 and i=927)
At the owner's
Dead: mesh atDisposeHostedHubs;cache/…Quiescing, queue empty,PendingCallbacks=1[PatchDataRequest@…]. The watch was still armed 5,001 ms later: the caller's 2 s response wait expires before the cache hub's quiesce cancel and deliberately leaves the watch armed, so the writer reportsOwnerUnreachableafter 31 s for an applied write. Remaining teardown: 1,849 ms locally, 1,948/1,949 ms in both CI failures, against a passing maximum of 69–107 ms.The fix
RouteReplyDuringMeshTeardown, scoped exactly as #3303 scoped its seam. AtDisposeHostedHubsa delivery carryingPostOptions.RequestId(an answer) is still delivered to a recipient that already exists, in the live path's order: a hosted hub (HostedHubCreation.Never; nothing is built during teardown), then a recipient registered with the router as a stream (TryDeliverToRegisteredStream, implemented byMonolithRoutingService; portal and session hubs register this way). Ordering is the live path's: while an activation serializer for the address is draining, the answer joins its queue, and the serializer'sRouteOnecarries it on at that run level instead of dropping it. Everything else keeps the historical drop. The recipient's own intake gate still decides admission. Monolith/CI only:RoutingServiceBasehas exactly one subclass (MonolithRoutingService), andOrleansRoutingServicehas no such guard.Controls (
ReplyRoutedDuringMeshTeardownReachesItsWaiterTest), each falsified locallyThe second commit addresses the Copilot review: FIFO parity with the activation serializer, registered-stream recipients (a third fact), and route errors are now asserted rather than only logged.
State is held open by construction (a parked recipient turn; fences on run levels) and judged at the mesh's
Dead, not on a clock.Evidence
Before: 4 failures in 3,600 stressed iterations (
DOTNET_PROCESSOR_COUNT=2+ 18 spinning processes), all consistent with this drop; the two fully instrumented captures show it directly. After: see the PR comments (stressed run and suite timing base vs branch).Still open (not in this change)
Judging the ordinary route at the owner's
Deadis a race: once, the reply was in the caller's hub queue and delivered 12 ms after the assertion (1 in 3,600 stressed, never in CI). Closing it changes what BOTH twins assert (break shape 7, Plugins pin bump), so it is left out here and documented (TeardownVerdictsAreCausalcorrection).Docs
DisposalStallVerdicts→ "Resolved 2026-09-11";TeardownVerdictsAreCausalcorrection; What's New (Category: Fix).No public surface removed or added (one private method). No i18n keys.
🤖 Generated with Claude Code