Repository navigation
Close the boot-GC race that lets a module activation entry outlive its bytes - #2371
Merged
Merged
Conversation
…y outlive its bytes ModuleLandingService.CollectGarbage ran at every pod's boot and deleted any generation directory under modules/ that no activation entry referenced. A landing writes two things on the shared /data volume, deliberately ordered bytes-then-entry (Directory.Move, then ModuleActivationSidecar.WriteEntry) so a crash mid-landing leaves an orphan rather than a dangling entry. Those two writes are adjacent in one synchronous call on the landing replica, but nothing serializes them against a GC pass running on a DIFFERENT replica at the same moment. A GC pass that read the sidecar in the gap between the other replica's two writes saw the freshly-landed generation as "unreferenced" — indistinguishable from a genuine orphan — and deleted it a moment before the landing's WriteEntry pointed a real, enabled activation entry at it. Nothing threw anywhere: both writes on the landing side succeeded, and the entry only revealed itself as unresolvable the next time something read it. That is the exact shape #2303 reported for MeshWeaver.Blazor.EntityViews: an ACTIVATED module whose landed assembly was gone, with no exception or stack frame naming why. CollectGarbage now carries a 5-minute grace period (DefaultGarbageMinAge): an unreferenced directory younger than the window is left for a later pass instead of reclaimed immediately. A directory that is still unreferenced past the window is a genuine orphan and is collected exactly as before — the grace period defers reclamation, it never leaks. The two writes of a real landing are back-to-back with no I/O between them, so the actual exposure the window has to cover is low-single-digit seconds even over a slow network volume; five minutes is generous headroom. Also verified against #2293 (Blazor + the portal hosts moved out of core, merged the same day): the detection/reporting half of this issue — the Pending vs. Unresolvable split, the loud startup LogError, and the Degraded health check (#2093) — already exists in src/MeshWeaver.PluginCatalog and is untouched by the move. The moved Memex.Portal.Distributed host in MeshWeaver.Plugins carries its own copy of the ReportUnloadedActivatedModules wiring and still calls this repo's ConfigureMemexMesh (which calls CollectGarbage) and ModuleLandingService, so this fix reaches the real, currently-deployed portal with no cross-repo change required. Tests: CollectGarbage_LeavesAFreshGeneration_ThatNoEntryReferencesYet pins the race directly (land bytes, run GC before the entry exists, then write the entry — the generation must survive). A companion test proves the grace period defers rather than disables collection. The pre-existing CollectGarbage_RemovesOnlyTheUnreferenced is updated to backdate its genuine orphans past the window. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Closes #2303 by hardening module-folder garbage collection against a cross-replica boot-time race where GC could delete a freshly landed generation directory before the activation entry that references it is written.
Changes:
- Add a configurable minimum-age (“grace period”) to
ModuleLandingService.CollectGarbage, defaulting to 5 minutes, so newly created unreferenced directories are not immediately reclaimed. - Extend
CollectGarbagewith test seams (minAge,nowUtc) and add targeted tests that pin both the race and the “still eventually collected” invariant. - Document the failure mode and fix in the architecture docs and a What’s New entry.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/MeshWeaver.PluginCatalog.Test/PendingLandingTest.cs | Adds coverage for the GC-vs-landing race and validates deferred vs eventual collection behavior. |
| src/MeshWeaver.PluginCatalog/ModuleLandingService.cs | Implements the grace-period check in boot-time modules GC and exposes test seams for deterministic testing. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-26-store-module-cleanup-no-longer-races-a-landing.md | Adds a user-visible Fix entry describing the symptom and behavioral change. |
| src/MeshWeaver.Documentation/Data/Architecture/Modules.md | Documents the race mechanism and the grace-period mitigation in the modules architecture guide. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
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.
Summary
Closes #2303 — root-caused and fixed.
ModuleLandingService.CollectGarbageruns at every pod's boot and deletes any generation directory undermodules/that no activation entry references. A landing writes two things on the shared/datavolume, deliberately ordered bytes-then-entry (Directory.Move, thenModuleActivationSidecar.WriteEntry) so a crash mid-landing leaves an orphan rather than a dangling entry — but those two writes are only atomic on the landing replica. Nothing serializes them against a GC pass on a different replica at the same moment (rolling restart / auto-update landing while a sibling pod boots).If a GC pass reads the sidecar in the gap between the other replica's two writes, the freshly-landed generation directory looks "unreferenced" — indistinguishable from a genuine orphan — and GC deletes it a moment before the landing's
WriteEntrylands, pointing a real, enabled activation entry at bytes that no longer exist. Nothing throws anywhere: both of the landing's own writes succeeded. The entry only reveals itself as unresolvable the next time something reads it — exactly the shape #2303 reported forMeshWeaver.Blazor.EntityViews: an ACTIVATED module whose landed assembly was gone, with no exception naming why.Fix
CollectGarbagenow carries a 5-minute grace period (ModuleLandingService.DefaultGarbageMinAge): an unreferenced directory younger than the window is left for a later pass instead of reclaimed immediately. A directory still unreferenced past the window is a genuine orphan and is collected exactly as before — the grace period defers reclamation, it never leaks. The two writes of a real landing are back-to-back with no I/O between them, so the actual exposure is low-single-digit seconds even over a slow network volume; 5 minutes is generous headroom.On PR #2293 (Blazor + the portal hosts moved out of core, merged the same day)
Checked whether the extraction changed this issue rather than assuming either way:
ModuleActivationStatus's Pending-vs-Unresolvable split, the loud startupLogError, and theDegradedhealth check (Store-delivered endpoint module (MeshWeaver.Mcp) activates but is NOT host-loaded → /mcp 404s wholesale #2093) — lives insrc/MeshWeaver.PluginCatalog, is untouched by the move, and is exactly what produced the issue's own log evidence (the reporter was working correctly; it was reporting a real defect in the landing lane).memex/Memex.Portal.Shared/MemexConfiguration.cslost the call site that used to invoke itsReportUnloadedActivatedModules(the caller, a Blazor-typedStartMemexApplication<TApp>, moved out) — but I verified the movedMemex.Portal.Distributedhost inSystemorph/MeshWeaver.Pluginscarries its own copy of that wiring (Memex.Portal.Gui/MemexPortalComposition.cs), and it still calls this repo'sConfigureMemexMesh(which callsCollectGarbage) andModuleLandingService. So this fix reaches the real, currently-deployed portal with no cross-repo change required. (The now-dead private method left behind in core'sMemexConfiguration.csis harmless — unused, no build impact — and out of scope here per the stay-out list for that file.)Tests
CollectGarbage_LeavesAFreshGeneration_ThatNoEntryReferencesYetpins the race directly: land bytes, run GC before the entry exists, then write the entry — the generation must survive GC for the entry to resolve.CollectGarbage_ReclaimsAnUnreferencedGeneration_OnceItIsOldEnoughproves the grace period defers collection rather than disabling it.CollectGarbage_RemovesOnlyTheUnreferenced(pre-existing) is updated to backdate its genuine orphans past the grace window.Test plan
dotnet build src/MeshWeaver.PluginCatalog -c Release -warnaserror— cleandotnet build test/MeshWeaver.PluginCatalog.Test -c Release -warnaserror— cleandotnet test test/MeshWeaver.PluginCatalog.Test -c Release --no-build— 649/649 passed (fresh .trx)dotnet build test/MeshWeaver.Documentation.Test -c Release -warnaserror— cleandotnet test test/MeshWeaver.Documentation.Test -c Release --no-build --filter WhatsNewEntryIntegrityTest|DocumentationLinkIntegrityTest— 2/2 passedCategory: Fix)🤖 Generated with Claude Code