diff --git a/.changeset/tricky-masks-accept.md b/.changeset/tricky-masks-accept.md new file mode 100644 index 000000000000..2429ed0fb469 --- /dev/null +++ b/.changeset/tricky-masks-accept.md @@ -0,0 +1,12 @@ +--- +"miniflare": patch +--- + +fix: cleanup temporary directory after shutting down `workerd` + +Previously on exit, Miniflare would attempt to remove its temporary directory +before shutting down `workerd`. This could lead to `EBUSY` errors on Windows. +This change ensures we shutdown `workerd` before removing the directory. +Since we can only clean up on a best effort basis when exiting, it also catches +any errors thrown when removing the directory, in case the runtime doesn't +shutdown fast enough. \ No newline at end of file diff --git a/packages/miniflare/src/index.ts b/packages/miniflare/src/index.ts index d0712b225f4d..602c750b8dbf 100644 --- a/packages/miniflare/src/index.ts +++ b/packages/miniflare/src/index.ts @@ -563,7 +563,7 @@ export class Miniflare { #log: Log; readonly #runtime?: Runtime; - readonly #removeRuntimeExitHook?: () => void; + readonly #removeExitHook?: () => void; #runtimeEntryURL?: URL; #socketPorts?: SocketPorts; #runtimeDispatcher?: Dispatcher; @@ -573,7 +573,6 @@ export class Miniflare { // Object storage. Note this may not exist, it's up to the consumers to // create this if needed. Deleted on `dispose()`. readonly #tmpPath: string; - readonly #removeTmpPathExitHook: () => void; // Mutual exclusion lock for runtime operations (i.e. initialisation and // updating config). This essentially puts initialisation and future updates @@ -637,13 +636,21 @@ export class Miniflare { os.tmpdir(), `miniflare-${crypto.randomBytes(16).toString("hex")}` ); - this.#removeTmpPathExitHook = exitHook(() => { - fs.rmSync(this.#tmpPath, { force: true, recursive: true }); - }); // Setup runtime this.#runtime = new Runtime(); - this.#removeRuntimeExitHook = exitHook(() => void this.#runtime?.dispose()); + this.#removeExitHook = exitHook(() => { + void this.#runtime?.dispose(); + try { + fs.rmSync(this.#tmpPath, { force: true, recursive: true }); + } catch (e) { + // `rmSync` may fail on Windows with `EBUSY` if `workerd` is still + // running. `Runtime#dispose()` should kill the runtime immediately. + // `exitHook`s must be synchronous, so we can only clean up on a best + // effort basis. + this.#log.debug(`Unable to remove temporary directory: ${String(e)}`); + } + }); this.#disposeController = new AbortController(); this.#runtimeMutex = new Mutex(); @@ -1506,9 +1513,8 @@ export class Miniflare { try { await this.#waitForReady(/* disposing */ true); } finally { - // Remove exit hooks, we're cleaning up what they would've cleaned up now - this.#removeTmpPathExitHook(); - this.#removeRuntimeExitHook?.(); + // Remove exit hook, we're cleaning up what they would've cleaned up now + this.#removeExitHook?.(); // Cleanup as much as possible even if `#init()` threw await this.#proxyClient?.dispose(); diff --git a/packages/miniflare/test/index.spec.ts b/packages/miniflare/test/index.spec.ts index f01f23da165b..3635303ce18b 100644 --- a/packages/miniflare/test/index.spec.ts +++ b/packages/miniflare/test/index.spec.ts @@ -1,12 +1,14 @@ // noinspection TypeScriptValidateJSTypes import assert from "assert"; +import childProcess from "child_process"; +import { once } from "events"; import fs from "fs/promises"; import http from "http"; import { AddressInfo } from "net"; import path from "path"; import { Writable } from "stream"; -import { json } from "stream/consumers"; +import { json, text } from "stream/consumers"; import util from "util"; import { D1Database, @@ -1088,6 +1090,54 @@ unixSerialTest( } ); +test("Miniflare: exits cleanly", async (t) => { + const miniflarePath = require.resolve("miniflare"); + const result = childProcess.spawn( + process.execPath, + [ + "--no-warnings", // Hide experimental warnings + "-e", + ` + const { Miniflare, Log, LogLevel } = require(${JSON.stringify( + miniflarePath + )}); + const mf = new Miniflare({ + verbose: true, + modules: true, + script: \`export default { + fetch() { + return new Response("body"); + } + }\` + }); + (async () => { + const res = await mf.dispatchFetch("http://placeholder/"); + const text = await res.text(); + process.send(text); + process.disconnect(); + })(); + `, + ], + { + stdio: [/* in */ "ignore", /* out */ "pipe", /* error */ "pipe", "ipc"], + } + ); + + // Make sure workerd started + const [message] = await once(result, "message"); + t.is(message, "body"); + + // Check exit doesn't output anything + const closePromise = once(result, "close"); + result.kill("SIGINT"); + assert(result.stdout !== null && result.stderr !== null); + const stdout = await text(result.stdout); + const stderr = await text(result.stderr); + await closePromise; + t.is(stdout, ""); + t.is(stderr, ""); +}); + test("Miniflare: allows the use of unsafe eval bindings", async (t) => { const log = new TestLog(t);