Skip to content

Commit

Permalink
Fix Miniflare#dispose() immediately after new Miniflare() (#705)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mrbbot committed Nov 1, 2023
1 parent 139f0e5 commit 864f4a5
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 4 deletions.
8 changes: 6 additions & 2 deletions packages/miniflare/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -1465,7 +1469,7 @@ export class Miniflare {
async dispose(): Promise<void> {
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();
Expand Down
5 changes: 3 additions & 2 deletions packages/miniflare/src/runtime/index.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions packages/miniflare/test/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down

0 comments on commit 864f4a5

Please sign in to comment.