From fef4d07b044a24eaf5e578face288f092f0e16b7 Mon Sep 17 00:00:00 2001 From: MrBBot Date: Thu, 5 Oct 2023 15:21:44 +0200 Subject: [PATCH] Fix `Miniflare#dispose()` immediately after `new Miniflare()` (#705) We previously waited for Miniflare to be ready before `dispose()`ing. Unfortunately, we weren't waiting for the `workerd` config to finish being written to stdin. Calling `dispose()` immediately after `new Miniflare()` would stop waiting for socket ports to be reported, and kill the `workerd` process while data was still being written. This threw an unhandled `EPIPE` error. This changes makes sure we don't report that Miniflare is ready until after the config is fully-written. Closes #680 --- packages/miniflare/src/index.ts | 8 ++++++-- packages/miniflare/src/runtime/index.ts | 5 +++-- packages/miniflare/test/index.spec.ts | 6 ++++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/miniflare/src/index.ts b/packages/miniflare/src/index.ts index b7ce402b08a4..35c0b3bac7e9 100644 --- a/packages/miniflare/src/index.ts +++ b/packages/miniflare/src/index.ts @@ -1180,7 +1180,7 @@ export class Miniflare { } } - async #waitForReady() { + async #waitForReady(disposing = false) { // If `#init()` threw, we'd like to propagate the error here, so `await` it. // Note we can't use `async`/`await` with getters. We'd also like to wait // for `setOptions` calls to complete before resolving. @@ -1191,6 +1191,10 @@ export class Miniflare { // waiters on the mutex to avoid logging ready/updated messages to the // console if there are future updates) await this.#runtimeMutex.drained(); + // If we called `dispose()`, we may not have a `#runtimeEntryURL` if we + // `dispose()`d synchronously, immediately after constructing a `Miniflare` + // instance. In this case, return a discard URL which we'll ignore. + if (disposing) return new URL("http://[100::]/"); // `#runtimeEntryURL` is assigned in `#assembleAndUpdateConfig()`, which is // called by `#init()`, and `#initPromise` doesn't resolve until `#init()` // returns. @@ -1465,7 +1469,7 @@ export class Miniflare { async dispose(): Promise { this.#disposeController.abort(); try { - await this.ready; + await this.#waitForReady(/* disposing */ true); } finally { // Remove exit hooks, we're cleaning up what they would've cleaned up now this.#removeTmpPathExitHook(); diff --git a/packages/miniflare/src/runtime/index.ts b/packages/miniflare/src/runtime/index.ts index 257abb75263f..d1301938cd0b 100644 --- a/packages/miniflare/src/runtime/index.ts +++ b/packages/miniflare/src/runtime/index.ts @@ -1,6 +1,6 @@ import assert from "assert"; import childProcess from "child_process"; -import type { Abortable } from "events"; +import { Abortable, once } from "events"; import rl from "readline"; import { Readable } from "stream"; import { red } from "kleur/colors"; @@ -146,9 +146,10 @@ export class Runtime { const controlPipe = runtimeProcess.stdio[3]; assert(controlPipe instanceof Readable); - // 3. Write config + // 3. Write config, and wait for writing to finish runtimeProcess.stdin.write(configBuffer); runtimeProcess.stdin.end(); + await once(runtimeProcess.stdin, "finish"); // 4. Wait for sockets to start listening return waitForPorts(controlPipe, options); diff --git a/packages/miniflare/test/index.spec.ts b/packages/miniflare/test/index.spec.ts index 4322d086333c..3f20a1535aed 100644 --- a/packages/miniflare/test/index.spec.ts +++ b/packages/miniflare/test/index.spec.ts @@ -709,6 +709,12 @@ test("Miniflare: listens on ipv6", async (t) => { t.true(response.ok); }); +test("Miniflare: dispose() immediately after construction", async (t) => { + const mf = new Miniflare({ script: "", modules: true }); + await mf.dispose(); + t.pass(); +}); + test("Miniflare: getBindings() returns all bindings", async (t) => { const tmp = await useTmp(t); const blobPath = path.join(tmp, "blob.txt");