Skip to content

A scan that lost its load context bound the hub to nothing - #1165

Merged
rbuergi merged 3 commits into
mainfrom
fix/1151-alc-scan-pin-supersession
Aug 10, 2026
Merged

rbuergi merged 3 commits into
mainfrom
fix/1151-alc-scan-pin-supersession

Conversation

@rbuergi

@rbuergi rbuergi commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Not a fix for the main red — that is gone (#1153). This is a third, independent
failure mode on the same StaleStampRootBindingTest cluster, captured on CI and fixed at
its root. It is orthogonal to both #1153 (installer teardown outliving the install) and
#1163 (a raced subscribe parking on an un-re-probed ShuttingDown NACK).

The evidence

CI run 31404522444,
shard 4, commit 169672e1. The test does not time out — it completes in 831 ms and
fails the content assertion:

[type +609ms] v6  Status=Ok       asm='ShopA_Front/v3-93d7ee2d-694c899111a5.dll'
rebuild settled at +609ms
[type +642ms] v7  Status=Pending
[type +643ms] v9  Status=Compiling
[Warning] Failed to extract NodeTypeConfigurations from …/v3-93d7ee2d-694c899111a5.dll:
    Cannot scan an assembly whose context is unloading
    Object name: 'DynamicNode_ShopA_Front'
    at NodeAssemblyLoadContext.Pin()  … at MeshNodeCompilationService.CompileResultFromAssembly
[type +697ms] v10 Status=Ok       asm='ShopA_Front/v8-93d7ee2d-694c899111a5.dll'
Tests frame at +831ms:  … Area not found
    at StaleStampRootBindingTest.cs(319)  — NotContain("Area not found")

Note the two assembly names: v3-93d7ee2d-694c899111a5 and v8-93d7ee2d-694c899111a5 —
same framework hash, same source hash. A byte-identical recompile is what evicted the
load context out from under a concurrent reader.

The defect

A per-instance hub activation hydrates its NodeType's HubConfiguration by reflecting over
the compiled assembly (GetConfigurationsFromExistingAssembly →
CompileResultFromAssembly). Both scan sites resolved the load context and pinned it as
two statements:

var context = cacheService.GetOrCreateLoadContextForPath(nodeName, assemblyLocation);
using var scanPin = context.Pin();

A recompile landing in between registers its own path-keyed context, and
EvictSupersededContexts → UnloadContext disposes every superseded one — including the
reference the reader had just resolved. The pin then throws on a context that was live one
instruction earlier.

Pin() documents exactly what to do about that, in its own XML doc:

Throws ObjectDisposedException if the context is already unloading — the caller must
then re-resolve against the current context
rather than scan a doomed assembly.

Neither caller implemented it. The catch below recorded an empty configuration list
instead, and the activation path reads that as authoritative: no matching configuration →
ApplyEntry(…, hubConfig: null) → the hub binds the mesh defaults. A hub resolves its
configuration exactly once, at activation, so the freshly installed root then serves
"Area not found" for its whole lifetime — from a rebuild it merely ran next to. The
neighbouring branch in NodeTypeEnrichmentHelpers already calls this outcome out by name as
"the silent default-config fallback … cached for the grain's lifetime".

The fix

Resolve and pin become one operation, ICompilationCacheService.PinForScan. The cache
service owns it because it owns _loadContexts — it is the only component that can
re-resolve. Both scan sites go through it, so two readers racing each other's evictions
both converge instead of both failing.

It terminates rather than spins: the evictor removes the key before it disposes, so the
next resolve constructs a fresh context; the defensive TryRemove covers the remove-less
ordering so the loop can never be handed the same doomed context twice. Exhaustion rethrows,
so a genuinely dying cache still surfaces.

Verification

  • ScanPinSupersessionTest (new, deterministic, dependency-free) pins three facts: a
    recompile really does doom a concurrent reader's already-resolved context (the
    precondition, asserted so the fix is pinned against a race that happens); PinForScan
    survives it; and a disposed context left in the cache is replaced rather than returned.
    3/3 in 346 ms.

  • dotnet build -c Release -warnaserror clean for MeshWeaver.Graph + MeshWeaver.Graph.Test.

  • MeshWeaver.PluginCatalog.Test, DOTNET_PROCESSOR_COUNT=4, Release — reported in full because
    one run was red and the number matters:

    tree result
    A stray recycle killed the hub the install was still using #1159's tree + this fix 236/236, 1 m 21 s
    origin/main (0e4cc93) + this fix 235/1, 2 m 1 s
    origin/main (0e4cc93) alone 236/236, 1 m 22 s

    The red is SelfTypedRootWhoseTypeNeverCompiles_SkipsThePublishPromptly — a different test
    from the one this PR is about — and it is the pre-existing installer flake, not this change:

    • This change is not on that test's code path. ShopC's type never compiles, so
      assemblyLocation is empty and CompileAndGetConfigurations takes the
      BuildFailureDiagnostics branch, never reaching CompileResultFromAssembly. Its
      HasUsableBuild is false (the committed stamp carries a stale framework hash), so the
      hydration path never calls GetConfigurationsFromExistingAssembly either, and nothing emits,
      so LoadAndExtractConfigurationsFromReleaseAsync never runs. Neither of the two call sites
      this PR touches executes for that test.
    • A recycled hub's subscriber now re-probes instead of parking two minutes #1163 independently measured this class at 2 of 5 class runs red on origin/main + #1153.
      A single green baseline run is exactly what a 3-in-5 green rate produces, so it exonerates
      nothing by itself — the mechanism argument above is the load-bearing one.
    • The failure's own shape puts it in the installer: the log goes silent from +2.3 s (the
      expected CS0103 compile failure for ShopC/Front) to +30.2 s, then an Rx
      TimeoutException: The operation has timed out. surfaces at Install. 30 s is not the test's
      own bound (StepTimeout is 120 s) — PackageInstaller has several uncaught
      .Timeout(TimeSpan.FromSeconds(30)) (lines 465, 561, 1544, 1680, 1702, 1965), one of which
      fired. Handed to the installer cluster rather than guessed at here.

