Repository navigation
fix(modules): build module singletons before they are published (#6542) - #6545
Merged
Merged
Conversation
A root singleton reaching a module service while a module singleton needed a root singleton deadlocked: Autofac holds each scope's shared-instance lock for the whole build, so two first builds in opposite orders cycled. Every shared instance of a module container is now built before the root can reach it - at boot in a root-container build callback before the provider is handed out, and on a live swap after every staged generation is prepared and before CommitAll. The root only ever reads built module instances, answered without the lock. A constructor that throws refuses the swap (N keeps serving) or, at boot, withdraws the module and its dependents and reports each as an IncompatibleModule. Staged containers bind the other generations of their own swap; a sealed stage forgets its list. Hosted services stay with their host (#2449). Residual: on-demand closed generics no constructor takes. Refs #6542 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Staged publication can expose mixed generations, failure records can be cached incomplete, and callback failure leaks the root provider.
3 open findings
What changed in this PR
Pre-builds shared module instances before root publication or live-swap commit to prevent root/module lock inversion.
Changes:
- Adds root-container build hooks and eager shared-instance construction.
- Adds staged-generation binding and failure withdrawal.
- Expands regression tests and architecture documentation.
| File | Description |
|---|---|
test/MeshWeaver.Compiler.Pipeline.Test/RootAndModuleSingletonsBuildInOneLockOrderTest.cs |
Adds lock-order regression tests. |
test/MeshWeaver.Compiler.Pipeline.Test/ModuleRootServicesSwapTest.cs |
Tests failed singleton construction during swaps. |
test/MeshWeaver.Compiler.Pipeline.Test/AModuleWhoseSingletonCannotBeBuiltAtBootTest.cs |
Tests boot-time module withdrawal. |
test/MeshWeaver.Compiler.Pipeline.Test/ACreateNeverWaitsOnAModuleContainerUnderConstructionTest.cs |
Updates create-operation concurrency coverage. |
src/MeshWeaver.ServiceProvider/ServiceProviderExtensions.cs |
Executes post-root-build hooks. |
src/MeshWeaver.ServiceProvider/ModuleServiceProvider.cs |
Adds shared-instance pre-building. |
src/MeshWeaver.ServiceProvider/IRootContainerBuilt.cs |
Defines the root build hook. |
src/MeshWeaver.Mesh.Contract/ModuleServices.cs |
Pre-builds module services with staged visibility. |
src/MeshWeaver.Mesh.Contract/ModuleOwnedRootSource.cs |
Exposes withdrawn-module records. |
src/MeshWeaver.Mesh.Contract/ModuleContexts.cs |
Coordinates building, staging, and withdrawal. |
src/MeshWeaver.Mesh.Contract/MeshBuilder.cs |
Registers the module build hook. |
src/MeshWeaver.Graph/Configuration/ModuleLiveUpdater.cs |
Builds staged generations before commit. |
src/MeshWeaver.Documentation/Data/Architecture/WriteVerdictTotality.md |
Updates create-verdict behavior. |
src/MeshWeaver.Documentation/Data/Architecture/ReadingASiloEviction.md |
Documents the resolved lock inversion. |
src/MeshWeaver.Documentation/Data/Architecture/LiveModuleUpdate.md |
Documents build-before-publication semantics. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Test Results 20 files - 4 20 suites - 4 55m 32s ⏱️ + 2m 16s Results for commit 268e24d. ± Comparison against base commit 70362c4. This pull request removes 349 and adds 27 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
…t; a failed build callback disposes the root - ModuleSwapStage: Seal() hands the generations over but keeps answering them; CommitAll releases the stage only after every generation is current, so a staged container reached through the first committed generation never falls back to the retiring generation of a later module in the same swap. - SetupModules: an IRootContainerBuilt that throws disposes the just-built root (and the shared instances earlier callbacks built) before rethrowing. Both new tests fail with the fixes reverted. Refs #6542 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Cleanup failures can still escape the boot and live-swap paths, defeating per-module isolation.
5 open findings
3 resolved since last review
🧠 Review effort: Balanced
Comment on lines
+281
to
+286
| generation.Services?.Dispose(); | ||
| var record = new IncompatibleModule(generation.Location, generation.Name, | ||
| $"its shared instances could not be built before it was published, so it was withdrawn: {why}", null); | ||
| ImmutableInterlocked.Update(ref notBuilt, list => list.Add(record)); | ||
| Console.Error.WriteLine($"[MeshWeaver.Mesh.IncompatibleModule] {record.Report()}"); | ||
| logger?.LogError("[MeshWeaver.Mesh.IncompatibleModule] {Report}", record.Report()); |
Comment on lines
+318
to
+324
| if (contexts.BuildBeforePublish(generation) is { } notBuilt) | ||
| { | ||
| contexts.DiscardStaged(stage); | ||
| return refused with | ||
| { | ||
| Outcome = Fail(name, old.Location, target, | ||
| $"{generation.Name}'s shared instances could not be built before the swap commits: {notBuilt}"), |
Comment on lines
+83
to
+88
| catch | ||
| { | ||
| // Nobody else holds this root: a callback that throws leaves no caller who could dispose | ||
| // it, or the shared instances the callbacks before it already built. | ||
| root.Dispose(); | ||
| throw; |
Comment on lines
+253
to
+255
| providers' contributions, not the retiring ones'. A sealed stage answers nothing and forgets its | ||
| list, so from the commit on every lookup is the current generations, and no stage pins a generation | ||
| a later swap retires. |
Comment on lines
+250
to
+254
| var ordered = new List<ModuleGeneration>(); | ||
| var visited = new HashSet<string>(StringComparer.Ordinal); | ||
| foreach (var generation in current.Values.OrderBy(g => g.Name, StringComparer.Ordinal)) | ||
| Visit(generation); | ||
| var withdrawn = new HashSet<string>(StringComparer.Ordinal); |
This was referenced Oct 13, 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 #6542 (sev:M)
What
The maintainer chose build before publish. Every shared instance of a module container (each singleton, and each scoped registration, which is shared at the container root) is now built before the root can reach it:
IRootContainerBuiltinstance hook runs on the building thread inSetupModules, after the root container is built and before the provider is returned. It callsModuleContexts.BuildAllBeforePublish, which builds a module after the modules it depends on.ModuleLiveUpdater.LoadAndCommitbuilds every staged generation after all of them are loaded, materialised and prepared, and beforeCommitAll.After this, the root only ever reads module instances that already exist. Autofac answers those without taking the lock, so the root lock never waits on a module container's lock and the lock-order cycle cannot form. Module constructors still run under no lock except their own container's, so a slow constructor delays only its own boot step or swap. That is why the rejected "take the root lock first" approach (the #6432 shape) does not come back.
Root-reachable set. It covers every explicit shared registration in the module container except the module's hosted services. Dependencies are covered transitively. This covers the four paths:
ModuleOwnedRootSourcestand-ins, theCurrentforwarder,ModuleServiceProxy.Invoke, andModuleHealthCheckOptionsForwarder. A descriptor's factory does not name its dependencies, so "everything shared" is the set that is safe to compute. Hosted services stay with their host: the root never reaches them, and #2449 isolation is unchanged.Residual (documented). One case is not covered: a shared closed generic that a registration source makes on demand, which no constructor takes and which is only resolved later from inside a factory or a method (
ILogger<ModuleType>,IOptions<ModuleOptions>). When a constructor takes it, it is built along with that constructor.Failure. On a swap, a constructor that throws ends the swap as
Failedwith the constructor's own message.DiscardStagedruns, N keeps serving, and N's hosted services are never stopped. At boot, the module and its dependents are withdrawn (uncommitted, container disposed). Each is reported as anIncompatibleModuleon stderr and in the log, and throughModuleOwnedRootSource, so/healthand the module-status surfaces read it. The context stays loaded until restart because a dependent may already hold its types.Staged view. A staged container is built before the commit, so it binds the other generations of the same swap (
ModuleServices.Stage,*SeenFromlookups). For example, the AI module's catalog sees its re-loaded providers' contributions.ModuleSwapStage.Seal()now also forgets its list. Without that, a context or container that keeps its stage for life would pin generations that a later swap retires.Tests
RootAndModuleSingletonsBuildInOneLockOrderTestis the repro fromwip/6542-lock-order-repro, adapted. It forces the two first builds to overlap and goes green when the container's singletons are built before publish. Its negative control publishes the same production container without building it first and deadlocks, which is the lifecycle on main.ModuleRootServicesSwapTest.AnUpdateWhoseSingletonCannotBeBuilt_IsRefusedBeforeItCommits_AndNKeepsServing: the swap fails, v1 keeps serving through the cached proxy and the module-type stand-ins, and the hosted journal stays["start v1"]. The existing live swap is its control.AModuleWhoseSingletonCannotBeBuiltAtBootTestchecks that the module is withdrawn and reported as anIncompatibleModule, and that the forwarder names the module.AModuleWhoseSingletonsBuildAtBootTestis the negative control.ACreateWaitsOnAModuleContainerUnderConstructionTesthas been renamed toACreateNeverWaitsOnAModuleContainerUnderConstructionTest. Its premise was a published module whose singleton is built lazily, and that state no longer exists: the handler is built at boot, so the old preconditionHandlersBuilt == 0is false by design. The new test covers three things:Okmeanwhile. This is the slow-constructor property that the rejected fix broke.Matchesinstead of a construction.WriteVerdictTotalityis updated to match.Local, Release: the whole
MeshWeaver.Compiler.Pipeline.Testproject passes 1406/1406, which includes the module swap and GC-collection tests.Memex.Portal.Shared.Test~Modulepasses 445/445 andStaticNodePrecedenceTestpasses 6/6. Touched projects build with-c Release -warnaserror: 0 warnings, 0 errors.Behaviour change to watch in the dependent suites. Module singleton constructors (MeshWeaver.Plugins modules included) now run at boot or swap instead of on first use, so their side effects and failures move there. A module whose singleton cannot be built at boot is now withdrawn and reported, where before it failed on first use.
Docs:
Doc/Architecture/LiveModuleUpdatehas a new section "Built before published".ReadingASiloEviction's open item is now closed.WriteVerdictTotalityis updated.Pairs-with: none — no public type or member removed; adds the public interface
MeshWeaver.ServiceProvider.IRootContainerBuilt(no existing interface gains a member)Mirror-sync: none — no i18n catalog change
🤖 Generated with Claude Code