Repository navigation
fix(#2480): the teardown join's EXPIRY report now names the pool and the leaf - #4921
Conversation
…the leaf `IoPoolSiloTeardown` reports at Error when pooled I/O does not unwind inside the 30 s join budget, and that line is the only attribution a subsequent native use-after-unload crash will ever get. It ended with an instruction — "fix the leaf, do not widen the budget" — and named no leaf, no pool, no site. All 18 production occurrences over three weeks on 16 pods are that sentence, character for character. ROOT CAUSE: the attribution was subscribed to the SUCCESS signal. The per-pool residual warning added for #2480 rides each pool's own `Disposed`, and `IoPool.TryFinishDisposal`'s own remarks say a pool whose leaf never unwinds never fires it. So the one path this fault takes was the one path that could not report. `IoPoolRegistry.Dispose` also clears `_pools` on the way in — correctly, so a pool handed out after disposal can never be issued work nobody will join — which removed the caller's last way to enumerate what it was waiting for. The fix reads the state DIRECTLY at expiry instead of waiting to be told: `IoPoolRegistry.UnreportedResiduals()` subtracts the pools that did report from the set the disposal captured, and each remaining one carries its in-flight count and its pending leaf call sites. A lambda's compiler-generated name carries its enclosing method, so the line now points at the operation that did not unwind. The empty case is spelled as the different, sharper finding it is: budget expired with nothing outstanding is a defect in the registry's own Zip, not in a leaf. This is the shape three of the four teardown budget reports already had — `MeshTeardownHostedService` and `MeshTeardownExtensions` attach `GetDisposalDiagnostics()`, and `RoutingQuiescenceSiloParticipant` names its stuck legs with target and delivery id. #2833 filed the identical ask on the same day ("the same ask #2480 made for the pool name"), answered it by reading its registry at expiry, and production shows bare lines until 2026-09-02 and named legs from then on. IoPool was the only outlier. `IoPoolOptions.SiloJoinBudget` makes the budget settable, DEFAULT UNCHANGED at 30 s, for one reason: at 30 s no test can observe this report at all under `test/xunit.runner.json`'s 30 s `methodTimeout`, which is how its wording went three weeks unread. No bound is widened anywhere. Control: `WhenTheJoinBudgetExpires_TheReportNamesThePoolAndTheLeaf` parks a leaf that genuinely ignores its token. Against the pre-fix wording it fails on the verbatim production sentence ("...do not widen the budget." — no pool, no site); with the fix it passes, 4/4 in the file. Findings conserved as Shape 4 in Doc/Architecture/AFailureReportAnswersItsOwnInstruction, with the four-budget sweep table and a new checklist item: which signal is the report subscribed to? Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
IoPoolRegistry.cs has an XML doc comment structure issue (duplicate <summary>/misattached docs) that should be corrected before merge.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
What changed in this PR
Improves shutdown-time diagnostics for pooled I/O teardown (#2480) by capturing and reporting which IoPool(s) still have in-flight leaves when the join budget expires, and documenting the failure-reporting pattern in the architecture docs.
Changes:
- Add
IoPoolOptions.SiloJoinBudget(default 30s unchanged) and surface it viaIoPoolRegistry.SiloJoinBudgetso tests can exercise the timeout-path report. - Preserve the set of pools being drained and expose
IoPoolRegistry.UnreportedResiduals()for direct “what’s still stuck” reporting on timeout. - Add a regression test that forces a join-budget expiry and asserts the error line names both pool and leaf site; add a What’s New entry + extend the “failure report” architecture note.
| File | Description |
|---|---|
| test/MeshWeaver.Hosting.Orleans.Test/IoPoolSiloTeardownTest.cs | Adds a regression test that forces join-budget expiry and asserts the report names pool + leaf site. |
| src/MeshWeaver.Mesh.Contract/Threading/IoPoolRegistry.cs | Captures draining pools, tracks which pools reported, and exposes UnreportedResiduals() + SiloJoinBudget. |
| src/MeshWeaver.Mesh.Contract/Threading/IoPoolOptions.cs | Introduces configurable SiloJoinBudget with default unchanged. |
| src/MeshWeaver.Connection.Orleans/IoPoolSiloTeardown.cs | Uses the registry-provided budget and reports unreported residuals on timeout. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-19-shutdown-now-names-the-work-it-could-not-wait-for.md | Documents the user-visible operational improvement in shutdown diagnostics. |
| src/MeshWeaver.Documentation/Data/Architecture/AFailureReportAnswersItsOwnInstruction.md | Extends the architecture rule with the new “success-signal attribution” failure shape. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| /// <summary> | ||
| /// The budget a host's terminal drain holds shutdown for while waiting on <see cref="Disposed"/> | ||
| /// — see <see cref="IoPoolOptions.SiloJoinBudget"/>. Read from the registry so the waiter needs | ||
| /// no DI resolution of its own: <c>IoPoolSiloTeardown</c> captures this registry while the | ||
| /// container is provably alive and its stop must resolve NOTHING (#1898/#1899). | ||
| /// </summary> | ||
| public TimeSpan SiloJoinBudget => _options.SiloJoinBudget; | ||
|
|
||
| public IReadOnlyList<PoolResidual> UnreportedResiduals() => | ||
| _draining | ||
| .Where(kvp => !_reported.ContainsKey(kvp.Key)) | ||
| .Select(kvp => new PoolResidual(kvp.Key, kvp.Value.CurrentInFlight) | ||
| { | ||
| Sites = kvp.Value.PendingLeafSites | ||
| }) | ||
| .ToArray(); |
There was a problem hiding this comment.
Correct, and thank you — this is a real defect, not a style point. I added SiloJoinBudget immediately after the UnreportedResiduals doc block and before the method, so the long block now attaches to the property and UnreportedResiduals() — the whole point of the change — is left undocumented, with two <summary> elements in one comment.
Verified in the file before fixing: line 317 closes the first </summary>, line 318 opens a second one, and the member that follows at 324 is SiloJoinBudget, with UnreportedResiduals() bare at 326.
Fixed by moving SiloJoinBudget and its own doc block ABOVE the UnreportedResiduals block, so each <summary> sits directly on the member it describes and neither member is undocumented. Pushing that now.
Test Results (shard 0)266 tests 266 ✅ 2m 43s ⏱️ Results for commit 6773fc9. ♻️ This comment has been updated with latest results. |
Test Results (shard 3)429 tests 429 ✅ 1m 1s ⏱️ Results for commit 6773fc9. ♻️ This comment has been updated with latest results. |
Test Results (shard 1)533 tests 533 ✅ 59s ⏱️ Results for commit 6773fc9. ♻️ This comment has been updated with latest results. |
Test Results (shard 4) 2 files 2 suites 5m 26s ⏱️ Results for commit 6773fc9. ♻️ This comment has been updated with latest results. |
Copilot was right, and it is a real defect rather than a style point: SiloJoinBudget was inserted between the UnreportedResiduals doc block and the method, so the long block attached to the property and UnreportedResiduals() - the whole point of the change - was left undocumented, with two <summary> elements in one comment. SiloJoinBudget and its own block now sit ABOVE the UnreportedResiduals block, so each </summary> is immediately followed by the member it describes (302->303 and 325->326) and neither member is bare. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Test Results (shard 2)1 503 tests 1 503 ✅ 7m 31s ⏱️ Results for commit 6773fc9. |
Test Results (shard 5) 4 files 4 suites 12m 26s ⏱️ Results for commit 6773fc9. |
Test Results 12 files 12 suites 30m 7s ⏱️ Results for commit 6773fc9. |
What was wrong
IoPoolSiloTeardownreports at Error when pooled I/O does not unwind inside the 30 s joinbudget. That line is the only attribution a subsequent native use-after-unload crash will ever get,
and it ends by telling the reader to "fix the leaf, do not widen the budget" — while naming no
leaf, no pool and no call site. All 18 production occurrences of #2480, over three weeks on 16
pods, are that sentence character for character.
Root cause: the attribution was subscribed to the SUCCESS signal
The per-pool residual warning that was added for #2480 is real, correct and tested — and wired to
a signal the failing case never publishes:
IoPool.TryFinishDisposal's own remarks say it: "If a leaf never unwinds,Disposedsimply neverfires and the caller's bounded wait surfaces that as the timeout it is." So the one path this fault
takes is the one path with no attribution.
IoPoolRegistry.Disposealso clears_poolson the wayin — correctly, so a pool handed out after disposal can never be issued work nobody will join —
which removed the caller's last way to enumerate what it was waiting for.
This is the report-shaped form of #4466's ordering rule. Turned around: a report about a failure
must not be subscribed to a signal that only success publishes.
The fix
Read the state directly at expiry instead of waiting to be told.
IoPoolRegistry.UnreportedResiduals()subtracts the pools that did report from the set the disposalcaptured, and each remaining one carries its in-flight count and its pending leaf sites:
The empty case is spelled as the different, sharper finding it is — budget expired with nothing
outstanding is a defect in the registry's own
Zip, not in a leaf — rather than rendering as theabsence of a finding.
No bound is widened anywhere.
IoPoolOptions.SiloJoinBudgetmakes the budget settable with thedefault unchanged at 30 s, for one reason: at 30 s no test can observe this report at all under
test/xunit.runner.json's 30 smethodTimeout, which is exactly how its wording went three weeksunread. Same precedent as the existing
DrainTimeout/DrainGrace.This is the shape three of the four teardown budgets already had
MeshTeardownHostedServicemesh.GetDisposalDiagnostics()MeshTeardownExtensions.TeardownAsyncTimeoutExceptionRoutingQuiescenceSiloParticipantInFlightSample()— stuck legs, target + delivery idIoPoolSiloTeardown#2833 filed the identical ask on the same day ("the same ask #2480 made for the pool name"),
answered it by reading its registry at expiry, and production shows the difference: bare lines until
2026-09-02, named legs from then on. IoPool was the only outlier — this makes it match the sibling.
Control
WhenTheJoinBudgetExpires_TheReportNamesThePoolAndTheLeafparks a leaf that genuinely ignores itsCancellationToken. Falsified both ways:Expected "IoPoolSiloTeardown: pooled I/O did not finish within 00:00:00.3000000 — … do not widen the budget." to contain "Query"IoPoolSiloTeardownTest, and 68/68 IoPool tests inMeshWeaver.Hosting.Teststill green (the change touchesIoPoolRegistry.Dispose).Builds:
MeshWeaver.Mesh.Contract,MeshWeaver.Connection.Orleans,MeshWeaver.Documentationandboth touched test projects, each
-c Release -warnaserror, one project per invocation, all0 Warning(s) / 0 Error(s). Doc guards (link integrity, front matter, What's New) 4/4.Issues
Fixes the diagnosability half of #2480 — not the leaf. The leaf is still unidentified, which is
precisely what this change makes possible: the next occurrence names it. #2480 therefore stays open
on that closing condition rather than being closed here.
Addresses feedback
No review feedback yet — this is the first push.
Pairs-with: none — no public type or member is removed; this adds
IoPoolRegistry.UnreportedResiduals(),IoPoolRegistry.SiloJoinBudgetandIoPoolOptions.SiloJoinBudget, and renames one private constant.Implementers: none — no interface or abstract member is added.
Mirror-sync: none — no i18n catalog key is added or re-worded (the changed strings are operator log
lines, which are not viewer-facing catalog text).
🤖 Generated with Claude Code