Repository navigation
Disposal now KNOWS when it finishes: a terminal signal, and drains that cannot lie - #888
Merged
Merged
Conversation
…at cannot lie
The FutuRe.Test teardown SIGSEGV (dump dotnet-3029, shard 2 of run 31151666185)
is the shape this closes: DISPOSE_DONE traced after a 2004ms dispose, the next
test's INIT starts 8ms later, and the process dies in libc via libcoreclr with
si_addr=0 and no DAC loaded -- a ThreadPool leaf dereferencing a collectible
node ALC's freed metadata after the scope disposed and unloaded it. "Disposal
completed" had been signalled; it was not true.
Three defects let it lie, each fixed at its root:
1. ImportManager ran its async leaves on IoPool.Unbounded UNCONDITIONALLY.
Unbounded work is invisible to teardown by construction: CurrentInFlight is
hard-wired 0 and there is no cancellation source, so DrainAll() can neither
see nor stop it. An import still running there survives every drain phase --
and FutuRe is import-heavy. It now bridges through the mesh-scoped
FileSystem pool (registry-resolved, Unbounded only outside DI where no ALC
unload follows).
2. The drains gave up SILENTLY. IoPool.Drain() broke out of its join loop on
timeout and returned void; AsyncDisposeQueue.DrainAsync swallowed both of
its timeouts. Callers could not distinguish "everything unwound" from "live
work survives, proceed at your peril". Drain()/DrainAll() now return the
leaked-leaf count and DrainAsync returns clean/dirty -- the join is either
real or REPORTED.
3. There was no terminal signal. Every scenario re-derived "done" from partial
signals -- the Fixture HubTestBase awaited DisposalCompleted ALONE (no pool
drain, no queue drain at all). New MeshTeardownSignal (mesh-scoped singleton,
ReplaySubject(1) + CAS exactly like MessageHub.disposalCompleted): fired
exactly once by the teardown orchestrator after EVERY drain phase, carrying
a TeardownReport of what survived. Late subscribers replay -- nothing that
orders on "all is done" can miss it.
Every scenario now runs the same orchestrated teardown and surfaces the report:
- MeshTeardownExtensions.TeardownAsync / WaitForDisposalAndIoDrainAsync
build the report and fire the signal (phase 4).
- MonolithMeshTestBase FAILS the test class on a dirty report (same idiom as
the quiesce-leak failure; aggregated, never clobbered) -- the straggler is
named while the evidence exists, instead of a SIGSEGV 8ms into the next
test that nothing can attribute.
- HubTestBase switches from DisposalCompleted-only to the shared drain and
throws on dirty.
- MeshTeardownHostedService (prod monolith + Orleans silo shutdown) logs the
report, error-level when dirty.
- MessageHubGrain: the per-grain ALC Unload gap is documented honestly --
pools are mesh-shared so a grain cannot drain them; closing it needs
per-ALC in-flight tracking (follow-up), while silo shutdown is covered.
No budget was widened and no retry added; a wedge still surfaces at the same
bounds -- it now surfaces WITH ITS NAME.
Tests: AsyncDisposeQueueTest extended for the clean/dirty contract (including
a cleanup that ignores cancellation), new MeshTeardownSignalTest pins replay +
first-report-wins + report truth. Hosting.Test 8/8, Import.Test 21/21,
FutuRe.Test green locally; all touched projects build Release -warnaserror.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hx6BiUYBu5U2Zdnom4Uz4X
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens mesh teardown correctness by making “teardown finished” a verifiable, terminal contract: drains now report whether work actually stopped, and a mesh-scoped terminal signal replays the final teardown report to any late subscribers (preventing teardown stragglers from silently running into scope/ALC unload).
Changes:
- Introduces
TeardownReport+MeshTeardownSignalas a terminal, replayed teardown completion signal. - Makes pool/cleanup drains reportable (
IoPool.Drain()/IoPoolRegistry.DrainAll()leak counts;AsyncDisposeQueue.DrainAsync()returns clean/dirty) and propagates the report through teardown orchestrators. - Routes import I/O leaves through the mesh-scoped file-system pool (instead of
IoPool.Unbounded) so teardown can cancel/join them.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Hosting.Test/MeshTeardownSignalTest.cs | Adds tests pinning the replay + idempotence contract of the new terminal teardown signal/report. |
| test/MeshWeaver.Hosting.Test/AsyncDisposeQueueTest.cs | Updates drain tests for the new bool drain outcome and adds a “dirty when cancellation ignored” case. |
| src/MeshWeaver.Mesh.Contract/Threading/MeshTeardownSignal.cs | Adds TeardownReport and MeshTeardownSignal (replayed terminal teardown report). |
| src/MeshWeaver.Mesh.Contract/Threading/IoPoolServiceCollectionExtensions.cs | Registers MeshTeardownSignal as a mesh-scoped singleton. |
| src/MeshWeaver.Mesh.Contract/Threading/IoPoolRegistry.cs | Changes DrainAll() to return total leaked-leaf count across pools. |
| src/MeshWeaver.Mesh.Contract/Threading/IoPool.cs | Changes Drain() to return leaked-leaf count (join that can’t lie). |
| src/MeshWeaver.Mesh.Contract/Threading/AsyncDisposeQueue.cs | Changes DrainAsync() to return clean/dirty depending on whether cancellation was honored. |
| src/MeshWeaver.Mesh.Contract/MeshTeardownExtensions.cs | Returns TeardownReport from teardown orchestration and fires the terminal signal. |
| src/MeshWeaver.Import/Implementation/ImportManager.cs | Switches import leaves from IoPool.Unbounded to mesh file-system pool (teardown-visible). |
| src/MeshWeaver.Hosting/MeshTeardownHostedService.cs | Logs clean vs dirty teardown on host shutdown; error-logs dirty teardown. |
| src/MeshWeaver.Hosting.Orleans/MessageHubGrain.cs | Documents the known per-ALC drain gap for single-grain unload on live silo. |
| src/MeshWeaver.Hosting.Monolith.TestBase/MonolithMeshTestBase.cs | Produces and surfaces a dirty teardown report during test mesh disposal (fails class). |
| src/MeshWeaver.Fixture/HubTestBase.cs | Aligns fixture teardown with the shared drain+report contract and throws on dirty teardown. |
Suppressed comments (1)
src/MeshWeaver.Import/Implementation/ImportManager.cs:323
- This I/O leaf is now routed through the drainable pool, but the lambda still ignores the pool CancellationToken. At minimum, respect the pool token so teardown can prevent starting new work and can stop awaiting when draining.
// ── The ONLY async: open the stream. Deferred to the very end and
// bridged through the IIoPool; the Excel parse + EntityStore build
// that follow are SYNCHRONOUS (ConfiguredExcelImporter.Import returns
// IEnumerable), so they live in a plain Select. No async/await here. ──
return ioPool.Run(_ => streamProvider.Invoke(importRequest))
.Select(stream =>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
256
to
263
| // Read the data set (file/stream I/O leaf), then run the format import — | ||
| // composed reactively, each leaf bridged through the IIoPool once. | ||
| return IoPool.Unbounded | ||
| // composed reactively, each leaf bridged through the mesh-scoped IIoPool once | ||
| // (never Unbounded: teardown must be able to see, cancel and join these — see ioPool). | ||
| return ioPool | ||
| .Run(_ => ReadDataSetAsync(importRequest, activity, cancellationToken)) | ||
| .SelectMany(read => IoPool.Unbounded | ||
| .SelectMany(read => ioPool | ||
| .Run(_ => read.format.Import(importRequest, read.dataSet, activity, cancellationToken))) | ||
| .Select(imported => imported!); |
Comment on lines
+76
to
89
| var queue = new AsyncDisposeQueue(); | ||
| var entered = new TaskCompletionSource(); | ||
| var release = new TaskCompletionSource(); | ||
|
|
||
| // A cleanup that ignores ct entirely — the defect class the report exists to name. | ||
| queue.Enqueue(async _ => { entered.TrySetResult(); await release.Task; }); | ||
|
|
||
| await entered.Task.WaitAsync(Quiesce, TestContext.Current.CancellationToken); | ||
|
|
||
| var clean = await queue.DrainAsync(TimeSpan.FromMilliseconds(200)); | ||
|
|
||
| clean.Should().BeFalse("the cleanup ignored cancellation and is STILL RUNNING — the caller must surface it"); | ||
| release.SetResult(); // let the leaked cleanup finish so it cannot outlive the test | ||
| } |
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
This was referenced Aug 30, 2026
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.
Closes the teardown-straggler class behind the FutuRe.Test SIGSEGV (dump
dotnet-3029, shard 2 of run31151666185):DISPOSE_DONEtraced after a 2004 ms dispose, the next test's INIT starts 8 ms later, and the process dies inlibcvialibcoreclr(si_addr=0x0, DAC not loaded) — a ThreadPool leaf dereferencing a collectible node ALC's freed metadata after the scope disposed and unloaded it. "Disposal completed" had been signalled. It was not true.Three defects let it lie
ImportManagerran its async leaves onIoPool.Unboundedunconditionally —CurrentInFlighthard-wired 0, no cancellation source, invisible toDrainAll(). FutuRe is import-heavy.FileSystempool;Unboundedonly outside DI, where no ALC unload follows.IoPool.Drain()broke out of its join on timeout returningvoid;AsyncDisposeQueue.DrainAsyncswallowed both timeouts.Drain()/DrainAll()return the leaked-leaf count;DrainAsyncreturns clean/dirty. The join is either real or reported.HubTestBaseawaitedDisposalCompletedalone (no pool or queue drain at all).MeshTeardownSignal(mesh-scoped,ReplaySubject(1)+ CAS — the exactMessageHub.disposalCompletedidiom): fired exactly once after every drain phase, carrying aTeardownReport. Late subscribers replay.Every scenario, one contract
TeardownAsync/WaitForDisposalAndIoDrainAsyncbuild the report and fire the signal (phase 4)Unloadgap is documented honestly — pools are mesh-shared so a grain must not drain them; closing it needs per-ALC in-flight tracking (named follow-up). Silo shutdown is covered.No band-aid: no budget widened, no retry added — a wedge surfaces at the same bounds, now with its name.
Verification
MeshWeaver.FutuRe.Test(the reproducer)MeshWeaver.Import.TestMeshWeaver.Hosting.Test(3 new + 5 extended teardown tests)All touched projects build Release
-warnaserror.What's New skipped: internal teardown correctness, no user-visible change.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Hx6BiUYBu5U2Zdnom4Uz4X