From 264c70018cfd1bf9e19a0b86934933a3ccd511b5 Mon Sep 17 00:00:00 2001 From: antony Date: Thu, 17 Sep 2026 05:45:58 +0200 Subject: [PATCH] fix(modules): tear a destroyed module down under its own context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A module's registrations are released by calling the providing module's unregister functions, through interface facades the core bound to the module that registered them. Those calls are the module's own last work, but `Events.ModuleDestroyed` invalidated its execution context before running any of them, so every facade threw `ModuleContextInvalidatedError` and the registration stayed in the provider for the life of the process — a DMS hot reload left 138 page routes bound to a dead context and every one of them answering 500. Run the async detachments, the register cleanups and `unregisterOwner` first, and invalidate the context once they are done. --- src/modules.ts | 25 ++++++- src/tests/teardown-module-context.test.ts | 86 +++++++++++++++++++++++ 2 files changed, 108 insertions(+), 3 deletions(-) create mode 100644 src/tests/teardown-module-context.test.ts diff --git a/src/modules.ts b/src/modules.ts index 7d9be4d..7f07559 100644 --- a/src/modules.ts +++ b/src/modules.ts @@ -104,9 +104,18 @@ function getDestroyedOwner(module: string): string { return context.owner ?? module; } -Events.ModuleDestroyed.register((module) => { - const owner = getDestroyedOwner(module); - invalidateModuleContext(owner); +/** + * Takes a destroyed module's registrations back down. + * + * Runs while the owner's context is still valid: un-registering is the + * module's own last piece of work, and the disposers doing it legitimately + * call interface functions bound to that context — a route registered through + * `api` is released by calling `api`'s unregister function, through a facade + * captured when the module registered it. Invalidating first turned every such + * call into a `ModuleContextInvalidatedError` and left the registration + * standing in the providing module for as long as the process lived. + */ +function releaseOwnerRegistrations(module: string, owner: string) { for (const cleanup of internal.knownAsync.get(owner) ?? []) { runCleanup(cleanup, owner, "detach-async-provider"); } @@ -135,6 +144,16 @@ Events.ModuleDestroyed.register((module) => { }); } } +} + +Events.ModuleDestroyed.register((module) => { + const owner = getDestroyedOwner(module); + try { + releaseOwnerRegistrations(module, owner); + } finally { + // Only now: nothing else may claim to act for this generation. + invalidateModuleContext(owner); + } }); /** diff --git a/src/tests/teardown-module-context.test.ts b/src/tests/teardown-module-context.test.ts new file mode 100644 index 0000000..af09c59 --- /dev/null +++ b/src/tests/teardown-module-context.test.ts @@ -0,0 +1,86 @@ +import { expect } from "chai"; + +import { RegisteringProxy } from ".."; +import { Events, RunWithModuleContext } from "../modules"; +import { + captureModuleContext, + internal, + runWithCapturedModuleContext, +} from "../internal"; + +describe("teardown under the destroyed module's own context", () => { + let reported: unknown[]; + + beforeEach(() => { + reported = []; + internal.runtimeErrorReporter = (error) => reported.push(error); + }); + + afterEach(() => { + internal.runtimeErrorReporter = undefined; + }); + + it("lets a register cleanup call an interface function bound to that module", () => { + // The shape that broke DMS hot reload: a page registered with one module + // is taken down by calling another module's unregister function, through + // a facade the core bound to the module that did the registering. That + // call is the dying module's own last work — invalidating its context + // before running it turned every teardown step into an error and left the + // route registered in the provider for the life of the process. + const pages = new RegisteringProxy<(id: string) => void>( + "teardown.RegisterPage", + ); + const routes = new Set(); + let unregisterRoute: ((id: string) => void) | undefined; + + RunWithModuleContext( + { module: "dms", owner: "dms#1", provider: "dms" }, + () => + pages.onHandlers( + (id) => { + routes.add(id); + }, + (id) => unregisterRoute?.(id), + ), + ); + + RunWithModuleContext( + { module: "playground", owner: "playground#1" }, + () => { + // What the resolver hands a consumer: the provider's function pinned to + // the context that imported it. + const captured = captureModuleContext(); + expect(captured).to.not.equal(undefined); + unregisterRoute = (id) => + runWithCapturedModuleContext(captured!, () => { + routes.delete(id); + }); + pages.register("/modules/demo/pages/contact"); + }, + ); + + expect([...routes]).to.deep.equal(["/modules/demo/pages/contact"]); + + RunWithModuleContext({ module: "playground", owner: "playground#1" }, () => + Events.ModuleDestroyed.emit("playground"), + ); + + expect(reported).to.deep.equal([]); + expect([...routes]).to.deep.equal([]); + }); + + it("still invalidates the context once teardown is done", () => { + let captured: ReturnType; + RunWithModuleContext({ module: "gone", owner: "gone#1" }, () => { + captured = captureModuleContext(); + }); + + RunWithModuleContext({ module: "gone", owner: "gone#1" }, () => + Events.ModuleDestroyed.emit("gone"), + ); + + expect(() => + runWithCapturedModuleContext(captured!, () => undefined), + ).to.throw(/invalidated/); + }); +});