Triage discriminator for the next red of this test

Three modes, told apart on sight:

Fails at Signature Owner
~60 s, on the install reply died with a hub torn down mid-handler #1153 (merged)
~120 s, on the frame wait TimeoutException, subscribe parked on an un-re-probed ShuttingDown NACK #1163
< 1 s, at StaleStampRootBindingTest.cs:319 NotContain("Area not found") + log carries Cannot scan an assembly whose context is unloading this PR

#1163's re-probe does not cover this one: a re-probe reactivates the owner, but if that
reactivation's configuration lookup hits an unloading context again it binds the defaults
again and answers promptly with the wrong content. If anything the re-probe schedules the
reactivation closer to the rebuild, so the two changes are complementary.

Named, deliberately deferred

  • EvictSupersededContexts ping-pong. Two readers on different assembly paths of the
    same NodeType each evict the other on registration. Bounded and convergent with this fix,
    but it is avoidable churn (an ALC re-created and re-loaded per hydration of a superseded
    path). Not touched here — narrowing it would only make the race rarer, which is not a fix.
  • The redundant recompile itself. The second compile had an identical framework hash
    and source hash; CompiledSources keys on LastModifiedTicks, not content. Skipping it
    would shrink this window, but again only rarer, not gone — a real source edit produces the
    same eviction.
  • The silent default-binding still has no self-heal. With this fix the scan no longer
    fails for supersession, but a genuine reflection failure (missing dependency) still lands
    on ApplyEntry(hubConfig: null) with no overlay and no WithOverlaySelfHeal, unlike its
    sibling byte-resolution branch — which the sibling's own comment says is mandatory-gated.
    Getting that version gate subtly wrong hot-loops a recycle, so it is not being done
    alongside a fix that already stands on its own.

