Repository navigation
Unload node ALCs at the END of teardown, not before the I/O drain (#613) - #967
Merged
Merged
Conversation
The FutuRe.Test teardown crash (#613) was a phase inversion, not a leak or a re-entrant construction: node hubs unloaded their collectible AssemblyLoadContext from a SYNCHRONOUS hub-disposal callback, which MessageHub.DisposeImpl runs strictly BEFORE DisposalCompleted. But the pooled layout-render leaves that execute those compiled types are cancelled and joined only by IoPoolRegistry.DrainAll(), which every teardown orchestrator runs AFTER DisposalCompleted. In between, a leaf still inside IoPool.SubscribeThroughPool read a NodeType-compiled type's GC statics out of the just-freed LoaderAllocator -> AccessViolationException -> SIGABRT (exit 134). The deferral cannot be unconditional. In prod the mesh never tears down, so a node hub disposing on its own (node deleted, hub evicted or recycled) must still unload immediately, or the ALC leaks for the process lifetime -- which is the late-project CI OOM that put this hook in SubscribeToOwnDeletion to begin with. So the callback gates on IMessageHub.IsDisposing of the MESH hub, captured at buildup (Configuration.ParentHub re-resolves from a ParentServiceProvider that is routinely already torn down inside a disposal callback): - mesh alive -> unload NOW, exactly as before - mesh tearing down -> hand the unload to MeshTeardownSignal, which fires only after DrainAll + the async-dispose quiesce That is the role MeshTeardownSignal's own XML doc already assigns it ("unloading node ALCs ... subscribes here"). With no signal registered we keep the immediate unload rather than leak. NodeAlcUnloadTeardownOrderingTest is the proof, and needs no crash: it decorates ICompilationCacheService to stamp every UnloadNodeContexts call, parks a real pooled leaf whose cancellation callback stamps from INSIDE DrainAll, and drives the production teardown phases. On unmodified main all seven unloads land before drain-all:started; with this change every one lands after drain-all:returned -- same seven unloads, none skipped. A second test pins the other half of the gate: disposing only the node hub while the mesh keeps running must reclaim immediately (it times out if the deferral is made unconditional). NodeTypeAssemblyLeakTest was pinning the buggy ordering by waiting on DisposalCompleted; its wait moves to the terminal teardown signal. What it asserts -- zero surviving DynamicNode_* ALCs -- is unchanged. Refs #613 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a teardown ordering bug where collectible NodeType AssemblyLoadContexts could be unloaded before pooled I/O work is fully drained, causing process aborts (SIGSEGV/SIGABRT) during test/host shutdown.
Changes:
- Defer node ALC unloading to the mesh’s terminal
MeshTeardownSignalwhen (and only when) the mesh is tearing down; keep immediate unload when a node hub disposes while the mesh remains live. - Update
NodeTypeAssemblyLeakTestto assert after the terminal teardown signal (and validate a clean teardown report), aligning the test contract with the corrected teardown sequencing. - Add a deterministic ordering test (
NodeAlcUnloadTeardownOrderingTest) that pins “no unload beforeIoPoolRegistry.DrainAll()returns”.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/MeshWeaver.Graph/MeshDataSource.cs |
Gates node ALC unload: immediate for live-mesh node hub disposal; deferred to MeshTeardownSignal during mesh teardown. |
test/MeshWeaver.Hosting.Monolith.Test/NodeTypeAssemblyLeakTest.cs |
Waits for terminal teardown signal and asserts teardown cleanliness before checking for collected dynamic ALCs. |
test/MeshWeaver.Hosting.Monolith.Test/NodeAlcUnloadTeardownOrderingTest.cs |
Adds an ordering regression test to ensure unloads occur only after DrainAll() completes (and still occur immediately in the live-mesh case). |
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-08-clean-shutdown-for-compiled-node-types.md |
Documents the shutdown crash fix and the corrected teardown sequencing at a user-visible level. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Test Results (shard 0)685 tests 685 ✅ 4m 46s ⏱️ Results for commit 33c44e8. |
Contributor
Test Results (shard 1)1 191 tests 1 191 ✅ 5m 55s ⏱️ Results for commit 33c44e8. |
Contributor
Contributor
Contributor
Contributor
Test Results (shard 2)1 843 tests +6 1 841 ✅ +6 7m 44s ⏱️ -25s Results for commit 33c44e8. ± Comparison against base commit bfea7bd. This pull request removes 94 and adds 100 tests. Note that renamed tests count towards both. |
Contributor
This was referenced Aug 9, 2026
The FutuRe exit=139 is one background-GC fault, three times — and the dump reads fine on macOS
#1004
Merged
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 the teardown SIGSEGV/SIGABRT in
MeshWeaver.FutuRe.Test(#613).The bug: a teardown phase inversion
Node hubs unloaded their collectible
NodeAssemblyLoadContextfrom a synchronous hub-disposalcallback (
MeshDataSource.cs:879, insideSubscribeToOwnDeletion), whichMessageHub.DisposeImplruns strictly before
SignalDisposalCompleted(). But the pooled layout-render leaves thatexecute those compiled types are cancelled and joined only by
IoPoolRegistry.DrainAll(), whichevery teardown orchestrator runs after
DisposalCompleted(
MeshTeardownExtensions.WaitForDisposalAndIoDrainAsync,MonolithMeshTestBase,HubTestBase).In that window a leaf still inside
IoPool.SubscribeThroughPoolranWorkspace's.Cast<T>().ToArray()over a NodeType-compiledTand read its GC thread-static base out of thejust-freed LoaderAllocator →
AccessViolationException→ SIGABRT (exit 134).This violated the framework's own written contract:
MeshTeardownSignal's XML doc already names"unloading node ALCs" as one of the things that subscribes to the terminal signal.
The proof — deterministic, no crash needed
This is the durable artifact of the PR. The SIGSEGV is CI/load-dependent and did not reproduce
locally (3/3 clean runs on
main), so the bug is pinned as an ordering test instead:NodeAlcUnloadTeardownOrderingTestdecoratesICompilationCacheServiceto stamp everyUnloadNodeContextscall, parks a real pooled leaf whose cancellation callback stamps frominside
DrainAll(IoPool.Drain→_poolCts.Cancel()), compiles a real NodeType so a genuinecollectible ALC exists, and drives the production teardown phases.
Red on unmodified
main— all seven unloads land before the drain even starts:Green with this change — the same seven unloads, none skipped, all after the drain returned:
The assertion is "no unload may precede
DrainAllreturning", so it fails on the ordering itself,not on a timing threshold.
The fix, and why the deferral is CONDITIONAL
"Defer the unload past
DrainAll" is right at mesh teardown but cannot be unconditional: in prodthe mesh never tears down, so a node hub disposing on its own (node deleted, hub evicted, recycled)
must still unload immediately — otherwise the ALC leaks for the whole process lifetime, which is
precisely the late-project CI OOM / GC-stall that
MeshDataSource.cs:879was added to fix.Gated on
IMessageHub.IsDisposingof the MESH hub (hub.GetMeshHub()), captured at buildup —the existing idiom from
MonolithRoutingService.cs:73. Capture-at-construction is required becauseMessageHubConfiguration.ParentHubre-resolves from aParentServiceProviderthat is routinelyalready torn down by the time a disposal callback runs.
MeshTeardownSignal, which fires only afterDrainAll+ the async-dispose quiesceMeshTeardownSignalregisteredA second test,
NodeAssemblyContexts_AreUnloaded_Immediately_WhenTheMeshKeepsRunning, pins thatfirst row. It is not decorative: making the deferral unconditional makes it time out (verified by
temporarily mutating the gate), so the leak-preventing half of the branch cannot silently regress.
NodeTypeAssemblyLeakTestwas pinning the buggy ordering — it waited onDisposalCompletedandasserted zero surviving ALCs, i.e. it required the unload to have happened before
DrainAll. Itswait moves to the terminal teardown signal. What it asserts is unchanged — still zero surviving
DynamicNode_*ALCs, plus a newreport.Cleancheck; only when it checks moved.Verification
All on a tree merged with current
main(b65c831b9), built with CI's flags(
dotnet build -c Release -warnaserror, clean forMeshWeaver.Graph,MeshWeaver.Hosting.Monolith.Test,MeshWeaver.FutuRe.Test,MeshWeaver.Hosting.Test,MeshWeaver.Graph.Test):NodeAlcUnloadTeardownOrderingTest+NodeTypeAssemblyLeakTestmainbefore the fix)MeshWeaver.FutuRe.Test(the project #613 was filed against)MeshWeaver.Hosting.Test(teardown-ordering guards)MeshWeaver.Graph.TestALC-leak + compilation-cacheCompileFinishAndDispose,ClrMdDacUnloadCrash,DisposalPendingSaveFlush,CodeEditRecompile,DynamicTypePreWarmer, …)Regression history checked and not re-broken: #635, #715, #774 (the
CloseCreation/ re-entrant-hubwork) —
FutuRe.Testis green with a zero exit code, which is the signal those fixes are measured on.One caveat, stated honestly: a full
MeshWeaver.Hosting.Monolith.Testrun was 492 passed / 1failed —
MeshNodeLanguageServiceTest.CheckSpeculative_AddsNuGetReferenceDirective_ResolvesAndCompileshung on resolving
nuget:Humanizer 2.14.1. That is network, not this change: the test's own commentnotes it relies on the package being warm in the NuGet cache, this worktree is fresh, and the network
on this machine was badly degraded at the time (a 149 MB build-time download was running at ~50 KB/s).
Its teardown reported clean, and it fails inside the test body, nowhere near the disposal path
this PR touches. CI, with a warm cache, is the real check.
Not proven: that the original SIGABRT is gone in CI. It never reproduced locally, so the evidence
here is the ordering contract, not the absence of the crash. If #613 recurs after this merges, the
phase inversion is no longer the explanation.
#618 — related, but needs its own fix
Assessed as asked; this PR does not fix #618 and does not claim to. #618 is a corrupt/evicted
DynamicNode_*assembly poisoning Orleans'ReferencedAssemblyProvideratTestCluster.DeployAsync.Reasoning:
UnloadNodeContextsruns during mesh teardown. The live-meshpath (node deleted / hub evicted) is deliberately unchanged, and
MessageHubGrain.OnDeactivateAsync— which does its own
loadContext.Unload()and already carries a documented KNOWN GAP for exactlythis — is untouched.
AssemblyLoadContext.Unload()is asynchronous by contract, so theAssemblystays reachable fromAppDomain.CurrentDomain.GetAssemblies()until the LoaderAllocator is actually collected — andOrleans' scan calls
assembly.IsDefined(...)on it with no pin.NodeAssemblyLoadContext.Pin()drain already inDispose()was added for this exact errorstring (
TypeLoadException … format is invalid) and does not cover Orleans' scan, because thatscan takes no pin. That gap is what Orleans test host: corrupt/evicted DynamicNode assembly poisons every TestCluster.DeployAsync (TypeLoadException 'format is invalid') — whole-project cascade #618 needs — either refuse to load a truncated image, or keep
dying assemblies out of the provider's enumerable set. Neither is an ordering fix.
Refs #613