diff --git a/apps/geolibre-desktop/src/components/processing/ProcessingDialog.tsx b/apps/geolibre-desktop/src/components/processing/ProcessingDialog.tsx index d306d5f05..14d7ba683 100644 --- a/apps/geolibre-desktop/src/components/processing/ProcessingDialog.tsx +++ b/apps/geolibre-desktop/src/components/processing/ProcessingDialog.tsx @@ -190,8 +190,9 @@ function isSubsetUrlParameter(tool: WhiteboxTool, param: WhiteboxToolParameter): // vector inputs (points_to_line's `line_field`/`sort_field`, and ~170 other // tools), so the dialog can offer the selected layer's attribute names instead // of asking the user to recall a column name (GeoLibre#1459). The kind check is -// what keeps a same-named *dataset* param out (join_tables' `primary_key_field` -// is a vector input): only a scalar string names a column. +// what keeps a same-named *dataset* param out (the catalog types +// classify_objects_svm's `class_field` as a LiDAR input): only a scalar string +// names a column. function isFieldParameter(param: WhiteboxToolParameter): boolean { return parameterKind(param) === "string" && isFieldParameterName(param.name); } diff --git a/apps/geolibre-desktop/src/lib/whitebox-field-params.ts b/apps/geolibre-desktop/src/lib/whitebox-field-params.ts index 914116d3a..6c06daad0 100644 --- a/apps/geolibre-desktop/src/lib/whitebox-field-params.ts +++ b/apps/geolibre-desktop/src/lib/whitebox-field-params.ts @@ -17,9 +17,12 @@ export const FIELD_PARAM_SUFFIX = /(^|_)(fields?|attributes?)$/i; /** * Whether a parameter name reads as an attribute-column name. * - * Callers must also check the parameter is a scalar string: `join_tables` - * exposes `primary_key_field` as a *dataset* input, and a dataset parameter - * names a file, not a column. + * Callers must also check the parameter is a scalar string: the sidecar's + * catalog exposes `classify_objects_svm`'s `class_field` as a *dataset* input, + * and a dataset parameter names a file, not a column. (The WASM manifests used + * to mistype ~40 of these the same way, `dissolve_field` among them, until + * opengeos/whitebox-wasm#19 taught the manifest inference that a `*_field` name + * is a column; the sidecar catalog still carries a few.) * * @param name - The tool parameter's name. * @returns `true` when the name ends in a field/attribute suffix. diff --git a/package-lock.json b/package-lock.json index 13fcc5780..4111411e4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15291,9 +15291,9 @@ "link": true }, "node_modules/geolibre-wasm": { - "version": "1.5.1", - "resolved": "https://registry.npmjs.org/geolibre-wasm/-/geolibre-wasm-1.5.1.tgz", - "integrity": "sha512-axGG7xfuFsFiyk1Y1SkrmjXwzHu0/LfO1WX/xw4rrVbFbdTenqAs7VdDlpnIjBn4+Enm9hhINqq3p40f+87bRQ==", + "version": "1.5.2", + "resolved": "https://registry.npmjs.org/geolibre-wasm/-/geolibre-wasm-1.5.2.tgz", + "integrity": "sha512-YVG2DsWMfqx1UcdImpX3CKHhAwsCfQG8lO1u5k9RqxYwT+hm3q1GJo0YSYx1DM9sGJXiWlXyIjHGdyIqjpG0tQ==", "license": "MIT", "dependencies": { "@bjorn3/browser_wasi_shim": "^0.4.2" @@ -23911,7 +23911,7 @@ "@turf/voronoi": "^7.4.0", "dggal": "^0.0.6", "fflate": "^0.8.3", - "geolibre-wasm": "^1.5.1", + "geolibre-wasm": "^1.5.2", "geotiff": "^3.0.5", "onnxruntime-web": "1.27.0", "s2js": "^1.44.0" diff --git a/packages/processing/package.json b/packages/processing/package.json index 1058654b8..c138f499b 100644 --- a/packages/processing/package.json +++ b/packages/processing/package.json @@ -34,7 +34,7 @@ "@turf/voronoi": "^7.4.0", "dggal": "^0.0.6", "fflate": "^0.8.3", - "geolibre-wasm": "^1.5.1", + "geolibre-wasm": "^1.5.2", "geotiff": "^3.0.5", "onnxruntime-web": "1.27.0", "s2js": "^1.44.0" diff --git a/packages/processing/src/wasm-client.ts b/packages/processing/src/wasm-client.ts index eac56746a..682483ab2 100644 --- a/packages/processing/src/wasm-client.ts +++ b/packages/processing/src/wasm-client.ts @@ -8,6 +8,7 @@ import type { FeatureCollection } from "geojson"; import { convertGeoTiffToCog } from "./cog-convert"; import { normalizeVectorOutputFormat } from "./sidecar-client"; +import { runWasmToolInBackground } from "./wasm-tool-runner"; import type { RunWhiteboxToolRequest, VectorOutputFormat, @@ -651,7 +652,6 @@ export async function ensureWhiteboxRasterCog(bytes: Uint8Array): Promise { - const { runTool } = await loadToolsModule(); const encoder = new TextEncoder(); const input: Record = {}; const args: string[] = []; @@ -770,7 +770,14 @@ export async function runWhiteboxToolWasm(request: RunWhiteboxToolRequest): Prom } } - const { exitCode, stdout, files } = await runTool(request.tool_id, { args, input }); + // Off the main thread: the WASI runner is one synchronous call with no yield + // points, so running it here would freeze the UI for the tool's whole + // duration (~60s for the 290-polygon dissolve in GeoLibre#1977). + const { exitCode, stdout, files } = await runWasmToolInBackground({ + tool: request.tool_id, + args, + input, + }); if (exitCode !== 0) { return job( request.tool_id, diff --git a/packages/processing/src/wasm-convert.ts b/packages/processing/src/wasm-convert.ts index 248ab2a6c..98e48fb83 100644 --- a/packages/processing/src/wasm-convert.ts +++ b/packages/processing/src/wasm-convert.ts @@ -14,7 +14,7 @@ // All three run entirely client-side, so the web build needs no Python sidecar // for them. import type { RunToolOptions, ToolResult } from "geolibre-wasm/tools"; -import type { WasmToolRequest, WasmToolResponse } from "./wasm-convert.worker"; +import { runWasmToolInBackground } from "./wasm-tool-runner"; /** The subset of `geolibre-wasm/tools` these converters use. */ interface ConvertToolsModule { @@ -59,61 +59,6 @@ export async function initConvertTools( await initTools(source); } -/** - * Run a tool on a one-shot Web Worker and resolve with its result. - * - * No timeout: how long a tool runs is bounded by the data, not the clock (a - * country-scale tile pyramid is minutes), and cutting off work that would have - * finished is worse than waiting. `error`/`messageerror` still reject, so the - * promise settles on every failure the worker can report. - */ -function runToolOnWorker(request: WasmToolRequest): Promise { - return new Promise((resolve, reject) => { - const worker = new Worker(new URL("./wasm-convert.worker.ts", import.meta.url), { - type: "module", - }); - worker.addEventListener("message", (event: MessageEvent) => { - worker.terminate(); - if (event.data.ok) resolve(event.data.result); - else reject(new Error(event.data.error || `${request.tool} failed.`)); - }); - worker.addEventListener("error", (event) => { - worker.terminate(); - reject(new Error(event.message || `The ${request.tool} worker failed.`)); - }); - // `error` does not fire when a posted message cannot be deserialized, which - // would otherwise leave this promise pending forever. - worker.addEventListener("messageerror", () => { - worker.terminate(); - reject(new Error(`The ${request.tool} worker posted an undeserializable message.`)); - }); - // The input files are structured-cloned rather than transferred: these - // wrappers do not otherwise take ownership of the caller's bytes, and a - // neutered input array would be a trap the sibling converters don't set. - try { - worker.postMessage(request); - } catch (error) { - // A throw here (e.g. DataCloneError) rejects the promise on its own, but - // the worker is already spawned and would leak without this. - worker.terminate(); - reject(error instanceof Error ? error : new Error(String(error))); - } - }); -} - -/** - * Run a tool off the main thread where Workers exist, inline where they do not - * (node, tests). The inline path is why {@link initConvertTools} still takes an - * explicit wasm source: a worker resolves its own bundled copy instead. - */ -async function runToolInBackground(request: WasmToolRequest): Promise { - if (typeof Worker === "undefined") { - const { runTool } = await loadToolsModule(); - return runTool(request.tool, { args: request.args, input: request.input }); - } - return runToolOnWorker(request); -} - /** An input file for a WASM conversion: its name (the extension drives format * detection) and its raw bytes. */ export interface WasmConvertFile { @@ -295,7 +240,7 @@ export interface VectorToPmtilesOptions { * `siblings`, exactly as in {@link convertVectorWithWasm}. * * Unlike its siblings here this runs on a Web Worker (see - * {@link runToolInBackground}). Tiling is by far the heaviest of these tools — + * {@link runWasmToolInBackground}). Tiling is by far the heaviest of these tools — * a US-wide layer to the default zoom 14 is millions of tiles and minutes of * uninterrupted WASM — so running it on the main thread would freeze the UI for * the whole conversion. The others finish quickly enough not to warrant the @@ -322,7 +267,7 @@ export async function tileVectorToPmtiles( ]); const files: Record = { [input.name]: input.data }; for (const sibling of siblings) files[sibling.name] = sibling.data; - const result = await runToolInBackground({ + const result = await runWasmToolInBackground({ tool: "vector_to_pmtiles", args, input: files, diff --git a/packages/processing/src/wasm-convert.worker.ts b/packages/processing/src/wasm-convert.worker.ts deleted file mode 100644 index 11f7bcf8f..000000000 --- a/packages/processing/src/wasm-convert.worker.ts +++ /dev/null @@ -1,40 +0,0 @@ -/// -import { runTool } from "geolibre-wasm/tools"; -import type { ToolResult } from "geolibre-wasm/tools"; - -// Runs one `geolibre-wasm/tools` WASI tool off the main thread. The runner has -// no yield points once a tool starts, so on the main thread a long job freezes -// the whole UI for its duration — tiling a country-scale vector layer to zoom 14 -// is minutes of that, which is why wasm-convert.ts routes it here. -// -// One tool per worker: the caller terminates this worker as soon as the terminal -// message arrives, so nothing here has to be reusable across runs. -const worker = self as unknown as DedicatedWorkerGlobalScope; - -/** A tool to run: its id, CLI args, and the files to place under /work. */ -export interface WasmToolRequest { - tool: string; - args: string[]; - input: Record; -} - -/** The single message this worker posts back. */ -export type WasmToolResponse = { ok: true; result: ToolResult } | { ok: false; error: string }; - -worker.addEventListener("message", async (event: MessageEvent) => { - const { tool, args, input } = event.data; - try { - // runTool compiles the bundled geolibre-cli.wasm on first use, in this - // worker's own module scope — a copy already compiled on the main thread - // is not shared with it. - const result = await runTool(tool, { args, input }); - worker.postMessage({ ok: true, result } satisfies WasmToolResponse); - } catch (error) { - // A tool that merely exits non-zero resolves normally and is reported by - // the caller; reaching here means the runner itself threw. - worker.postMessage({ - ok: false, - error: error instanceof Error ? error.message : String(error), - } satisfies WasmToolResponse); - } -}); diff --git a/packages/processing/src/wasm-tool-runner.ts b/packages/processing/src/wasm-tool-runner.ts new file mode 100644 index 000000000..e6336ec1b --- /dev/null +++ b/packages/processing/src/wasm-tool-runner.ts @@ -0,0 +1,184 @@ +// Shared plumbing for running a `geolibre-wasm/tools` WASI tool off the main +// thread. +// +// The runner has no yield points once a tool starts: `wasi.start()` is one +// synchronous call that returns only when the tool is done. On the main thread +// that freezes the whole UI — no repaint, no input — for the tool's entire +// duration, which is bounded by the data rather than the clock. Dissolving the +// 290-polygon layer from GeoLibre#1977 blocks it for ~60s. +// +// wasm-convert.ts routed its tiling calls through a worker for exactly this +// reason. This module is that machinery, lifted out so the Whitebox toolbox +// (wasm-client.ts) shares one implementation with it instead of growing a +// second copy that could drift. +import type { ToolResult } from "geolibre-wasm/tools"; +import type { WasmToolRequest, WasmToolResponse } from "./wasm-tool.worker"; + +export type { WasmToolRequest, WasmToolResponse }; + +// Idle workers, kept alive to be reused. A worker compiles the ~23 MB +// `geolibre-cli.wasm` in its *own* module scope, and the main thread's +// already-compiled copy is not shared with it, so a worker discarded after +// every run makes each run pay that fetch and compile again. That is invisible +// next to a minutes-long tiling job — the only thing that used this path +// before — but not next to the many Whitebox tools that finish in well under a +// second, where it would dominate the run. +// +// Reuse rather than a single shared worker: a WASI run is synchronous inside +// its worker, so one worker would serialize concurrent runs that used to +// overlap. Taking an idle worker when there is one and spawning otherwise keeps +// that parallelism and still pays the compile once per worker. +const idleWorkers: Worker[] = []; + +// How many idle workers to keep warm. Each holds its compiled module (tens of +// MB) for the rest of the session, and real usage is one tool at a time, so +// one warm worker captures nearly all of the benefit; extras are terminated +// rather than parked. +const MAX_IDLE_WORKERS = 1; + +/** + * Terminate every parked worker and forget them, freeing the compiled module + * each one holds. Runs in flight are unaffected — they own their worker until + * it answers. Call it to reclaim that memory, and in tests, so a worker parked + * by one case is not handed to the next. + */ +export function releaseIdleWasmToolWorkers(): void { + for (const worker of idleWorkers.splice(0)) worker.terminate(); +} + +function spawnWorker(): Worker { + return new Worker(new URL("./wasm-tool.worker.ts", import.meta.url), { type: "module" }); +} + +/** A worker to run on, and whether it came from the pool rather than being new. */ +function acquireWorker(): { worker: Worker; reused: boolean } { + const parked = idleWorkers.pop(); + return parked ? { worker: parked, reused: true } : { worker: spawnWorker(), reused: false }; +} + +// How long a *reused* worker has to acknowledge a request before it is treated +// as dead. A parked worker is idle by construction — it is only parked once it +// has answered — so an ack is a message hop away and this window is enormous by +// comparison. It exists because a worker killed out of band (an OOM kill, a +// discarded tab) fires no `error` event and silently swallows `postMessage`, and +// this module deliberately puts no timeout on the run itself, so without the ack +// a dead worker would leave the run pending forever. A false positive costs a +// respawn, never a failed run. +const REUSED_WORKER_ACK_MS = 10_000; + +/** Park a still-healthy worker for reuse, or terminate it if enough are warm. */ +function releaseWorker(worker: Worker): void { + if (idleWorkers.length < MAX_IDLE_WORKERS) idleWorkers.push(worker); + else worker.terminate(); +} + +/** + * Run a tool on a Web Worker and resolve with its result. + * + * No timeout: how long a tool runs is bounded by the data, not the clock (a + * country-scale tile pyramid is minutes), and cutting off work that would have + * finished is worse than waiting. `error`/`messageerror` still reject, so the + * promise settles on every failure the worker can report. + * + * A worker that answers is parked for reuse; one that fails at the worker level + * is terminated, since its state after that is not something to hand the next + * caller. Listeners are removed on the way out so a reused worker does not + * accumulate them. + */ +function runToolOnWorker(request: WasmToolRequest): Promise { + return new Promise((resolve, reject) => { + let { worker, reused } = acquireWorker(); + // Only a reused worker is watched: a freshly spawned one has not had time to + // die, and a broken one reports itself through `error`. Its startup also + // includes loading this worker's module graph, which in a dev server can be + // slower than any ack window worth setting. + let ackTimer: ReturnType | undefined; + const armAck = () => { + if (!reused) return; + ackTimer = setTimeout(() => { + // Never acknowledged, so it died while parked. Replace it and re-post; + // the request has not started, so nothing is lost by retrying once. + cleanup(); + worker.terminate(); + worker = spawnWorker(); + reused = false; + attach(); + post(); + }, REUSED_WORKER_ACK_MS); + }; + const disarmAck = () => { + if (ackTimer !== undefined) clearTimeout(ackTimer); + ackTimer = undefined; + }; + const onMessage = (event: MessageEvent) => { + if (event.data.ok === "ack") { + disarmAck(); + return; + } + disarmAck(); + cleanup(); + releaseWorker(worker); + if (event.data.ok) resolve(event.data.result); + else reject(new Error(event.data.error || `${request.tool} failed.`)); + }; + const onError = (event: ErrorEvent) => { + disarmAck(); + cleanup(); + worker.terminate(); + reject(new Error(event.message || `The ${request.tool} worker failed.`)); + }; + // `error` does not fire when a posted message cannot be deserialized, which + // would otherwise leave this promise pending forever. + const onMessageError = () => { + disarmAck(); + cleanup(); + worker.terminate(); + reject(new Error(`The ${request.tool} worker posted an undeserializable message.`)); + }; + const cleanup = () => { + worker.removeEventListener("message", onMessage); + worker.removeEventListener("error", onError); + worker.removeEventListener("messageerror", onMessageError); + }; + const attach = () => { + worker.addEventListener("message", onMessage); + worker.addEventListener("error", onError); + worker.addEventListener("messageerror", onMessageError); + }; + // The input files are structured-cloned rather than transferred: these + // wrappers do not otherwise take ownership of the caller's bytes, and a + // neutered input array would be a trap the callers don't set. + const post = () => { + try { + worker.postMessage(request); + armAck(); + } catch (error) { + // A throw here (e.g. DataCloneError) rejects the promise on its own, but + // the worker is already spawned and would leak without this. + disarmAck(); + cleanup(); + worker.terminate(); + reject(error instanceof Error ? error : new Error(String(error))); + } + }; + attach(); + post(); + }); +} + +/** + * Run a tool off the main thread where Workers exist, inline where they do not + * (node, tests). The inline path is why the callers still expose an explicit + * wasm-source init (`initConvertTools`): a worker resolves its own bundled copy + * instead, in its own module scope. + * + * @param request - The tool id, CLI args, and files to place under `/work`. + * @returns The tool's exit code, captured output, and the files it wrote. + */ +export async function runWasmToolInBackground(request: WasmToolRequest): Promise { + if (typeof Worker === "undefined") { + const { runTool } = await import("geolibre-wasm/tools"); + return runTool(request.tool, { args: request.args, input: request.input }); + } + return runToolOnWorker(request); +} diff --git a/packages/processing/src/wasm-tool.worker.ts b/packages/processing/src/wasm-tool.worker.ts new file mode 100644 index 000000000..54b0455cc --- /dev/null +++ b/packages/processing/src/wasm-tool.worker.ts @@ -0,0 +1,58 @@ +/// +import { runTool } from "geolibre-wasm/tools"; +import type { ToolResult } from "geolibre-wasm/tools"; + +// Runs `geolibre-wasm/tools` WASI tools off the main thread. The runner has no +// yield points once a tool starts, so on the main thread a long job freezes the +// whole UI for its duration — tiling a country-scale vector layer to zoom 14 is +// minutes of that, and dissolving 290 polygons by an attribute is ~60s, which is +// why wasm-convert.ts and wasm-client.ts both route through wasm-tool-runner.ts. +// +// One run at a time, but reusable across runs: the caller parks this worker for +// its next call instead of terminating it, so the ~23 MB `geolibre-cli.wasm` +// compile in this worker's module scope is paid once rather than per run. Each +// run still gets a fresh WASI instance and a fresh /work from `runTool`, so +// nothing carries over between them. +const worker = self as unknown as DedicatedWorkerGlobalScope; + +/** A tool to run: its id, CLI args, and the files to place under /work. */ +export interface WasmToolRequest { + tool: string; + args: string[]; + input: Record; +} + +/** + * What this worker posts back: an immediate `ack` on receipt, then one terminal + * message when the tool finishes. + * + * The ack is what lets the caller tell a live parked worker from one that died + * while idle. Nothing here can report that: a worker killed out of band fires + * no `error` event, and posting to it silently does nothing, so without an ack + * a reused-but-dead worker would leave the run pending forever. + */ +export type WasmToolResponse = + | { ok: "ack" } + | { ok: true; result: ToolResult } + | { ok: false; error: string }; + +worker.addEventListener("message", async (event: MessageEvent) => { + const { tool, args, input } = event.data; + // Before the run, so it lands even though the WASI call that follows blocks + // this worker's thread until the tool is done. + worker.postMessage({ ok: "ack" } satisfies WasmToolResponse); + try { + // runTool compiles the bundled geolibre-cli.wasm on first use, in this + // worker's own module scope — a copy already compiled on the main thread + // is not shared with it. + const result = await runTool(tool, { args, input }); + worker.postMessage({ ok: true, result } satisfies WasmToolResponse); + } catch (error) { + // A tool that merely exits non-zero resolves normally and is reported by + // the caller; reaching here means the runner itself threw. + worker.postMessage({ + ok: false, + error: error instanceof Error ? error.message : String(error), + } satisfies WasmToolResponse); + } +}); diff --git a/tests/wasm-convert.test.ts b/tests/wasm-convert.test.ts index 44efa7a88..aa24124b2 100644 --- a/tests/wasm-convert.test.ts +++ b/tests/wasm-convert.test.ts @@ -9,6 +9,7 @@ import { renderRasterToPmtiles, tileVectorToPmtiles, } from "../packages/processing/src/wasm-convert"; +import { releaseIdleWasmToolWorkers } from "../packages/processing/src/wasm-tool-runner"; const fixture = (name: string) => new Uint8Array(readFileSync(fileURLToPath(new URL(`./fixtures/${name}`, import.meta.url)))); @@ -385,6 +386,13 @@ describe("wasm-convert", () => { this.listeners.set(type, [...(this.listeners.get(type) ?? []), fn]); } + removeEventListener(type: string, fn: (event: unknown) => void): void { + this.listeners.set( + type, + (this.listeners.get(type) ?? []).filter((it) => it !== fn), + ); + } + postMessage(message: unknown): void { if (FakeWorker.postMessageError) throw FakeWorker.postMessageError; this.posted.push(message); @@ -408,8 +416,11 @@ describe("wasm-convert", () => { }); afterEach(() => { + // A worker parked for reuse would otherwise be handed to the next case, + // which then spawns none and sees an empty `instances`. + releaseIdleWasmToolWorkers(); // Leaving the stub installed would push the inline tests above onto the - // worker path, where `new URL("./wasm-convert.worker.ts")` cannot load. + // worker path, where `new URL("./wasm-tool.worker.ts")` cannot load. if (!hadWorker) delete (globalThis as { Worker?: unknown }).Worker; }); @@ -456,26 +467,91 @@ describe("wasm-convert", () => { }); // Vite only bundles the worker when it can statically see this exact shape. + // The script is shared with the Whitebox toolbox runner (wasm-client.ts), + // which is why it is named for tools rather than for conversion. it("loads the worker as an ES module", () => { const { worker } = startRun(); - assert.match(worker.url.href, /wasm-convert\.worker\.ts$/); + assert.match(worker.url.href, /wasm-tool\.worker\.ts$/); assert.deepEqual(worker.options, { type: "module" }); }); - it("resolves with the worker's result and terminates it", async () => { + it("resolves with the worker's result and parks it for reuse", async () => { const { promise, worker } = startRun(); worker.emit("message", { data: { ok: true, result: okResult } }); const result = await promise; assert.deepEqual(result.data, Uint8Array.from(PMTILES_MAGIC)); assert.deepEqual(result.messages, ["packing PMTiles archive"]); - assert.equal(worker.terminated, true, "the worker should not leak"); + // Parked, not terminated: a worker compiles the ~23 MB wasm in its own + // module scope, so discarding it makes the next run pay that again. + assert.equal(worker.terminated, false, "a healthy worker should be reused"); + }); + + // The compile is per worker, so the second run must not spawn a second one. + it("reuses the parked worker for the next run", async () => { + const first = startRun(); + first.worker.emit("message", { data: { ok: true, result: okResult } }); + await first.promise; + const second = startRun(); + assert.equal(FakeWorker.instances.length, 1, "the second run should reuse the first worker"); + assert.equal(second.worker, first.worker); + // Its listeners are removed on the way out, so the reused worker answers + // the new run once rather than resolving the previous promise again. + assert.equal(second.worker.listeners.get("message")?.length, 1); + second.worker.emit("message", { data: { ok: true, result: okResult } }); + await second.promise; + }); + + // A worker killed while parked fires no `error` and swallows postMessage, so + // without the ack the run would stay pending forever. It is replaced instead. + it("replaces a parked worker that never acknowledges the request", async (t) => { + t.mock.timers.enable({ apis: ["setTimeout"] }); + const first = startRun(); + first.worker.emit("message", { data: { ok: true, result: okResult } }); + await first.promise; + + const second = startRun(); + assert.equal(second.worker, first.worker, "it should try the parked worker first"); + t.mock.timers.tick(10_000); + + assert.equal(first.worker.terminated, true, "the silent worker should be terminated"); + assert.equal(FakeWorker.instances.length, 2, "a replacement should be spawned"); + const replacement = FakeWorker.instances[1]; + assert.deepEqual( + replacement.posted, + [first.worker.posted[0]], + "the request should be re-sent", + ); + + // The replacement answers, and the run resolves as if nothing happened. + replacement.emit("message", { data: { ok: true, result: okResult } }); + const result = await second.promise; + assert.deepEqual(result.data, Uint8Array.from(PMTILES_MAGIC)); }); - it("rejects with the error the worker reports", async () => { + // A live parked worker acks, so it is kept rather than replaced. + it("keeps a parked worker that acknowledges the request", async (t) => { + t.mock.timers.enable({ apis: ["setTimeout"] }); + const first = startRun(); + first.worker.emit("message", { data: { ok: true, result: okResult } }); + await first.promise; + + const second = startRun(); + second.worker.emit("message", { data: { ok: "ack" } }); + t.mock.timers.tick(10_000); + assert.equal(FakeWorker.instances.length, 1, "no replacement should be spawned"); + assert.equal(second.worker.terminated, false); + + second.worker.emit("message", { data: { ok: true, result: okResult } }); + await second.promise; + }); + + it("rejects with the error the worker reports and still parks it", async () => { const { promise, worker } = startRun(); worker.emit("message", { data: { ok: false, error: "runner exploded" } }); await assert.rejects(promise, /runner exploded/); - assert.equal(worker.terminated, true); + // A tool that failed inside a healthy worker says nothing about the + // worker: the runner caught it and answered, so it is still reusable. + assert.equal(worker.terminated, false); }); it("rejects when the worker itself fails", async () => {