🤖 Generated with Claude Code

rbuergi and others added 2 commits August 10, 2026 18:16
`StaleStampRootBindingTest` still reds shard 4 on the branch that removed the
installer's stray recycle — but with a DIFFERENT failure, and the trace on
169672e says so on sight: the test now runs in 831 ms and fails the render
assertion, not on a 60 s timeout.

    [type +609ms] v6  Status=Ok       asm='ShopA_Front/v3-93d7ee2d-694c899111a5.dll'
    [type +642ms] v7  Status=Pending
    [type +643ms] v9  Status=Compiling
    Failed to extract NodeTypeConfigurations from …/v3-93d7ee2d-694c899111a5.dll:
        Cannot scan an assembly whose context is unloading
        at NodeAssemblyLoadContext.Pin() … at CompileResultFromAssembly
    [type +697ms] v10 Status=Ok       asm='ShopA_Front/v8-93d7ee2d-694c899111a5.dll'
    Tests frame at +831ms: … Area not found

A second compile (byte-identical inputs — same framework hash, same source
hash) registered its own path-keyed load context. That eviction disposes every
superseded context for the NodeType, including the one the root hub's
activation had ALREADY resolved and was about to scan. Its pin then threw on a
reference that was live one instruction earlier.

The scan sites resolved and pinned in two statements:

    var context = cacheService.GetOrCreateLoadContextForPath(nodeName, assemblyLocation);
    using var scanPin = context.Pin();

`Pin()` documents the answer — "the caller must then re-resolve against the
current context rather than scan a doomed assembly" — and neither caller
implemented it. The catch below recorded an EMPTY configuration list instead,
which the activation path reads as authoritative: no matching configuration →
the hub binds the mesh defaults. A hub resolves its configuration exactly once,
at activation, so the freshly installed root then serves "Area not found" for
its whole lifetime, from a recompile it merely ran next to.

Resolve and pin are now ONE operation. `ICompilationCacheService.PinForScan`
owns the loop because it owns the dictionary — it is the only component that
can re-resolve. It terminates because the evictor removes the key before it
disposes, so the next resolve constructs a fresh context; the defensive
TryRemove covers the remove-less ordering so the loop can never be handed the
same doomed context twice. Both scan sites go through it, so the two readers
racing each other's evictions both converge instead of both failing.

`ScanPinSupersessionTest` pins all three facts deterministically: that a
recompile really does doom a concurrent reader's already-resolved context, that
PinForScan survives it, and that a disposed context left in the cache is
replaced rather than returned.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 10, 2026 16:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a concurrency race in dynamic NodeType assembly scanning where a scan could resolve an AssemblyLoadContext, have it evicted/unloaded by a concurrent recompile, and then fail to pin for reflection—silently producing an empty configuration list and causing a per-instance hub to bind default configuration for its entire activation lifetime.

Changes:

  • Introduce an atomic “resolve + pin” API (PinForScan / PinForScanOfRelease) in CompilationCacheService that re-resolves on ObjectDisposedException instead of caching an empty scan result.
  • Update NodeType configuration scan sites to use the new atomic pin API to prevent “unloading context” scan failures from mis-binding hubs.
  • Add a deterministic regression test (ScanPinSupersessionTest) covering supersession, successful re-resolve, and the “disposed context left in cache” ordering case; add a What’s New entry describing the user-visible impact.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
test/MeshWeaver.Graph.Test/ScanPinSupersessionTest.cs Adds deterministic coverage that reproduces the resolve-then-pin supersession race and asserts PinForScan recovery.
src/MeshWeaver.Graph/Configuration/MeshNodeCompilationService.cs Switches reflection scan sites to atomic resolve+pin to prevent empty-config fallback on transient ALC supersession.
src/MeshWeaver.Graph/Configuration/CompilationCacheService.cs Implements PinForScan/PinForScanOfRelease and a bounded re-resolve loop with defensive cache cleanup on disposed contexts.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-10-a-page-no-longer-loses-its-views-to-a-rebuild-next-door.md Documents the fixed user-facing failure mode and outcome.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

