From c1da2d7b31a7490715a5d71c1619a2f8b1034d09 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Tue, 4 Aug 2026 21:57:58 +0100 Subject: [PATCH] [vite-plugin] Don't advertise a dev session's runtime until it is final The plugin starts workerd twice: once to discover each Worker's exports by running it, then again with a config built from what it found. The first runtime was published to the dev registry and then torn down, leaving peers holding a debug port that no longer existed. On Windows a peer with a tail_consumers edge to it aborts its own workerd with std::terminate. Miniflare gains unsafeDeferDevRegistryRegistration to hold back self-advertisement, and unsafeRegisterInDevRegistry() to release it once the runtime is final. Reading the registry is unaffected, so a starting session still resolves Workers from sessions already running. --- .changeset/tidy-donkeys-listen.md | 12 ++ .../dev-registry/tests/dev-registry.test.ts | 41 +++++++ packages/miniflare/src/index.ts | 41 ++++++- packages/miniflare/src/plugins/core/index.ts | 12 ++ packages/miniflare/test/dev-registry.spec.ts | 113 ++++++++++++++++++ .../src/miniflare-options.ts | 7 ++ .../vite-plugin-cloudflare/src/plugins/dev.ts | 11 ++ 7 files changed, 236 insertions(+), 1 deletion(-) create mode 100644 .changeset/tidy-donkeys-listen.md diff --git a/.changeset/tidy-donkeys-listen.md b/.changeset/tidy-donkeys-listen.md new file mode 100644 index 00000000000..f7fd0f3d742 --- /dev/null +++ b/.changeset/tidy-donkeys-listen.md @@ -0,0 +1,12 @@ +--- +"@cloudflare/vite-plugin": patch +"miniflare": minor +--- + +Don't advertise a `vite dev` session's Workers in the dev registry until its runtime is final + +The Vite plugin brings `workerd` up twice while starting: once to discover each Worker's exports by running it, then again with a config built from what it found. The first runtime was advertised in the dev registry before being torn down, so another dev session that resolved it was left holding a debug port that no longer existed. On Windows that can abort the other session's runtime, taking down a Worker that had been running happily. + +Registration is now held back until the runtime that Vite settles on is the one peers will actually connect to. Reading the registry is unaffected, so a starting session still reaches Workers from sessions that are already running. + +This adds two Miniflare APIs for consumers that bring their runtime up in more than one step: the `unsafeDeferDevRegistryRegistration` option, and `unsafeRegisterInDevRegistry()` to release the hold once the final runtime is ready. diff --git a/fixtures/dev-registry/tests/dev-registry.test.ts b/fixtures/dev-registry/tests/dev-registry.test.ts index c60c9e7ac41..f6a784218e5 100644 --- a/fixtures/dev-registry/tests/dev-registry.test.ts +++ b/fixtures/dev-registry/tests/dev-registry.test.ts @@ -808,6 +808,47 @@ describe("Dev Registry: vite dev <-> vite dev", () => { ); }, waitForTimeout); }); + + it("only ever advertises one debug port while starting up", async ({ + expect, + devRegistryPath, + }) => { + // The plugin brings the runtime up twice during startup: once to discover + // each Worker's exports by running it, then again with a config built from + // what it found. The second runtime gets a new debug port, so advertising the + // first one hands peers an address that is about to disappear - and a peer + // holding a dead address can have its own `workerd` aborted. + // + // Sampling the registry throughout startup catches any intermediate address: + // before the fix the first one stayed published for well over a second. + const definitionPath = path.join(devRegistryPath, "worker-entrypoint"); + const advertised = new Set(); + let sampling = true; + const sampler = (async () => { + while (sampling) { + try { + const { debugPortAddress } = JSON.parse( + await fs.readFile(definitionPath, "utf8") + ); + if (typeof debugPortAddress === "string") { + advertised.add(debugPortAddress); + } + } catch { + // Not registered yet, or a partially written file. + } + await new Promise((resolve) => setTimeout(resolve, 15)); + } + })(); + + try { + await runViteDev("vite.worker-entrypoint.config.ts", devRegistryPath); + } finally { + sampling = false; + await sampler; + } + + expect([...advertised]).toHaveLength(1); + }); }); describe("Dev Registry: vite dev <-> wrangler dev", () => { diff --git a/packages/miniflare/src/index.ts b/packages/miniflare/src/index.ts index 87314a6b275..2cb56ce503a 100644 --- a/packages/miniflare/src/index.ts +++ b/packages/miniflare/src/index.ts @@ -984,6 +984,12 @@ export class Miniflare { readonly #webSocketServer: WebSocketServer; readonly #webSocketExtraHeaders: WeakMap; readonly #devRegistry: DevRegistry; + // Whether `unsafeRegisterInDevRegistry()` has released the hold that + // `unsafeDeferDevRegistryRegistration` puts on advertising our Workers. + // Reset by every `setOptions()` that asks to defer again, so a consumer that + // rebuilds its config (a Vite dev server restart, for instance) gets the same + // protection on each cycle rather than only on the first one. + #devRegistryRegistrationReleased = false; #maybeInspectorProxyController?: InspectorProxyController; #previousRuntimeInspectorPort?: number; @@ -2588,7 +2594,15 @@ export class Miniflare { this.#proxyClient.setRuntimeEntryURL(this.#runtimeEntryURL); } - await this.#registerWorkers(); + // Deferred registration only holds back advertising *our* Workers. The + // registry push below is what lets us reach everyone else's, so it always + // runs. + if ( + !this.#sharedOpts.core.unsafeDeferDevRegistryRegistration || + this.#devRegistryRegistrationReleased + ) { + await this.#registerWorkers(); + } // Catch any registry updates that occurred while workerd was booting. if (this.#devRegistry.isEnabled()) { @@ -2729,6 +2743,24 @@ export class Miniflare { this.#devRegistry.register(Object.fromEntries(entries)); } + /** + * Advertise this instance's Workers in the dev registry, releasing the hold + * put in place by `unsafeDeferDevRegistryRegistration`. + * + * Call this once the runtime is the one peers should actually connect to. It + * is idempotent, and a no-op unless `unsafeDeferDevRegistryRegistration` is + * set. Registration does not restart `workerd`, so this is cheap. + */ + async unsafeRegisterInDevRegistry(): Promise { + this.#checkDisposed(); + await this.ready; + + return this.#runtimeMutex.runWith(async () => { + this.#devRegistryRegistrationReleased = true; + await this.#registerWorkers(); + }); + } + get ready(): Promise { return this.#waitForReady(); } @@ -2825,6 +2857,13 @@ export class Miniflare { this.#log = this.#sharedOpts.core.log ?? this.#log; this.#hyperdriveProxyController.log = this.#log; + // `updateRegistryPath()` below unregisters our Workers, and the runtime is + // about to be replaced, so re-arm the hold and wait to be told that the new + // runtime is the one to advertise. + if (sharedOpts.core.unsafeDeferDevRegistryRegistration) { + this.#devRegistryRegistrationReleased = false; + } + const newExternalOnUpdate = sharedOpts.core.unsafeHandleDevRegistryUpdate; await this.#devRegistry.updateRegistryPath( sharedOpts.core.unsafeDevRegistryPath, diff --git a/packages/miniflare/src/plugins/core/index.ts b/packages/miniflare/src/plugins/core/index.ts index 53569af0075..95e01fece11 100644 --- a/packages/miniflare/src/plugins/core/index.ts +++ b/packages/miniflare/src/plugins/core/index.ts @@ -281,6 +281,18 @@ export const CoreSharedOptionsSchema = z.object({ // Enable auto service / durable objects discovery with the dev registry unsafeDevRegistryPath: z.string().optional(), + // Don't advertise this instance's Workers in the dev registry until + // `unsafeRegisterInDevRegistry()` is called. + // + // Consumers that bring the runtime up in more than one step (the Vite plugin + // discovers each Worker's exports by running it, then rebuilds the config and + // calls `setOptions()`) would otherwise publish a debug port that is about to + // be torn down. Peers who resolve it in that window hold an address that no + // longer exists, which on Windows can abort their `workerd` outright. + // + // Only self-advertisement is held back; the registry is still read, so + // external services provided by other sessions keep resolving as usual. + unsafeDeferDevRegistryRegistration: z.boolean().optional(), // Called when external workers this instance depends on are updated in the dev registry unsafeHandleDevRegistryUpdate: z .function({ diff --git a/packages/miniflare/test/dev-registry.spec.ts b/packages/miniflare/test/dev-registry.spec.ts index 5f7714e07f6..9323a11e0e7 100644 --- a/packages/miniflare/test/dev-registry.spec.ts +++ b/packages/miniflare/test/dev-registry.spec.ts @@ -1787,4 +1787,117 @@ describe.sequential("DevRegistry", () => { { timeout: 10_000, interval: 100 } ); }); + + describe("unsafeDeferDevRegistryRegistration", () => { + test("withholds advertisement until registration is released, and re-arms on setOptions", async ({ + expect, + }) => { + const unsafeDevRegistryPath = await useTmp(); + const sharedOptions = { + name: "deferred-worker", + unsafeDevRegistryPath, + unsafeDeferDevRegistryRegistration: true, + modules: true, + } satisfies Partial; + const mf = new Miniflare({ + ...sharedOptions, + script: `export default { fetch() { return new Response("one"); } }`, + }); + useDispose(mf); + + await mf.ready; + + // A peer resolving us now would get a debug port we are about to replace. + expect( + getWorkerRegistry(unsafeDevRegistryPath)["deferred-worker"] + ).toBeUndefined(); + + await mf.unsafeRegisterInDevRegistry(); + + const firstEntry = getWorkerRegistry(unsafeDevRegistryPath)[ + "deferred-worker" + ]; + expect(firstEntry).toBeDefined(); + expect(firstEntry.debugPortAddress).toMatch(/^127\.0\.0\.1:\d+$/); + + // A fresh runtime means a fresh debug port, so the hold goes back on + // rather than leaving the previous address advertised. + await mf.setOptions({ + ...sharedOptions, + script: `export default { fetch() { return new Response("two"); } }`, + }); + + expect( + getWorkerRegistry(unsafeDevRegistryPath)["deferred-worker"] + ).toBeUndefined(); + + await mf.unsafeRegisterInDevRegistry(); + + expect( + getWorkerRegistry(unsafeDevRegistryPath)["deferred-worker"] + ).toBeDefined(); + }); + + test("still resolves external services while its own advertisement is held back", async ({ + expect, + }) => { + const unsafeDevRegistryPath = await useTmp(); + const remote = new Miniflare({ + name: "remote-worker", + unsafeDevRegistryPath, + modules: true, + script: `export default { fetch() { return new Response("Hello from remote!"); } }`, + }); + useDispose(remote); + await remote.ready; + + // Deferring only holds back what we publish about ourselves; reading the + // registry has to keep working or startup would not be able to talk to + // sessions that are already running. + const local = new Miniflare({ + name: "local-worker", + unsafeDevRegistryPath, + unsafeDeferDevRegistryRegistration: true, + serviceBindings: { SERVICE: { name: "remote-worker" } }, + modules: true, + script: ` + export default { + fetch(request, env) { + return env.SERVICE.fetch(request); + } + } + `, + }); + useDispose(local); + + await vi.waitFor( + async () => { + const res = await local.dispatchFetch("http://placeholder"); + expect(await res.text()).toBe("Hello from remote!"); + }, + { timeout: 10_000, interval: 100 } + ); + + expect( + getWorkerRegistry(unsafeDevRegistryPath)["local-worker"] + ).toBeUndefined(); + }); + + test("advertises immediately when not deferring", async ({ expect }) => { + const unsafeDevRegistryPath = await useTmp(); + const mf = new Miniflare({ + name: "eager-worker", + unsafeDevRegistryPath, + modules: true, + script: `export default { fetch() { return new Response("ok"); } }`, + }); + useDispose(mf); + + await mf.ready; + + expect( + getWorkerRegistry(unsafeDevRegistryPath)["eager-worker"] + ).toBeDefined(); + }); + }); }); diff --git a/packages/vite-plugin-cloudflare/src/miniflare-options.ts b/packages/vite-plugin-cloudflare/src/miniflare-options.ts index 46ceb863a6f..3e37d9b54d6 100644 --- a/packages/vite-plugin-cloudflare/src/miniflare-options.ts +++ b/packages/vite-plugin-cloudflare/src/miniflare-options.ts @@ -558,6 +558,13 @@ export async function getDevMiniflareOptions( inspectorPort: inputInspectorPort === false ? undefined : inputInspectorPort, unsafeDevRegistryPath: getDefaultDevRegistryPath(), + // We bring the runtime up in two steps: once to discover each Worker's + // exports by running it, then again with a config built from what we + // found. Advertising the first runtime would publish a debug port we are + // about to tear down, and a peer holding a dead address can have its own + // `workerd` aborted. `configureServer` releases the hold once the runtime + // it leaves behind is the one peers should connect to. + unsafeDeferDevRegistryRegistration: true, unsafeTriggerHandlers: true, unsafeLocalExplorer: getLocalExplorerEnabledFromEnv(), // The switch for local observability capture: tells Miniflare core to diff --git a/packages/vite-plugin-cloudflare/src/plugins/dev.ts b/packages/vite-plugin-cloudflare/src/plugins/dev.ts index a386f517891..0ddded877c3 100644 --- a/packages/vite-plugin-cloudflare/src/plugins/dev.ts +++ b/packages/vite-plugin-cloudflare/src/plugins/dev.ts @@ -311,6 +311,17 @@ export const devPlugin = createPlugin("dev", (ctx) => { } } + // The runtime we are leaving behind is the one peers should connect to, so + // it is now safe to advertise it. Everything above may have replaced the + // runtime (rediscovering Worker exports, rebuilding container images), and + // publishing a debug port before that settles hands peers an address that + // is about to disappear. + // + // This runs for every plugin config type, and whether or not the export + // types turned out to differ, because `unsafeDevRegistryRegistration` is + // deferred unconditionally in `getDevMiniflareOptions`. + await ctx.miniflare.unsafeRegisterInDevRegistry(); + return () => { // In Vite 6, pre-middleware is placed before the host check middleware, // leaving the server vulnerable to DNS rebinding attacks. We move it to