Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 33 additions & 13 deletions packages/cli/src/browser/chromeOrphans.process.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ import {
const here = dirname(fileURLToPath(import.meta.url));
const cli = join(here, "..", "cli.ts");

function project(busy: boolean): string {
function project(busy: boolean, seconds = 1): string {
const dir = mkdtempSync(join(tmpdir(), "hf-chrome-orphans-"));
onTestFinished(() => rm(dir, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 }));
mkdirSync(join(dir, "compositions"));
writeFileSync(
join(dir, "index.html"),
`<!doctype html><html><body><div data-composition-id="main" data-width="320" data-height="180" data-duration="1">
<div data-composition-id="scene" data-composition-src="compositions/scene.html" data-start="0" data-duration="1"></div></div>
<script>window.__timelines={main:{duration:()=>1,seek:()=>{},pause:()=>{}}};</script></body></html>`,
`<!doctype html><html><body><div data-composition-id="main" data-width="320" data-height="180" data-duration="${seconds}">
<div data-composition-id="scene" data-composition-src="compositions/scene.html" data-start="0" data-duration="${seconds}"></div></div>
<script>window.__timelines={main:{duration:()=>${seconds},seek:()=>{},pause:()=>{}}};</script></body></html>`,
);
const script = busy
? `const el=document.getElementById("box");while(el.offsetWidth<2){el.style.height="1px";}`
Expand All @@ -35,15 +35,16 @@ function project(busy: boolean): string {
return dir;
}

function check(dir: string, entry = cli, env: Record<string, string> = {}) {
const child = spawn(
process.execPath,
["--import", "tsx", entry, "check", dir, "--json", "--no-browser-gpu", "--samples=1"],
{
env: { ...process.env, CI: "1", HYPERFRAMES_NO_TELEMETRY: "1", ...env },
stdio: ["ignore", "ignore", "ignore"],
},
);
function check(
dir: string,
entry = cli,
env: Record<string, string> = {},
command = ["check", dir, "--json", "--no-browser-gpu", "--samples=1"],
) {
const child = spawn(process.execPath, ["--import", "tsx", entry, ...command], {
env: { ...process.env, CI: "1", HYPERFRAMES_NO_TELEMETRY: "1", ...env },
stdio: ["ignore", "ignore", "ignore"],
});
const seen = { chrome: new Set<number>(), watchdog: new Set<number>() };
const record = () => {
for (const pid of testProcessDescendants(child.pid!, /chrom/i)) seen.chrome.add(pid);
Expand Down Expand Up @@ -89,3 +90,22 @@ it("leaves no Chrome process or watchdog behind after a check that finishes", as
10_000,
);
}, 90_000);

it("leaves no Chrome process behind when the CLI is killed mid-render", async () => {
const dir = project(false, 30);
const run = check(dir, cli, {}, [
"render",
dir,
"--output",
join(dir, "out.mp4"),
"--quality",
"draft",
]);
await waitForTestCondition(() => {
run.record();
return run.seen.chrome.size >= 3;
}, 60_000);
expect(alive(run.seen.chrome).length).toBeGreaterThan(0);
run.child.kill("SIGKILL");
await waitForTestCondition(() => alive(run.seen.chrome).length === 0, 30_000);
}, 120_000);
3 changes: 3 additions & 0 deletions packages/cli/src/utils/cancellableProcess.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { spawn, type ChildProcess, type SpawnOptions } from "node:child_process";
import { fileURLToPath } from "node:url";
import { killWithThisProcess } from "./childReaper.js";
import { terminateProcessTree } from "./processTree.js";
import { renderSetupFailureFrom, renderSetupResultFrom } from "../renderSetupWorkerLifecycle.js";

Expand Down Expand Up @@ -75,6 +76,7 @@ export function runCancellableProcess(
return;
}

const release = child.pid ? killWithThisProcess(child.pid, detached) : undefined;
if (child.pid) options.onSpawn?.(child.pid);
let stdout = "";
let stderr = "";
Expand Down Expand Up @@ -127,6 +129,7 @@ export function runCancellableProcess(
child.once("error", (error) => void finishReject(error));
// fallow-ignore-next-line complexity
child.once("close", async (status, closeSignal) => {
release?.();
if (settled) return;
if (terminationTask) terminationSignal = await terminationTask.catch(() => terminationSignal);
if (settled) return;
Expand Down
25 changes: 25 additions & 0 deletions packages/cli/src/utils/childReaper.fixture.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// fallow-ignore-file unused-file
import { writeFileSync } from "node:fs";
import { runCancellableProcess } from "./cancellableProcess.js";

const [readyPath] = process.argv.slice(2);
if (!readyPath) throw new Error("Missing fixture arguments");

// Acts like a synchronous lint: its event loop never turns, so no handler in it can notice the parent go.
const childSource = [
'import { spawn } from "node:child_process";',
'import { writeFileSync } from "node:fs";',
'process.on("SIGTERM", () => {});',
'const grandchild = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { stdio: "ignore" });',
"writeFileSync(process.argv[1], JSON.stringify({ child: process.pid, grandchild: grandchild.pid }));",
"Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0);",
].join("\n");

void runCancellableProcess(
process.execPath,
["--input-type=module", "--eval", childSource, `${readyPath}.child`],
{
onSpawn: () => writeFileSync(readyPath, String(process.pid)),
},
).catch(() => undefined);
setInterval(() => undefined, 1000);
49 changes: 49 additions & 0 deletions packages/cli/src/utils/childReaper.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { spawn, type ChildProcess } from "node:child_process";
import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { dirname, join } from "node:path";
import { fileURLToPath } from "node:url";
import { afterEach, describe, expect, it } from "vitest";
import { testProcessIsAlive, waitForTestCondition } from "./processTestUtils.js";

const fixturePath = join(dirname(fileURLToPath(import.meta.url)), "childReaper.fixture.ts");
const leftovers: number[] = [];
let parent: ChildProcess | undefined;

afterEach(() => {
parent?.kill("SIGKILL");
for (const pid of leftovers.splice(0)) {
try {
process.kill(pid, "SIGKILL");
} catch {}
}
});

describe.skipIf(process.platform === "win32")("a CLI killed outright", () => {
it("takes every process it started with it, a child stuck in synchronous work included", async () => {
const dir = mkdtempSync(join(tmpdir(), "hyperframes-child-reaper-"));
const readyPath = join(dir, "ready");
try {
parent = spawn(process.execPath, ["--import", "tsx", fixturePath, readyPath], {
stdio: "ignore",
});
await waitForTestCondition(() => existsSync(`${readyPath}.child`), 15_000);
await waitForTestCondition(
() => readFileSync(`${readyPath}.child`, "utf8").endsWith("}"),
5_000,
);
const { child, grandchild } = JSON.parse(readFileSync(`${readyPath}.child`, "utf8")) as {
child: number;
grandchild: number;
};
leftovers.push(child, grandchild);
expect([child, grandchild].every(testProcessIsAlive)).toBe(true);

parent.kill("SIGKILL");

await waitForTestCondition(() => ![child, grandchild].some(testProcessIsAlive), 5_000);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
});
48 changes: 48 additions & 0 deletions packages/cli/src/utils/childReaper.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { spawn, type ChildProcess } from "node:child_process";

// Ends with this process's end of its stdin, however this process died, and kills what is still held.
const REAPER = `const held = new Map();
let rest = "";
const kill = (pid, group) => {
try {
if (process.platform === "win32")
require("node:child_process").spawnSync("taskkill", ["/PID", pid, "/T", "/F"], { windowsHide: true, timeout: 10000 });
else process.kill(group ? -Number(pid) : Number(pid), "SIGKILL");
} catch {}
};
process.stdin.on("data", (data) => {
const lines = (rest + data).split("\\n");
rest = lines.pop();
for (const [op, pid, group] of lines.map((line) => line.split(" ")))
op === "+" ? held.set(pid, group === "g") : held.delete(pid);
});
const done = () => {
for (const [pid, group] of held) kill(pid, group);
process.exit(0);
};
process.stdin.on("end", done);
process.stdin.on("error", done);`;

let reaper: ChildProcess | undefined;

function lifeline(): NodeJS.WritableStream | null {
if (!reaper) {
reaper = spawn(process.execPath, ["-e", REAPER], {
detached: true,
stdio: ["pipe", "ignore", "ignore"],
windowsHide: true,
});
reaper.on("error", () => {});
reaper.stdin?.on("error", () => {});
reaper.unref();
(reaper.stdin as unknown as { unref?: () => void } | null)?.unref?.();
}
return reaper.stdin;
}

/** Kills `pid`, its whole process group when `group`, if this process ends before the returned release runs. */
export function killWithThisProcess(pid: number, group: boolean): () => void {
const stdin = lifeline();
stdin?.write(`+ ${pid} ${group ? "g" : "p"}\n`);
return () => void stdin?.write(`- ${pid}\n`);
}
6 changes: 3 additions & 3 deletions packages/engine/src/services/browserManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export function setHostHandlesSigint(owned: boolean): void {
hostHandlesSigint = owned;
}

export const sigintLaunchOptions = () => ({ handleSIGINT: !hostHandlesSigint });
export const ownedLaunchOptions = () => ({ handleSIGINT: !hostHandlesSigint, pipe: true });

interface WebGlProbeInfo {
hasWebGL: boolean;
Expand Down Expand Up @@ -110,7 +110,7 @@ async function probeHardwareWebGlInfo(
let probeBrowser: Browser | undefined;
try {
const launchOptions: LaunchOptions = {
...sigintLaunchOptions(),
...ownedLaunchOptions(),
headless: true,
args: withHyperframesBrowserMarker(options.args),
defaultViewport: { width: 64, height: 64 },
Expand Down Expand Up @@ -886,7 +886,7 @@ async function launchBrowser(
let browser: Browser | undefined;
const launch = (args: readonly string[]) =>
ppt.launch({
...sigintLaunchOptions(),
...ownedLaunchOptions(),
headless: true,
args: withHyperframesBrowserMarker(args),
defaultViewport: null,
Expand Down
4 changes: 2 additions & 2 deletions packages/engine/src/services/hdrCapture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

import { withHyperframesBrowserMarker } from "@hyperframes/core/browser-marker";
import type { Page, Browser, PuppeteerNode } from "puppeteer-core";
import { resolveFullChromePath, sigintLaunchOptions } from "./browserManager.js";
import { resolveFullChromePath, ownedLaunchOptions } from "./browserManager.js";

// ── PQ (SMPTE 2084) OETF ─────────────────────────────────────────────────────

Expand Down Expand Up @@ -247,7 +247,7 @@ export async function launchHdrBrowser(
}

const browser = await ppt.launch({
...sigintLaunchOptions(),
...ownedLaunchOptions(),
headless: false,
executablePath: chromePath,
args: withHyperframesBrowserMarker(buildHdrChromeArgs(width, height)),
Expand Down
Loading