commit 799d86fe7b2ca37ac90dc790e1b533b23ed84314 parent 83a0a65cf892ae5cfe0fdd7e85842ce70516c1f9 Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st> Date: Fri, 7 Aug 2026 16:27:38 -0400 Stop e2e fixture binaries from outliving the run that spawned them A full suite once left 27 orphaned fake-ytdlp processes alive, each spinning ~29% of a core (800%/27 — they share all 8 threads). Load hit 46 and the suite went from 30 min with 1 failure to 1.3 h with 13, spread across unrelated specs and every one of them pure contention. A test suite that lies about what broke is the real cost here, not the wasted CPU. The cause was not interrupted runs, as long assumed. /api/test/invalidate-cache nulled globalThis.__yttJobRegistry__ without cancelling anything first, and that registry holds the ONLY reachable kill path for an in-flight child: yt-dlp-style jobs run via runManagedFunction, which sets record.abortController but never record.child, so cancel()'s child.kill() branch is a no-op and abort() is all there is. resetData()/writeSettings() call that route — 303 resetData() calls across 83 of 87 spec files, many mid-test — so this leaked on every run. Four layers, because each covers a case the others cannot: 1. invalidate-cache cancels + forceReleases every queued/running job before dropping the singletons. Reads the global directly: getRegistry() CONSTRUCTS on read, so using it would build an empty registry and cancel nothing. 2. common/jobs/shutdownCancel.ts, lazy-imported by instrumentation.ts, cancels live jobs on SIGTERM/SIGINT. It has to be a separate module — Next statically scans instrumentation.ts for the Edge bundle and rejects process.once/kill/ listenerCount there, even though the NEXT_RUNTIME guard means they never run. 3. e2e/fixtures/bin/_watchdog.mjs, imported by all nine fakes: self-terminate when orphaned or past a lifetime backstop. Detects a PPID CHANGE rather than ppid === 1, because a systemd-user subreaper adopts orphans here and the pid-1 test would silently never fire. Both timers are unref'd, or the watchdog would itself become the leak. 4. globalSetup/globalTeardown sweeps (e2e/fixtureProcs.ts) as a net, matching argv[1] under the absolute fixtures/bin path — `pgrep -f fixtures/bin` is unsafe, it matches the sweeping shell's own command line. Falsified before trusting it: with layer 1 reverted, the new regression spec fails ("fixture child outlived the registry wipe") and the teardown sweep reaps and logs the stray it leaves behind — so both the detector and the net are known to work, not merely present. Verification: 434/435 e2e in 22.6 min (down from a 30.1 min baseline), zero strays afterwards, and zero sweep log lines — the first three layers held, so the net caught nothing, which is the intended outcome. Editor build clean with no Edge-runtime warnings; common + editor tsc clean. The single failure, jobs-batch-tasks-drain "hard Cancel during a drain", is pre-existing flake: it fails 2 of 3 with every change in this commit reverted, and fails more on a fast idle box because the batch finishes before the Cancel click lands. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Diffstat:
18 files changed, 468 insertions(+), 0 deletions(-)
diff --git a/common/jobs/shutdownCancel.ts b/common/jobs/shutdownCancel.ts @@ -0,0 +1,63 @@ +// SIGTERM every child we still own when the server shuts down gracefully. +// +// WHY THIS IS ITS OWN MODULE, and not inline in editor/instrumentation.ts: +// `register()` runs in every runtime, and Next statically analyses that file for +// the EDGE bundle. Node APIs there (`process.once`, `process.kill`, +// `process.listenerCount`) are flagged as unsupported and the file fails to +// compile — the `NEXT_RUNTIME !== "nodejs"` early return happens at runtime, and +// the bundler's static scan does not know about it. Keeping the Node-only code +// behind a lazy `await import()` inside that guard is the idiom instrumentation.ts +// already uses for the heartbeat, worker pool and digest sweep. +// +// WHAT IT FIXES: a spawned yt-dlp/whisper/parakeet child otherwise outlives the +// server. Route handlers run in a Next-spawned worker, so when that worker dies +// the child reparents and keeps burning CPU. The registry holds the only handle +// that can reach it — runManagedFunction sets `record.abortController` but never +// `record.child` (controller/../jobs/streamCommand.ts), so `cancel()`'s +// `child.kill()` branch is a no-op for those jobs and `abort()` is the only kill +// path. That handle dies with the process unless we use it first. +// +// HONEST LIMIT: graceful shutdown only. A SIGKILLed server still orphans its +// children because nothing in-process gets to run — that case is covered by the +// fixtures' own watchdog (editor/e2e/fixtures/bin/_watchdog.mjs) in tests, and is +// simply unavoidable for a real `kill -9` in production. + +// Set once the handlers are armed so a second register() — or a second signal — +// cannot re-enter. register() is documented as running once per server instance, +// but this is cheap and keeps shutdown re-entrancy-free. +let shutdownArmed = false; + +export function armShutdownCancel(): void { + if (shutdownArmed) return; + shutdownArmed = true; + for (const signal of ["SIGTERM", "SIGINT"] as const) { + process.once(signal, () => { + try { + // Read the global directly: getRegistry() CONSTRUCTS on read, and + // building a registry during shutdown just to cancel nothing would be + // worse than useless. No registry means nothing ever ran. + const registry = globalThis.__yttJobRegistry__; + for (const job of registry?.list() ?? []) { + if (job.status !== "running" && job.status !== "queued") continue; + try { + registry?.cancel(job.id); + } catch { + /* one wedged job must not block the rest, or the exit */ + } + } + } catch { + /* shutdown must proceed even if the registry is in a bad state */ + } + // No await, no delay: abort() fires execa's cancelSignal inline, so the + // SIGTERMs are already delivered by the time we get here. This runs on + // every ordinary dev restart, so it must not add latency. + // + // Re-raise only if nobody else is listening. `once` has already removed + // this listener, so a remaining count means another handler (Next's own) + // owns the exit — re-raising then would run that handler a second time. + if (process.listenerCount(signal) === 0) { + process.kill(process.pid, signal); + } + }); + } +} diff --git a/editor/app/api/test/invalidate-cache/route.ts b/editor/app/api/test/invalidate-cache/route.ts @@ -4,6 +4,39 @@ import { resetSnapshotScheduler } from "yt-dlp-transcript-common/jobs/snapshotSc export const dynamic = "force-dynamic"; +// Cancel everything still live BEFORE the registry is dropped. +// +// The registry holds the only reachable AbortController for an in-flight child. +// yt-dlp-style jobs run via runManagedFunction, which sets record.abortController +// but never record.child (common/jobs/streamCommand.ts:306-319) — so abort() is +// their ONLY kill path, and the moment the singleton is nulled the child is +// unkillable by the app and reparents to init when the Next worker dies. +// +// Read the global DIRECTLY rather than calling getRegistry(): per this repo's +// Next-16 lazy-singleton rule, getRegistry() *constructs* on read, so using it +// here would build a fresh empty registry and dutifully cancel nothing. Reading +// the (already globally-declared) var also means this route imports no registry +// module, so it cannot construct one as an import side-effect either. +function cancelLiveJobs() { + const registry = globalThis.__yttJobRegistry__; + if (!registry) return; + for (const job of registry.list()) { + if (job.status !== "running" && job.status !== "queued") continue; + try { + // Synchronous by design: abort() fires execa's cancelSignal inline, so + // SIGTERM is delivered before this route returns. execa's default + // forceKillAfterDelay escalates to SIGKILL, so we need not await the child. + registry.cancel(job.id); + // Then free the scheduler slot even if the record never settles (a child + // that ignores SIGTERM would otherwise leave a phantom busy slot behind + // for the next spec). Both calls are idempotent. + registry.forceRelease(job.id); + } catch { + /* one wedged job must not stop us cancelling the rest */ + } + } +} + // E2E test harness only. Mounted unconditionally so it's reachable from the // dev:test script; the editor is intended for localhost use, not deployment. export async function POST() { @@ -18,6 +51,11 @@ function invalidate() { // Clear any pending debounced snapshot regen FIRST (also clears its timer) so // it can't fire against the about-to-be-reset registry mid-spec. resetSnapshotScheduler(); + // Kill live children while we still have the handle that can kill them. This + // serves the same intent the wipe below was added for (4057f76: "tests don't + // observe stale jobs from a prior spec") rather than fighting it — a job whose + // process is gone is a great deal less observable than one that isn't. + cancelLiveJobs(); // Reset the in-memory job registry too so tests don't observe stale jobs // from a prior spec in the same dev-server lifetime. // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/editor/e2e/fixtureProcs.ts b/editor/e2e/fixtureProcs.ts @@ -0,0 +1,121 @@ +// Find (and reap) e2e fixture-binary processes that outlived the run that +// spawned them. +// +// Why this needs to be strict: these helpers kill processes, so a loose match is +// a footgun aimed at the developer's own machine. `pgrep -f fixtures/bin` is NOT +// safe — it matches any shell whose command line merely *mentions* the path, +// including the very command running the sweep. Verified on this box: +// +// a real fixture: argv[0]="node" argv[1]="<...>/e2e/fixtures/bin/fake-ytdlp.mjs" +// a shell that (path appears in argv[2], the -c script) +// mentions it: argv[0]="/usr/bin/zsh" +// +// So we match on argv[1] specifically — the script path node was handed via the +// fixtures' `#!/usr/bin/env node` shebang — and require it to sit under this +// checkout's absolute fixtures/bin directory. A shell can never satisfy that. +// +// Linux-only (reads /proc). That matches where this suite runs; on any other +// platform the scan returns nothing rather than guessing with `ps`, so a sweep +// degrades to a no-op instead of killing something it misidentified. + +import { readFile, readdir } from "node:fs/promises"; +import { dirname, resolve, basename } from "node:path"; +import { fileURLToPath } from "node:url"; + +const here = dirname(fileURLToPath(import.meta.url)); + +// Absolute, normalised, with a trailing separator so a startsWith() test cannot +// match a sibling directory whose name merely shares the prefix. +export const fixtureBinDir = resolve(here, "fixtures", "bin") + "/"; + +export type FixtureProc = { pid: number; script: string; argv: string[] }; + +async function readCmdline(pid: string): Promise<string[] | null> { + try { + const raw = await readFile(`/proc/${pid}/cmdline`, "utf8"); + // cmdline is NUL-separated with a trailing NUL; drop the empty tail. + const argv = raw.split("\0").filter((s) => s.length > 0); + return argv.length > 0 ? argv : null; + } catch { + // The process exited between readdir and readFile, or it isn't ours to + // inspect. Either way it is not a stray we can act on. + return null; + } +} + +// Every live process whose argv[1] is a script inside this checkout's +// e2e/fixtures/bin. Never includes the calling process. +export async function listFixtureProcesses(): Promise<FixtureProc[]> { + if (process.platform !== "linux") return []; + let entries: string[]; + try { + entries = await readdir("/proc"); + } catch { + return []; + } + const found: FixtureProc[] = []; + for (const entry of entries) { + if (!/^\d+$/.test(entry)) continue; + const pid = Number(entry); + if (pid === process.pid) continue; + const argv = await readCmdline(entry); + if (!argv) continue; + // argv[0] is the interpreter (the shebang resolves to node), argv[1] the + // fixture script. Requiring both is what makes this safe to kill on. + if (!basename(argv[0]).startsWith("node")) continue; + const script = argv[1]; + if (!script || !resolve(script).startsWith(fixtureBinDir)) continue; + found.push({ pid, script: resolve(script), argv }); + } + return found; +} + +// SIGTERM every stray, then SIGKILL whatever is still alive after `graceMs` +// (a fixture may deliberately ignore SIGTERM — fake-parakeet-stitch does, in +// `hangterm` dirs). Returns what it killed so callers can log it: a silent +// reaper would hide a regression in the layers that are supposed to prevent +// strays in the first place. +export async function killFixtureProcesses( + graceMs = 2_000, +): Promise<FixtureProc[]> { + const strays = await listFixtureProcesses(); + if (strays.length === 0) return []; + for (const p of strays) { + try { + process.kill(p.pid, "SIGTERM"); + } catch { + /* already gone */ + } + } + await new Promise((r) => setTimeout(r, graceMs)); + for (const p of strays) { + try { + process.kill(p.pid, "SIGKILL"); + } catch { + /* exited on SIGTERM, as it should have */ + } + } + return strays; +} + +// The sweep both the globalSetup and globalTeardown hooks run. +// +// It LOGS whatever it reaps, deliberately and loudly. A silent reaper would hide +// the thing we actually care about: strays are supposed to be impossible now +// (jobs are cancelled before the registry is dropped, on graceful shutdown, and +// each fixture self-terminates when orphaned). Anything this finds is a +// regression in one of those three layers, not a routine cleanup. +export async function sweepFixtureProcesses(phase: "setup" | "teardown") { + const killed = await killFixtureProcesses(); + if (killed.length === 0) return; + const names = killed + .map((p) => `${basename(p.script)}(${p.pid})`) + .join(", "); + const cause = + phase === "setup" + ? "left behind by an EARLIER run (a killed run cannot run its own teardown)" + : "survived this run — the anti-orphan layers did not hold"; + console.warn( + `[fixture-sweep:${phase}] reaped ${killed.length} stray fixture process(es) ${cause}: ${names}`, + ); +} diff --git a/editor/e2e/fixtures/bin/_watchdog.mjs b/editor/e2e/fixtures/bin/_watchdog.mjs @@ -0,0 +1,56 @@ +// Self-termination for the e2e fake binaries: no fixture should be able to +// outlive the run that spawned it, whatever happens to the server above it. +// +// The app-side fixes (cancelling live jobs before the registry is wiped, and on +// graceful shutdown) cover every case where something is still alive to send a +// signal. This covers the case where nothing is: a SIGKILLed dev server can't +// run shutdown code, so its children reparent and keep spinning. A full suite +// once left 27 of these alive at ~29% of a core each — enough to saturate the +// box, push load to 46, and turn a 30-minute suite with 1 failure into a +// 1.3-hour suite with 13 failures that all looked like real regressions. +// +// Two independent triggers, because neither alone is sufficient: +// +// 1. Reparenting — our parent died. Checked as "ppid changed since startup", +// NOT "ppid === 1": on a systemd-user box (this one) a child subreaper +// adopts orphans, so the pid-1 test would silently never fire. +// 2. A max-lifetime backstop, for the case where the parent is alive but +// wedged. The longest legitimate fixture life is 30s +// (fake-parakeet-stitch's completion backstop, and fake-ytdlp --test-slow, +// which jobs-retry/jobs-reorder/queues specs rely on), so the default 120s +// leaves 4x margin. Override with FIXTURE_MAX_LIFETIME_MS if a future +// fixture legitimately needs longer. +// +// BOTH timers are unref'd. That is the load-bearing detail: a ref'd timer would +// hold the event loop open and keep an otherwise-finished fixture alive — this +// module would then create exactly the leak it exists to prevent. + +const POLL_MS = 1_000; +const DEFAULT_MAX_LIFETIME_MS = 120_000; + +export function installFixtureWatchdog() { + const label = process.argv[1]?.split("/").pop() ?? "fixture"; + const initialPpid = process.ppid; + + const poll = setInterval(() => { + // process.ppid is re-read from the OS on each access, so this observes the + // reparent rather than a value cached at startup. + if (process.ppid !== initialPpid) { + process.stderr.write( + `[watchdog] ${label}: parent ${initialPpid} died (now ${process.ppid}) — exiting rather than orphaning\n`, + ); + process.exit(1); + } + }, POLL_MS); + poll.unref(); + + const maxLifetimeMs = + Number(process.env.FIXTURE_MAX_LIFETIME_MS) || DEFAULT_MAX_LIFETIME_MS; + const backstop = setTimeout(() => { + process.stderr.write( + `[watchdog] ${label}: exceeded ${maxLifetimeMs}ms lifetime — exiting\n`, + ); + process.exit(1); + }, maxLifetimeMs); + backstop.unref(); +} diff --git a/editor/e2e/fixtures/bin/fake-chough.mjs b/editor/e2e/fixtures/bin/fake-chough.mjs @@ -6,6 +6,10 @@ // { duration_seconds, chunks, text, chunk_data: [{ start_time, end_time, text }] } // with times in SECONDS. Run from cwd == the video dir (set by runWhisperBatch). import { writeFile } from "node:fs/promises"; +import { installFixtureWatchdog } from "./_watchdog.mjs"; + +// Never outlive the run that spawned us — see _watchdog.mjs. +installFixtureWatchdog(); const argv = process.argv.slice(2); function arg(flag) { diff --git a/editor/e2e/fixtures/bin/fake-claude.mjs b/editor/e2e/fixtures/bin/fake-claude.mjs @@ -16,6 +16,10 @@ // rather than on a schema. import { readFileSync } from "node:fs"; +import { installFixtureWatchdog } from "./_watchdog.mjs"; + +// Never outlive the run that spawned us — see _watchdog.mjs. +installFixtureWatchdog(); const argv = process.argv.slice(2); function flag(name) { diff --git a/editor/e2e/fixtures/bin/fake-diarize.mjs b/editor/e2e/fixtures/bin/fake-diarize.mjs @@ -20,6 +20,10 @@ // failure path without renaming dirs into ids the batch would never pick up. import { writeFile } from "node:fs/promises"; import path from "node:path"; +import { installFixtureWatchdog } from "./_watchdog.mjs"; + +// Never outlive the run that spawned us — see _watchdog.mjs. +installFixtureWatchdog(); const argv = process.argv.slice(2); function arg(flag) { diff --git a/editor/e2e/fixtures/bin/fake-ffmpeg.mjs b/editor/e2e/fixtures/bin/fake-ffmpeg.mjs @@ -9,6 +9,10 @@ // same w.r.t. corruption detection but doesn't actually write // output. import { readFile, writeFile } from "node:fs/promises"; +import { installFixtureWatchdog } from "./_watchdog.mjs"; + +// Never outlive the run that spawned us — see _watchdog.mjs. +installFixtureWatchdog(); const CORRUPT_MARKER = "__CORRUPT__"; // When the probed file contains this marker, the probe (and transcode) sleeps diff --git a/editor/e2e/fixtures/bin/fake-ffprobe.mjs b/editor/e2e/fixtures/bin/fake-ffprobe.mjs @@ -5,6 +5,10 @@ // file (`__DUR=<seconds>__`). A file with no marker reports a large duration so // the guard never falsely trips on it (matches a full-length download). import { readFile } from "node:fs/promises"; +import { installFixtureWatchdog } from "./_watchdog.mjs"; + +// Never outlive the run that spawned us — see _watchdog.mjs. +installFixtureWatchdog(); const argv = process.argv.slice(2); // The input file is the only non-flag argument (and not a value of -show_entries diff --git a/editor/e2e/fixtures/bin/fake-gallery-dl.mjs b/editor/e2e/fixtures/bin/fake-gallery-dl.mjs @@ -9,6 +9,10 @@ // // Deterministic by design: the same tweets every run, so a re-run proves the // incremental (no-duplicate) guarantee. +import { installFixtureWatchdog } from "./_watchdog.mjs"; + +// Never outlive the run that spawned us — see _watchdog.mjs. +installFixtureWatchdog(); const args = process.argv.slice(2); diff --git a/editor/e2e/fixtures/bin/fake-parakeet-stitch.mjs b/editor/e2e/fixtures/bin/fake-parakeet-stitch.mjs @@ -25,6 +25,10 @@ import { mkdir, writeFile, rm } from "node:fs/promises"; import { existsSync } from "node:fs"; import path from "node:path"; +import { installFixtureWatchdog } from "./_watchdog.mjs"; + +// Never outlive the run that spawned us — see _watchdog.mjs. +installFixtureWatchdog(); const argv = process.argv.slice(2); function arg(flag) { diff --git a/editor/e2e/fixtures/bin/fake-whisper.mjs b/editor/e2e/fixtures/bin/fake-whisper.mjs @@ -4,6 +4,10 @@ // Writes <tmpBase>.json next to cwd (which runWhisperBatch sets to the // video dir). Format matches what common/lib/whisper.ts parses. import { writeFile } from "node:fs/promises"; +import { installFixtureWatchdog } from "./_watchdog.mjs"; + +// Never outlive the run that spawned us — see _watchdog.mjs. +installFixtureWatchdog(); const argv = process.argv.slice(2); function arg(flag) { diff --git a/editor/e2e/fixtures/bin/fake-ytdlp.mjs b/editor/e2e/fixtures/bin/fake-ytdlp.mjs @@ -15,6 +15,10 @@ import { mkdir, writeFile, readFile, appendFile, stat, rename } from "node:fs/promises"; import { existsSync, openSync, writeSync, closeSync } from "node:fs"; import path from "node:path"; +import { installFixtureWatchdog } from "./_watchdog.mjs"; + +// Never outlive the run that spawned us — see _watchdog.mjs. +installFixtureWatchdog(); // Used in audio-check scenarios. The fake-ffmpeg companion treats files // containing this string as malformed when probing. diff --git a/editor/e2e/globalSetup.ts b/editor/e2e/globalSetup.ts @@ -0,0 +1,17 @@ +// Reap fixture strays left by an EARLIER run before this one starts. +// +// A run killed with SIGKILL (Ctrl-C twice, `timeout`, an OOM) never gets to run +// its own globalTeardown, so its strays are still burning CPU when the next run +// begins. That is precisely how 27 of them once accumulated and turned a +// 30-minute suite into a 1.3-hour one whose 13 failures were pure contention. +// +// ORDERING CAVEAT: Playwright starts the `webServer` entries BEFORE globalSetup +// (the same constraint export/e2e-2origin/globalSetup.ts documents at :151-152). +// So this cannot clear a stale dev server ahead of the new one booting — it only +// reaps fixture children, which is what actually eats the CPU. + +import { sweepFixtureProcesses } from "./fixtureProcs"; + +export default async function globalSetup() { + await sweepFixtureProcesses("setup"); +} diff --git a/editor/e2e/globalTeardown.ts b/editor/e2e/globalTeardown.ts @@ -0,0 +1,17 @@ +// Last line of defence: nothing this suite spawned may outlive it. +// +// The three layers above this one should make strays impossible — live jobs are +// cancelled before /api/test/invalidate-cache drops the registry, the server +// cancels them again on graceful shutdown, and each fake binary self-terminates +// when it is orphaned. This hook exists because "should be impossible" is not +// the same as "is", and because the failure mode is so expensive: orphaned +// fixtures do not announce themselves, they just quietly make the next run slow +// and flaky in ways that read like real regressions. +// +// Anything reaped here is logged as a regression, not as routine cleanup. + +import { sweepFixtureProcesses } from "./fixtureProcs"; + +export default async function globalTeardown() { + await sweepFixtureProcesses("teardown"); +} diff --git a/editor/e2e/no-orphan-fixtures.spec.ts b/editor/e2e/no-orphan-fixtures.spec.ts @@ -0,0 +1,97 @@ +// Regression guard: a fixture child must never outlive the job that owns it. +// +// The bug this pins down: /api/test/invalidate-cache used to drop the job +// registry (globalThis.__yttJobRegistry__ = undefined) without cancelling +// anything first. For yt-dlp-style jobs the registry's AbortController is the +// ONLY kill path — runManagedFunction sets record.abortController but never +// record.child (common/jobs/streamCommand.ts:306-319), so cancel()'s +// child.kill() branch is a no-op for them. Dropping the registry therefore made +// every in-flight child unkillable by the app. +// +// That route is called by resetData() and writeSettings() — 303 resetData() +// calls across 83 of 87 spec files, many from inside test bodies while a job is +// running. So the leak fired on essentially every full run, not (as was long +// assumed) only on interrupted ones. A full run left 27 strays alive, each +// spinning ~29% of a core; the suite went from 30 min / 1 failure to 1.3 h / 13 +// failures, and all 13 read like real regressions in unrelated specs. +// +// A test suite that lies about what broke is the actual cost, which is why this +// is worth a dedicated spec. + +import { mkdir, writeFile } from "node:fs/promises"; +import { test, expect } from "@playwright/test"; +import { resetData, resolvePath, writeSettings, generateReport } from "./helpers"; +import { listFixtureProcesses } from "./fixtureProcs"; + +async function makeTranscribeChannel(slug: string, ids: string[]) { + const root = resolvePath(`test-transcripts/channels/${slug}`); + await mkdir(root, { recursive: true }); + await writeFile( + `${root}/config.json`, + JSON.stringify({ + handling: "transcribe", + name: slug, + url: "https://odysee.com/@example", + audioFormat: "mp3", + }), + ); + for (const id of ids) { + await mkdir(`${root}/data/${id}`, { recursive: true }); + await writeFile(`${root}/data/${id}/audio.mp3`, `fake audio ${id}\n`); + } +} + +test("resetData cancels in-flight fixture children instead of orphaning them", async ({ + page, +}) => { + test.setTimeout(90_000); + await resetData("empty"); + + // parakeet is the right lever here: for a "slowop" dir the fake wrapper waits + // on a 30s backstop before finishing (fixtures/bin/fake-parakeet-stitch.mjs). + // That is a wide enough window that a stray is unambiguous — a fixture with a + // sub-second sleep would "pass" simply by exiting on its own. + await writeSettings({ + workers: [ + { + id: "gpu", + name: "GPU parakeet", + kind: "local", + enabled: true, + priority: 0, + appId: "parakeet", + config: {}, + }, + ], + }); + await makeTranscribeChannel("orphan-chan", ["slowoporphan1"]); + + await generateReport(page, "orphan-chan"); + await page.goto("/channels/orphan-chan"); + await page.getByRole("button", { name: "Transcribe missing" }).click(); + + // Wait for the child to actually exist before trying to strand it, otherwise + // the assertion below could pass because nothing had spawned yet. + await expect + .poll(async () => (await listFixtureProcesses()).length, { + timeout: 20_000, + message: "expected a fixture child to be running", + }) + .toBeGreaterThan(0); + + // The stranding event: this hits /api/test/invalidate-cache, which wipes the + // registry singletons. Pre-fix, the running child survived this with nothing + // left holding a reference that could kill it. + await resetData("empty"); + + // Generous window: the app sends SIGTERM immediately (abort() fires execa's + // cancelSignal synchronously) and execa escalates to SIGKILL after 5s, so a + // correctly-cancelled child is gone well inside this. A child left orphaned + // would still be sleeping on its 30s backstop when this expires. + await expect + .poll(async () => (await listFixtureProcesses()).length, { + timeout: 15_000, + message: "fixture child outlived the registry wipe — it was orphaned", + }) + .toBe(0); +}); diff --git a/editor/instrumentation.ts b/editor/instrumentation.ts @@ -9,6 +9,23 @@ export async function register() { // so guard the import — and run nothing on Edge. if (process.env.NEXT_RUNTIME !== "nodejs") return; + // Cancel in-flight children on a graceful shutdown. Armed FIRST, before any of + // the runners below: a failure while starting those must not leave the server + // without its only way to reap the children they spawn. + // + // Must be a lazy import like the rest — the module uses process.once/kill/ + // listenerCount, and inlining it here fails the Edge bundle's static Node-API + // scan even though the guard above means it never runs there. See + // common/jobs/shutdownCancel.ts. + try { + const { armShutdownCancel } = await import( + "yt-dlp-transcript-common/jobs/shutdownCancel" + ); + armShutdownCancel(); + } catch { + /* failing to arm the reaper must not block server readiness */ + } + // Lazy import inside the guard keeps server-only code out of the Edge bundle. // startSyncHeartbeat only arms a timer (no synchronous tick), so it never // blocks the server from becoming ready. diff --git a/editor/playwright.config.ts b/editor/playwright.config.ts @@ -38,6 +38,12 @@ const exportSitesDir = path.resolve( export default defineConfig({ testDir: "./e2e", timeout: 30_000, + // Reap fixture binaries that outlived their run — before this one starts (a + // SIGKILLed run never gets to clean up after itself) and again after it ends. + // Both log anything they find, because by construction they should find + // nothing. See e2e/fixtureProcs.ts. + globalSetup: "./e2e/globalSetup.ts", + globalTeardown: "./e2e/globalTeardown.ts", // The sharded runner sets CI=true for `reuseExistingServer: !CI` below, not // for retries, and passes an explicit --retries=0 so its failures stay // comparable to a serial run's. See scripts/run-sharded-e2e.mjs.