[vite-plugin] Don't advertise a dev session's runtime until it is final

.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.

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<string>();
+		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", () => {

packages/miniflare/src/index.ts

@@ -984,6 +984,12 @@ export class Miniflare {
 	readonly #webSocketServer: WebSocketServer;
 	readonly #webSocketExtraHeaders: WeakMap<http.IncomingMessage, Headers>;
 	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<void> {
+		this.#checkDisposed();
+		await this.ready;
+
+		return this.#runtimeMutex.runWith(async () => {
+			this.#devRegistryRegistrationReleased = true;
+			await this.#registerWorkers();
+		});
+	}
+
 	get ready(): Promise<URL> {
 		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,

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({

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<MiniflareOptions>;
+			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();
+		});
+	});
 });

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

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