1 351 tests  ±0   1 346 ✅ ±0   9m 19s ⏱️ -10s
   11 suites ±0       5 💤 ±0 
   11 files   ±0       0 ❌ ±0 

Results for commit fb233c0. ± Comparison against base commit b4920dc.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

705 tests  ±0   705 ✅ ±0   13m 25s ⏱️ -14s
 10 suites ±0     0 💤 ±0 
 10 files   ±0     0 ❌ ±0 

Results for commit fb233c0. ± Comparison against base commit b4920dc.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

1 194 tests  ±0   1 194 ✅ ±0   6m 16s ⏱️ -8s
   11 suites ±0       0 💤 ±0 
   11 files   ±0       0 ❌ ±0 

Results for commit fb233c0. ± Comparison against base commit b4920dc.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

   11 files  ±0     11 suites  ±0   9m 50s ⏱️ +45s
1 984 tests ±0  1 876 ✅ ±0  108 💤 ±0  0 ❌ ±0 
2 168 runs  +1  2 060 ✅ +1  108 💤 ±0  0 ❌ ±0 

Results for commit fb233c0. ± Comparison against base commit b4920dc.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

2 024 tests  +3   2 022 ✅ +3   6m 56s ⏱️ +3s
   11 suites ±0       2 💤 ±0 
   11 files   ±0       0 ❌ ±0 

Results for commit fb233c0. ± Comparison against base commit b4920dc.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

1 786 tests  ±0   1 594 ✅ ±0   7m 51s ⏱️ -34s
   11 suites ±0     192 💤 ±0 
   11 files   ±0       0 ❌ ±0 

Results for commit fb233c0. ± Comparison against base commit b4920dc.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

   65 files  ±0     65 suites  ±0   53m 38s ⏱️ -20s
9 044 tests +3  8 737 ✅ +3  307 💤 ±0  0 ❌ ±0 
9 228 runs  +4  8 921 ✅ +4  307 💤 ±0  0 ❌ ±0 

Results for commit fb233c0. ± Comparison against base commit b4920dc.

♻️ This comment has been updated with latest results.

@rbuergi

rbuergi commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

CI: run 31408256551

Shard 4 — the shard that carries MeshWeaver.PluginCatalog.Test / StaleStampRootBindingTest — passed (6 m 36 s), as did shards 0, 1, 3, 5 and Plugins compile against this PR.

job result
Build solution (once) pass 9 m 40 s
Run tests (shard 0 / 1 / 4 / 5) pass 6 m 51 s / 7 m 5 s / 6 m 36 s / 9 m 10 s
Run tests (shard 3) pass 8 m 40 s
Run tests (shard 2) fail 9 m 0 s
Test Results (shard 0–5) pass (all six)
Consolidate test results fail — downstream of shard 2's job
Plugins compile against this PR pass

Shard 2's failure is not this diff, and not a test failure at all. Test Results (shard 2) is
pass — zero failed tests. The job failed on a process crash:

line 27: 3166 Segmentation fault  ( cd "$dir" && timeout … 8m dotnet MeshWeaver.FutuRe.Test.dll … )
[CI] MeshWeaver.FutuRe.Test exit=139

That is the CoreCLR background_sweep fault closed as external in #613. Two things rule this diff
out directly:

Not merging — CI is not fully green, and the remaining red is the #613 external fault rather than
anything this branch can fix.

@rbuergi
rbuergi merged commit 52a4e77 into main Aug 10, 2026
18 checks passed
@rbuergi
rbuergi deleted the fix/1151-alc-scan-pin-supersession branch August 19, 2026 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants