diff --git a/scripts/build-web-client.ts b/scripts/build-web-client.ts index 5fdc1a5d..ac6f4429 100644 --- a/scripts/build-web-client.ts +++ b/scripts/build-web-client.ts @@ -62,12 +62,13 @@ import { ensure, main, scoped, until } from "effection"; import type { Operation } from "effection"; -import { exec } from "@effectionx/process"; import { ensureDir, readTextFile, rm, writeTextFile } from "@effectionx/fs"; import { encodeBase64 } from "@std/encoding/base64"; -import { resolve } from "node:path"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; import { pathToFileURL } from "node:url"; +import { containedRun } from "./lib/contained-run.ts"; import { assertSideEffectFree, sideEffectFreeManifests } from "./lib/side-effect-free.ts"; import { byteLength, generatedModule } from "./lib/web-client-module.ts"; @@ -113,7 +114,12 @@ const FONT_FACES: FontFace[] = [ export const SIDE_EFFECT_FREE_MANIFESTS: URL[] = sideEffectFreeManifests(repoRoot); function* run(command: string, args: string[]): Operation { - yield* exec(command, { arguments: args, cwd: new URL(repoRoot).pathname }).expect(); + const exit = yield* containedRun(command, args, { cwd: new URL(repoRoot).pathname }); + if (exit.code !== 0) { + throw new Error( + `${command} ${args.join(" ")} exited with ${exit.code ?? exit.signal}\n${exit.stderr}`, + ); + } } /** @@ -122,11 +128,20 @@ function* run(command: string, args: string[]): Operation { * the battery's own per-command deadline settles a wedged command once and * reports it, so a worsening upstream defect stays visible instead of being * absorbed by an attempt count. + * + * The output path is chosen without touching the filesystem, and its removal + * is registered before the bundler starts. An interrupted build therefore has + * nothing on disk to leak before the bundler exists, and once it does, the + * removal runs strictly after `run`'s teardown has terminated and joined the + * bundler's process tree — nothing survives that could recreate the file. + * Creating the file up front instead would open a window where an abandoned + * in-flight creation lands after teardown, with no removal registered to + * cover it. */ function* bundleClient(scratch?: string): Operation { return yield* scoped(function* () { - const output = yield* until(Deno.makeTempFile({ dir: scratch, suffix: ".js" })); - yield* ensure(() => rm(output)); + const output = join(scratch ?? tmpdir(), `${crypto.randomUUID()}.js`); + yield* ensure(() => rm(output, { force: true })); yield* run(Deno.execPath(), [ "bundle", "--platform=browser", diff --git a/scripts/lib/contained-run.ts b/scripts/lib/contained-run.ts new file mode 100644 index 00000000..2f32a795 --- /dev/null +++ b/scripts/lib/contained-run.ts @@ -0,0 +1,82 @@ +/** + * Run a command so its process tree cannot outlive the calling scope. + * + * `exec` from @effectionx/process suspends between creating the child process + * and registering the teardown that terminates it, so a halt arriving in that + * window leaves the process running with nothing owning it. Here the + * terminate-and-join teardown is registered before the process exists and the + * process is created in the same synchronous continuation, so a halt lands + * either before there is anything to clean up or after the cleanup is armed — + * never between. + * + * The child is detached into its own process group and teardown signals the + * whole group, then joins the child's `close` event, which settles only when + * every holder of the child's piped stderr has exited — grandchildren + * included. Cleanup registered before this call therefore runs strictly after + * the tree is gone. + */ + +import { spawn } from "node:child_process"; +import { Buffer } from "node:buffer"; +import process from "node:process"; +import { ensure, withResolvers } from "effection"; +import type { Operation } from "effection"; + +export interface ContainedExit { + code: number | null; + signal: string | null; + stderr: string; +} + +interface Exit { + code: number | null; + signal: string | null; +} + +export function* containedRun( + command: string, + args: string[], + options: { cwd: string }, +): Operation { + const closed = withResolvers(); + let pid: number | undefined; + let exited = false; + let failed: Error | undefined; + yield* ensure(function* () { + if (pid === undefined) { + return; + } + if (!exited) { + try { + process.kill(-pid, "SIGTERM"); + } catch { + // the group ended between the exit observation and the signal + } + } + yield* closed.operation; + }); + const child = spawn(command, args, { + cwd: options.cwd, + detached: true, + stdio: ["ignore", "ignore", "pipe"], + }); + const stderr: Uint8Array[] = []; + if (child.stderr) { + child.stderr.on("data", (chunk: Uint8Array) => stderr.push(chunk)); + } + child.once("error", (error: Error) => { + exited = true; + failed = error; + closed.resolve({ code: null, signal: null }); + }); + child.once("close", (code: number | null, signal: string | null) => { + exited = true; + closed.resolve({ code, signal }); + }); + pid = child.pid; + const exit = yield* closed.operation; + if (failed) { + throw failed; + } + return { ...exit, stderr: Buffer.concat(stderr).toString("utf8") }; +} diff --git a/scripts/runtime-test-exclusions.ts b/scripts/runtime-test-exclusions.ts index 9a63b7fd..37ba5fcd 100644 --- a/scripts/runtime-test-exclusions.ts +++ b/scripts/runtime-test-exclusions.ts @@ -45,7 +45,13 @@ const DENO_ONLY_TOOLING: RuntimeExclusion[] = [ { path: "scripts/tests/build-web-client.test.ts", reason: - "subject is scripts/build-web-client.ts, which runs `deno bundle` and calls Deno.execPath()/makeTempFile — Deno-only", + "subject is scripts/build-web-client.ts, which runs `deno bundle` and calls Deno.execPath() — Deno-only", + issue: DERIVED_SCOPE, + }, + { + path: "scripts/tests/contained-run.test.ts", + reason: + "subject is scripts/lib/contained-run.ts, the process containment behind the Deno-only browser bundle build; the fixtures drive real process trees under Deno.execPath()", issue: DERIVED_SCOPE, }, { diff --git a/scripts/tests/contained-run.test.ts b/scripts/tests/contained-run.test.ts new file mode 100644 index 00000000..00b334a7 --- /dev/null +++ b/scripts/tests/contained-run.test.ts @@ -0,0 +1,94 @@ +import { describe, it } from "@executablemd/test-support/bdd"; +import { expect } from "@executablemd/test-support/expect"; +import { ensure, scoped, spawn } from "effection"; +import type { Operation } from "effection"; +import { rm } from "@effectionx/fs"; +import { when } from "@effectionx/converge"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { containedRun } from "../lib/contained-run.ts"; + +const CWD = new URL("../../", import.meta.url).pathname; + +/** A directory of the calling operation's own, gone when that operation shuts down. */ +function* scratchDirectory(prefix: string): Operation { + // @effectionx/fs has no mkdtemp. + const base = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + yield* ensure(() => rm(base, { recursive: true, force: true })); + return base; +} + +describe("contained-run", () => { + it("returns the exit status and captured stderr of a failed command", function* () { + const exit = yield* containedRun( + Deno.execPath(), + ["eval", "console.error('boom'); Deno.exit(3);"], + { cwd: CWD }, + ); + + expect(exit.code).toBe(3); + expect(exit.stderr).toContain("boom"); + }); + + it("completes a successful command", function* () { + const exit = yield* containedRun(Deno.execPath(), ["eval", "Deno.exit(0);"], { cwd: CWD }); + + expect(exit).toEqual({ code: 0, signal: null, stderr: "" }); + }); + + /** + * The property `bundleClient` stands on: cleanup registered before the + * command starts must observe the command's whole process tree already + * gone — a grandchild included, because the bundler's own grandchild is + * the esbuild service that writes the output file. + * + * The grandchild inherits the child's piped stderr and would run forever, + * so the scope releasing at all proves the terminate reached the whole + * group, and the marker it writes while shutting down proves the join + * finished before the earlier-registered cleanup ran. + */ + it("terminates and joins the whole tree before earlier-registered cleanup runs", function* () { + const base = yield* scratchDirectory("contained-run-"); + const ready = path.join(base, "ready"); + const terminated = path.join(base, "terminated"); + const grandchild = ` + Deno.writeTextFileSync(${JSON.stringify(ready)}, "ready"); + Deno.addSignalListener("SIGTERM", () => { + Deno.writeTextFileSync(${JSON.stringify(terminated)}, "terminated"); + Deno.exit(0); + }); + setInterval(() => {}, 1000); + `; + const child = ` + const grandchild = new Deno.Command(Deno.execPath(), { + args: ["eval", ${JSON.stringify(grandchild)}], + stdin: "null", + stdout: "null", + stderr: "inherit", + }).spawn(); + await grandchild.status; + `; + const observed: string[] = []; + + yield* scoped(function* () { + yield* ensure(() => { + observed.push(fs.existsSync(terminated) ? "tree gone" : "tree still alive"); + }); + yield* spawn(function* () { + yield* containedRun(Deno.execPath(), ["eval", child], { cwd: CWD }); + }); + yield* when( + function* () { + if (!fs.existsSync(ready)) { + throw new Error("the fixture tree has not started"); + } + }, + { timeout: 30_000 }, + ); + }); + + expect(observed).toEqual(["tree gone"]); + }); +});