Fix concurrent hub-construction SIGSEGV: evict Autofac reflection cache on collectible ALC unload#467
Open
rbuergi wants to merge 2 commits into
Open
Fix concurrent hub-construction SIGSEGV: evict Autofac reflection cache on collectible ALC unload#467rbuergi wants to merge 2 commits into
rbuergi wants to merge 2 commits into
Conversation
…he on collectible ALC unload Root cause: NodeAssemblyLoadContext (collectible, isCollectible:true) unloaded compiled-node assemblies without evicting Autofac's process-static shared reflection cache (ReflectionCacheSet.Shared). Autofac keys that cache on ConstructorInfo, so a retained key (1) rooted the collectible context — it could never be collected (a memory leak on every recompile) — and (2) let a later, unrelated CONCURRENT GetOrAdd during framework hub construction (AddMeshDataSource) compare against the freed metadata → AccessViolationException in ConcurrentDictionary.TryGetValueInternal. That is the native-host exit=139 the #447 exit-marker gate unmasked (FutuRe.Test / Hosting.Monolith.Test). Fix: NodeAssemblyLoadContext subscribes its Unloading event to ReflectionCacheEviction.EvictFor, which clears every shared-cache entry referencing the context's assemblies BEFORE unload (while the metadata the predicate walks is still valid). This mirrors exactly what Autofac performs automatically for BeginLoadContextLifetimeScope; MeshWeaver manages its collectible node contexts by hand, so it must run the eviction itself. No lock, no serialization, no convoy — the earlier global SetupModules lock was the wrong fix and is not used. Test: ReflectionCacheEvictionTest is a deterministic root-cause repro (not the flaky AVE) — a collectible NodeAssemblyLoadContext activated through Autofac must be GC-collectable after unload. It pins ReflectionCacheSet.Shared alive (it is WeakReference-backed, else a plain GC collects the whole cache and false-passes). RED pre-fix (context rooted), GREEN post-fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an intermittent native crash (SIGSEGV / exit 139) during concurrent hub construction by ensuring Autofac’s process-static reflection cache is purged of entries referencing assemblies in a collectible AssemblyLoadContext before the context unloads, preventing both ALC pinning (leak) and stale-metadata key comparisons.
Changes:
- Add
ReflectionCacheEviction.EvictFor(AssemblyLoadContext)to clear Autofac shared reflection-cache entries tied to a soon-to-unload collectible ALC. - Hook
NodeAssemblyLoadContext.Unloadingto run the eviction during unload initiation. - Add a deterministic regression test asserting the collectible node ALC is GC-collectable after unload following Autofac activation, plus a “What’s New” entry.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Graph.Test/ReflectionCacheEvictionTest.cs | New deterministic repro/regression test for ALC collection after Autofac activation. |
| test/MeshWeaver.Graph.Test/MeshWeaver.Graph.Test.csproj | Adds Autofac package reference needed for the new test. |
| src/MeshWeaver.ServiceProvider/ReflectionCacheEviction.cs | New helper to evict Autofac shared reflection cache entries for a collectible ALC. |
| src/MeshWeaver.Graph/Configuration/CompilationCacheService.cs | Subscribes NodeAssemblyLoadContext.Unloading to evict Autofac cache prior to unload. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-07-14-node-recompile-stability.md | Documents improved node recompile stability and reduced leak/crash risk. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+37
to
+41
| for (var i = 0; i < 15 && weak.IsAlive; i++) | ||
| { | ||
| GC.Collect(); | ||
| GC.WaitForPendingFinalizers(); | ||
| } |
Contributor
Author
|
Status from a verification pass (driving this via /pullrequest): leak is fixed, but the SIGSEGV persists — not merging.
Reading: consistent with the investigation's note that beyond the ALC keys a genuine Autofac concurrency bug / Reflection.Emit factory stayed un-isolated — so there's a second contributor, a residual eviction window, or (given the ~50% qemu intermittency) the fix reduces-but-doesn't-eliminate. One red run can't distinguish those, but it's definitive the crash can still fire with this fix. Not merged (green-main gate). Keeping this branch — the leak fix is worth landing once the residual crash cause is also closed. Confirmed independently: crash point is disconnected from any active construction (dump: all threads idle at crash), so it's use-after-free detonating on a later read — dump can't isolate the corrupting op further. |
The exit=139 crash in the ALC-heavy native test hosts (Hosting.Monolith.Test / FutuRe.Test) is a NATIVE use-after-unload during teardown: a process-wide reflection/serialization cache retains an accessor into a collectible node AssemblyLoadContext; when a node-hub disposes it unloads that ALC, and a later background GC dereferences the freed metadata -> SIGSEGV. The post-crash minidump is a delayed-detection snapshot (the ALC already unloaded, nothing to gcroot), so it can't name the offending cache. This is the known .NET collectible-ALC hazard class (dotnet/runtime #1388, #13283) -- the same SHAPE as the Autofac reflection cache this branch already evicts, but a DIFFERENT cache (that fix is orthogonal to the segfault). Diagnostic (NOT a fix), gated OFF by default so zero prod/CI cost: MESHWEAVER_ALC_UNLOAD_GC_PROBE=1 makes NodeAssemblyLoadContext.Dispose drive a synchronous full GC immediately after Unload, logging the node name first. A dangling native pointer then faults AT that unload -- naming the culprit node and yielding a corruption-time managed-stack dump -- instead of the opaque delayed crash. GCStress is the classic tool but needs a checked runtime the hosted runner lacks; forcing the collection is the retail-runtime equivalent for use-after-unload. Workflow .github/workflows/alc-unload-probe.yml (workflow_dispatch) loops the native host under the probe, breaks on a signal exit (139/134/132/136), greps the last ALC_UNLOAD_PROBE line = culprit node, and uploads the dump + logs. Run it to name the cache, then apply the evict-on-unload pattern to it. Co-Authored-By: Claude Opus 4.8 (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.
Root cause
The intermittent native-host SIGSEGV (exit 139) in concurrent per-node hub construction — the crash the #447 exit-marker gate unmasked in
FutuRe.Test/Hosting.Monolith.Test— was not a lock/serialization problem. (The earlier globalSetupModuleslock was the wrong fix and is not used.)NodeAssemblyLoadContextcompiles nodes into collectible load contexts (isCollectible: true) and unloads them on recompile / release / teardown. ItsDispose()calledUnload()but never evicted Autofac's process-static shared reflection cache (ReflectionCacheSet.Shared). Autofac keys that cache onConstructorInfo, so after unload a retained key:GetOrAddduring framework hub construction (AddMeshDataSource) whose key hashes into a bucket holding the stale key compare against freed metadata →AccessViolationExceptioninConcurrentDictionary.TryGetValueInternal(the observed crash frame).Autofac performs exactly this eviction automatically for scopes created via
BeginLoadContextLifetimeScope; MeshWeaver manages its collectible node contexts by hand, so it must run the eviction itself.Fix
MeshWeaver.ServiceProvider/ReflectionCacheEviction.EvictFor(AssemblyLoadContext)— clears every shared-cache entry referencing the context's assemblies.NodeAssemblyLoadContextsubscribesUnloading += ReflectionCacheEviction.EvictFor— runs before unload, while the metadata the predicate walks is still valid; static handler ⇒ no self-reference that would defeat collection. No lock, no serialization, no convoy. (Graph reaches ServiceProvider transitively; no new project reference.)Test (deterministic, not the flaky AVE)
ReflectionCacheEvictionTestis a root-cause repro: a collectibleNodeAssemblyLoadContextactivated through Autofac must be GC-collectable after unload. It pinsReflectionCacheSet.Sharedalive (the cache isWeakReference-backed — otherwise a plain GC collects the whole cache and false-passes). RED pre-fix (context rooted), GREEN post-fix — a permanent regression guard, unlike the ~50% qemu-amd64 crash.Validation
Release -warnaserror: Graph.Test and Hosting.Monolith.Test (the crash site) both build with 0 warnings / 0 errors.🤖 Generated with Claude Code