refactor(engine): manage child process lifecycles (#2160)

* refactor(engine): manage child process lifecycles * fix(engine): preserve child reaping after runtime errors * fix(engine): untrack child processes on exit
2026-09-04 16:42:27 +00:00 · 2026-07-17 01:17:53 -04:00
parent 8e162921bc
commit 57d3bf4960
17 changed files with 697 additions and 537 deletions
@@ -94,6 +94,12 @@ async function flushMuxCodecResolution(): Promise<void> {
  await Promise.resolve();
 }

+async function flushManagedProcessResolution(): Promise<void> {
+  await Promise.resolve();
+  await Promise.resolve();
+  await Promise.resolve();
+}
+
 describe("ENCODER_PRESETS", () => {
  it("has draft, standard, and high presets", () => {
    expect(ENCODER_PRESETS).toHaveProperty("draft");
@@ -310,7 +316,7 @@ describe("encodeFramesChunkedConcat ffmpegEncodeTimeout", () => {

    expect(calls).toHaveLength(1);
    emitClose(calls[0]!.proc, 0);
-    await Promise.resolve();
+    await flushManagedProcessResolution();

    expect(calls).toHaveLength(2);
    const concatProc = calls[1]!.proc;
@@ -353,7 +359,7 @@ describe("encodeFramesChunkedConcat ffmpegEncodeTimeout", () => {
    expect(chunkProc.kill).not.toHaveBeenCalled();

    emitClose(chunkProc, 0);
-    await Promise.resolve();
+    await flushManagedProcessResolution();

    expect(calls).toHaveLength(2);
    const concatProc = calls[1]!.proc;
@@ -6,10 +6,8 @@
 * Supports CPU (libx264) and GPU encoding.
 */

-import { spawn } from "child_process";
 import { copyFileSync, existsSync, mkdirSync, readdirSync, statSync, writeFileSync } from "fs";
 import { join, dirname, extname } from "path";
-import { trackChildProcess } from "../utils/processTracker.js";
 import { DEFAULT_CONFIG, type EngineConfig } from "../config.js";
 import {
  type GpuEncoder,
@@ -20,7 +18,6 @@ import {
 import { type HdrTransfer, getHdrEncoderColorParams } from "../utils/hdr.js";
 import { withEvenDimensionPad } from "../utils/evenDimensions.js";
 import { formatFfmpegError, runFfmpeg } from "../utils/runFfmpeg.js";
-import { getFfmpegBinary } from "../utils/ffmpegBinaries.js";
 import { extractAudioMetadata } from "../utils/ffprobe.js";
 import { type Fps, fpsToFfmpegArg } from "@hyperframes/core";
 import type { EncoderOptions, EncodeResult, MuxResult } from "./chunkEncoder.types.js";
@@ -484,82 +481,40 @@ export async function encodeFramesFromDir(
  const inputPath = join(framesDir, framePattern);
  const inputArgs = ["-framerate", fpsToFfmpegArg(options.fps), "-i", inputPath];
  const args = buildEncoderArgs(options, inputArgs, outputPath, gpuEncoder);
-
-  return new Promise((resolve) => {
-    const ffmpeg = spawn(getFfmpegBinary(), args);
-    trackChildProcess(ffmpeg);
-    let stderr = "";
-    const onAbort = () => {
-      ffmpeg.kill("SIGTERM");
+  const encodeTimeout = config?.ffmpegEncodeTimeout ?? DEFAULT_CONFIG.ffmpegEncodeTimeout;
+  const result = await runFfmpeg(args, { signal, timeout: encodeTimeout });
+  if (result.terminationReason === "abort") {
+    return {
+      success: false,
+      outputPath,
+      durationMs: result.durationMs,
+      framesEncoded: 0,
+      fileSize: 0,
+      error: "FFmpeg encode cancelled",
    };
-    if (signal) {
-      if (signal.aborted) {
-        ffmpeg.kill("SIGTERM");
-      } else {
-        signal.addEventListener("abort", onAbort, { once: true });
-      }
-    }
-
-    const encodeTimeout = config?.ffmpegEncodeTimeout ?? DEFAULT_CONFIG.ffmpegEncodeTimeout;
-    let timedOut = false;
-    const timer = setTimeout(() => {
-      timedOut = true;
-      ffmpeg.kill("SIGTERM");
-    }, encodeTimeout);
-
-    ffmpeg.stderr.on("data", (data) => {
-      stderr += data.toString();
-    });
-
-    ffmpeg.on("close", (code) => {
-      clearTimeout(timer);
-      if (signal) signal.removeEventListener("abort", onAbort);
-      const durationMs = Date.now() - startTime;
-      if (signal?.aborted && !timedOut) {
-        resolve({
-          success: false,
-          outputPath,
-          durationMs,
-          framesEncoded: 0,
-          fileSize: 0,
-          error: "FFmpeg encode cancelled",
-        });
-        return;
-      }
-
-      if (code !== 0 || timedOut) {
-        resolve({
-          success: false,
-          outputPath,
-          durationMs,
-          framesEncoded: 0,
-          fileSize: 0,
-          error: appendEncodeTimeoutMessage(
-            formatFfmpegError(code, stderr),
-            timedOut,
-            encodeTimeout,
-          ),
-        });
-        return;
-      }
-
-      const fileSize = existsSync(outputPath) ? statSync(outputPath).size : 0;
-      resolve({ success: true, outputPath, durationMs, framesEncoded: frameCount, fileSize });
-    });
-
-    ffmpeg.on("error", (err) => {
-      clearTimeout(timer);
-      if (signal) signal.removeEventListener("abort", onAbort);
-      resolve({
-        success: false,
-        outputPath,
-        durationMs: Date.now() - startTime,
-        framesEncoded: 0,
-        fileSize: 0,
-        error: appendEncodeTimeoutMessage(`[FFmpeg] ${err.message}`, timedOut, encodeTimeout),
-      });
-    });
-  });
+  }
+  if (!result.success) {
+    return {
+      success: false,
+      outputPath,
+      durationMs: result.durationMs,
+      framesEncoded: 0,
+      fileSize: 0,
+      error: appendEncodeTimeoutMessage(
+        formatFfmpegError(result.exitCode, result.stderr),
+        result.terminationReason === "deadline",
+        encodeTimeout,
+      ),
+    };
+  }
+  const fileSize = existsSync(outputPath) ? statSync(outputPath).size : 0;
+  return {
+    success: true,
+    outputPath,
+    durationMs: Date.now() - startTime,
+    framesEncoded: frameCount,
+    fileSize,
+  };
 }

 export async function encodeFramesChunkedConcat(
@@ -624,45 +579,18 @@ export async function encodeFramesChunkedConcat(
    let gpuEncoder: GpuEncoder = null;
    if (options.useGpu) gpuEncoder = await getCachedGpuEncoder();
    const args = buildEncoderArgs(options, inputArgs, chunkPath, gpuEncoder);
-    const chunkResult = await new Promise<{ success: boolean; error?: string }>((resolve) => {
-      const ffmpeg = spawn(getFfmpegBinary(), args);
-      trackChildProcess(ffmpeg);
-      let stderr = "";
-      const encodeTimeout = config?.ffmpegEncodeTimeout ?? DEFAULT_CONFIG.ffmpegEncodeTimeout;
-      let timedOut = false;
-      const timer = setTimeout(() => {
-        timedOut = true;
-        ffmpeg.kill("SIGTERM");
-      }, encodeTimeout);
-      ffmpeg.stderr.on("data", (d) => {
-        stderr += d.toString();
-      });
-      ffmpeg.on("close", (code) => {
-        clearTimeout(timer);
-        if (code === 0 && !timedOut) resolve({ success: true });
-        else {
-          resolve({
-            success: false,
-            error: appendEncodeTimeoutMessage(
-              `Chunk ${i} encode failed: ${stderr.slice(-400)}`,
-              timedOut,
-              encodeTimeout,
-            ),
-          });
-        }
-      });
-      ffmpeg.on("error", (err) => {
-        clearTimeout(timer);
-        resolve({
-          success: false,
-          error: appendEncodeTimeoutMessage(
-            `Chunk ${i} encode error: ${err.message}`,
-            timedOut,
+    const encodeTimeout = config?.ffmpegEncodeTimeout ?? DEFAULT_CONFIG.ffmpegEncodeTimeout;
+    const processResult = await runFfmpeg(args, { signal, timeout: encodeTimeout });
+    const chunkResult = {
+      success: processResult.success,
+      error: processResult.success
+        ? undefined
+        : appendEncodeTimeoutMessage(
+            `Chunk ${i} encode failed: ${processResult.stderr.slice(-400)}`,
+            processResult.terminationReason === "deadline",
            encodeTimeout,
          ),
-        });
-      });
-    });
+    };
    if (!chunkResult.success) {
      return {
        success: false,
@@ -692,45 +620,18 @@ export async function encodeFramesChunkedConcat(
    "-y",
    outputPath,
  ];
-  const concatResult = await new Promise<{ success: boolean; error?: string }>((resolve) => {
-    const ffmpeg = spawn(getFfmpegBinary(), concatArgs);
-    trackChildProcess(ffmpeg);
-    let stderr = "";
-    const encodeTimeout = config?.ffmpegEncodeTimeout ?? DEFAULT_CONFIG.ffmpegEncodeTimeout;
-    let timedOut = false;
-    const timer = setTimeout(() => {
-      timedOut = true;
-      ffmpeg.kill("SIGTERM");
-    }, encodeTimeout);
-    ffmpeg.stderr.on("data", (d) => {
-      stderr += d.toString();
-    });
-    ffmpeg.on("close", (code) => {
-      clearTimeout(timer);
-      if (code === 0 && !timedOut) resolve({ success: true });
-      else {
-        resolve({
-          success: false,
-          error: appendEncodeTimeoutMessage(
-            `Chunk concat failed: ${stderr.slice(-400)}`,
-            timedOut,
-            encodeTimeout,
-          ),
-        });
-      }
-    });
-    ffmpeg.on("error", (err) => {
-      clearTimeout(timer);
-      resolve({
-        success: false,
-        error: appendEncodeTimeoutMessage(
-          `Chunk concat error: ${err.message}`,
-          timedOut,
+  const encodeTimeout = config?.ffmpegEncodeTimeout ?? DEFAULT_CONFIG.ffmpegEncodeTimeout;
+  const concatProcessResult = await runFfmpeg(concatArgs, { signal, timeout: encodeTimeout });
+  const concatResult = {
+    success: concatProcessResult.success,
+    error: concatProcessResult.success
+      ? undefined
+      : appendEncodeTimeoutMessage(
+          `Chunk concat failed: ${concatProcessResult.stderr.slice(-400)}`,
+          concatProcessResult.terminationReason === "deadline",
          encodeTimeout,
        ),
-      });
-    });
-  });
+  };

  if (!concatResult.success) {
    return {
@@ -819,7 +819,7 @@ describe("spawnStreamingEncoder lifecycle and cleanup", () => {
    await expect(resolveWithin(writePromise)).resolves.toBe(false);
    expect(encoder.getExitStatus()).toBe("error");
    expect(proc.stdin.listenerCount("drain")).toBe(0);
-    expect(proc.listenerCount("close")).toBe(baselineCloseListeners);
+    expect(proc.listenerCount("close")).toBeLessThanOrEqual(baselineCloseListeners);

    const result = await encoder.close();
    expect(result.success).toBe(false);
@@ -16,6 +16,10 @@
 import { spawn, type ChildProcess } from "child_process";
 import { once } from "events";
 import { trackChildProcess } from "../utils/processTracker.js";
+import {
+  ManagedChildProcess,
+  type ManagedProcessTerminationReason,
+} from "../utils/managedChildProcess.js";
 import { existsSync, mkdirSync, statSync } from "fs";
 import { dirname } from "path";

@@ -447,7 +451,6 @@ export async function spawnStreamingEncoder(

  const args = buildStreamingArgs(options, outputPath, gpuEncoder);

-  const startTime = Date.now();
  const ffmpeg: ChildProcess = spawn(getFfmpegBinary(), args, {
    stdio: ["pipe", "pipe", "pipe"],
  });
@@ -456,43 +459,11 @@ export async function spawnStreamingEncoder(
  let exitStatus: "running" | "success" | "error" = "running";
  let stderr = "";
  let exitCode: number | null = null;
-  let exitPromiseResolve: ((value: void) => void) | null = null;
-  const exitPromise = new Promise<void>((resolve) => (exitPromiseResolve = resolve));
-
-  // Track stderr for progress and error messages
-  ffmpeg.stderr?.on("data", (data: Buffer) => {
-    stderr += data.toString();
-  });
-
-  ffmpeg.on("close", (code: number | null) => {
-    exitCode = code;
-    exitStatus = code === 0 ? "success" : "error";
-    exitPromiseResolve?.();
-  });
-
-  ffmpeg.on("error", (err: Error) => {
-    exitStatus = "error";
-    stderr += `\nProcess error: ${err.message}`;
-    exitPromiseResolve?.();
-  });
+  let terminationReason: ManagedProcessTerminationReason = "exit";

  ffmpeg.stdin?.on("error", () => {});
  ffmpeg.stdout?.on("error", () => {});

-  // Handle abort signal
-  const onAbort = () => {
-    if (exitStatus === "running") {
-      ffmpeg.kill("SIGTERM");
-    }
-  };
-  if (signal) {
-    if (signal.aborted) {
-      ffmpeg.kill("SIGTERM");
-    } else {
-      signal.addEventListener("abort", onAbort, { once: true });
-    }
-  }
-
  // Inactivity timeout: fires only when no frame has been written for
  // `ffmpegStreamingTimeout` ms. A slow-but-progressing capture (e.g. a CI
  // runner under load) keeps resetting the timer on each writeFrame, so total
@@ -503,16 +474,17 @@ export async function spawnStreamingEncoder(
  // libx264 printed its summary and exited 255, observable as
  // "Streaming encode failed: FFmpeg exited with code 255" with audio:0kB).
  const streamingTimeout = config?.ffmpegStreamingTimeout ?? DEFAULT_CONFIG.ffmpegStreamingTimeout;
-  let timer: NodeJS.Timeout | null = null;
-  const resetTimer = () => {
-    if (timer) clearTimeout(timer);
-    timer = setTimeout(() => {
-      if (exitStatus === "running") {
-        ffmpeg.kill("SIGTERM");
-      }
-    }, streamingTimeout);
-  };
-  resetTimer();
+  const managed = new ManagedChildProcess(ffmpeg, {
+    signal,
+    inactivityTimeoutMs: streamingTimeout,
+  });
+  const exitPromise = managed.wait().then((outcome) => {
+    exitCode = outcome.exitCode;
+    stderr = outcome.stderr;
+    terminationReason = outcome.reason;
+    exitStatus = outcome.reason === "exit" && outcome.exitCode === 0 ? "success" : "error";
+    return outcome;
+  });

  const waitForDrainOrExit = async (
    stdin: NonNullable<ChildProcess["stdin"]>,
@@ -539,7 +511,7 @@ export async function spawnStreamingEncoder(
        throw err;
      });

-      if (exitStatus !== "running") {
+      if (managed.isSettled || exitStatus !== "running") {
        return "exit";
      }

@@ -573,7 +545,7 @@ export async function spawnStreamingEncoder(
      // before draining, waitForDrainOrExit returns "exit", removes its
      // one-shot listeners, and callers see `false` instead of hanging.
      if (accepted) {
-        resetTimer();
+        managed.markActivity();
        return true;
      }

@@ -581,7 +553,7 @@ export async function spawnStreamingEncoder(
      if (drainResult !== "drain" || exitStatus !== "running") {
        return false;
      }
-      resetTimer();
+      managed.markActivity();
      return true;
    },

@@ -590,9 +562,6 @@ export async function spawnStreamingEncoder(
      // path tracks an `encoderClosed` flag and may still re-call close() in
      // the outer finally if the inner cleanup raised before the flag flipped.
      // Each step here must be safe to repeat:
-      //   - clearTimeout: safe to call on an already-cleared/fired timer
-      //   - removeEventListener: no-op if the listener was already removed
-      //     (and {once: true} would have removed it on the first abort anyway)
      //   - stdin.end gated on !destroyed: skipped on the second call
      //   - exitPromise: a single shared Promise; awaiting an already-resolved
      //     Promise resolves immediately with the same captured exitCode
@@ -600,12 +569,6 @@ export async function spawnStreamingEncoder(
      // repeated calls. If you change this method, preserve idempotency or
      // a regression here will silently double-close ffmpeg and produce
      // harder-to-trace errors at the orchestrator layer.
-      if (timer) {
-        clearTimeout(timer);
-        timer = null;
-      }
-      if (signal) signal.removeEventListener("abort", onAbort);
-
      const stdin = ffmpeg.stdin;
      if (stdin && !stdin.destroyed) {
        await new Promise<void>((resolve) => {
@@ -613,11 +576,10 @@ export async function spawnStreamingEncoder(
        });
      }

-      await exitPromise;
+      const outcome = await exitPromise;
+      const durationMs = outcome.durationMs;

-      const durationMs = Date.now() - startTime;
-
-      if (signal?.aborted) {
+      if (terminationReason === "abort") {
        return {
          success: false,
          durationMs,
@@ -627,11 +589,15 @@ export async function spawnStreamingEncoder(
      }

      if (exitCode !== 0) {
+        const inactivitySuffix =
+          terminationReason === "inactivity"
+            ? `\nFFmpeg stopped after ${streamingTimeout} ms without consuming a frame.`
+            : "";
        return {
          success: false,
          durationMs,
          fileSize: 0,
-          error: formatFfmpegError(exitCode, stderr),
+          error: `${formatFfmpegError(exitCode, stderr)}${inactivitySuffix}`,
        };
      }

@@ -6,12 +6,10 @@
 * Videos are replaced with <img> elements during capture.
 */

-import { spawn } from "child_process";
 import { copyFileSync, existsSync, linkSync, mkdirSync, readdirSync, rmSync } from "fs";
 import { isAbsolute, join, posix, resolve, sep } from "path";
 import { parseHTML } from "linkedom";
 import { decodeUrlPathVariants, MEDIA_DURATION_CLAMP_EPSILON_SECONDS } from "@hyperframes/core";
-import { trackChildProcess } from "../utils/processTracker.js";
 import { resolveReferencedStart, type RefResolverEl } from "./referenceResolver.js";
 import { extractMediaMetadata, type VideoMetadata } from "../utils/ffprobe.js";
 import {
@@ -20,7 +18,7 @@ import {
  type HdrTransfer,
 } from "../utils/hdr.js";
 import { downloadToTemp, isHttpUrl } from "../utils/urlDownloader.js";
-import { getFfmpegBinary } from "../utils/ffmpegBinaries.js";
+import { runFfmpeg } from "../utils/runFfmpeg.js";
 import { DEFAULT_CONFIG, type EngineConfig } from "../config.js";
 import { unwrapTemplate } from "../utils/htmlTemplate.js";
 import {
@@ -328,78 +326,51 @@ export async function extractVideoFramesRange(
  if (format === "png") args.push("-compression_level", "1");
  args.push("-y", outputPattern);

-  return new Promise((resolve, reject) => {
-    const ffmpeg = spawn(getFfmpegBinary(), args);
-    trackChildProcess(ffmpeg);
-    let stderr = "";
-    const onAbort = () => {
-      ffmpeg.kill("SIGTERM");
-    };
-    if (signal) {
-      if (signal.aborted) {
-        ffmpeg.kill("SIGTERM");
-      } else {
-        signal.addEventListener("abort", onAbort, { once: true });
-      }
+  const processResult = await runFfmpeg(args, { signal, timeout: ffmpegProcessTimeout });
+  if (processResult.terminationReason === "abort") {
+    throw new Error("Video frame extraction cancelled");
+  }
+  if (processResult.terminationReason === "spawn_error") {
+    if ((processResult.error as NodeJS.ErrnoException | undefined)?.code === "ENOENT") {
+      throw new Error("[FFmpeg] ffmpeg not found");
    }
+    throw processResult.error ?? new Error(processResult.stderr);
+  }
+  if (!processResult.success) {
+    // With the SDR-to-HDR remap folded into this pass, a filter failure
+    // (e.g. an ffmpeg built without the colorspace filter) would otherwise
+    // surface as a generic extract error and the operator has to grep the
+    // filter chain to learn it was the HDR conversion. Attribute it.
+    const hdrPrefix = options.sdrToHdrTransfer
+      ? `SDR→HDR conversion failed (colorspace filter in extract pass, target ${options.sdrToHdrTransfer}): `
+      : "";
+    const timeoutSuffix =
+      processResult.terminationReason === "deadline"
+        ? ` (timed out after ${ffmpegProcessTimeout} ms)`
+        : "";
+    throw new Error(
+      `${hdrPrefix}FFmpeg exited with code ${processResult.exitCode}${timeoutSuffix}: ${processResult.stderr.slice(-500)}`,
+    );
+  }

-    const timer = setTimeout(() => {
-      ffmpeg.kill("SIGTERM");
-    }, ffmpegProcessTimeout);
-
-    ffmpeg.stderr.on("data", (data) => {
-      stderr += data.toString();
-    });
-
-    ffmpeg.on("close", (code) => {
-      clearTimeout(timer);
-      if (signal) signal.removeEventListener("abort", onAbort);
-      if (signal?.aborted) {
-        reject(new Error("Video frame extraction cancelled"));
-        return;
-      }
-      if (code !== 0) {
-        // With the SDR-to-HDR remap folded into this pass, a filter failure
-        // (e.g. an ffmpeg built without the colorspace filter) would otherwise
-        // surface as a generic extract error and the operator has to grep the
-        // filter chain to learn it was the HDR conversion. Attribute it.
-        const hdrPrefix = options.sdrToHdrTransfer
-          ? `SDR→HDR conversion failed (colorspace filter in extract pass, target ${options.sdrToHdrTransfer}): `
-          : "";
-        reject(new Error(`${hdrPrefix}FFmpeg exited with code ${code}: ${stderr.slice(-500)}`));
-        return;
-      }
-
-      const framePaths = new Map<number, string>();
-      const files = readdirSync(videoOutputDir)
-        .filter((f) => f.startsWith(FRAME_FILENAME_PREFIX) && f.endsWith(`.${format}`))
-        .sort();
-      files.forEach((file, index) => {
-        framePaths.set(index, join(videoOutputDir, file));
-      });
-
-      resolve({
-        videoId,
-        srcPath: videoPath,
-        outputDir: videoOutputDir,
-        framePattern,
-        fps,
-        totalFrames: framePaths.size,
-        metadata,
-        framePaths,
-      });
-    });
-
-    ffmpeg.on("error", (err) => {
-      clearTimeout(timer);
-      if (signal) signal.removeEventListener("abort", onAbort);
-      if ((err as NodeJS.ErrnoException).code === "ENOENT") {
-        reject(new Error("[FFmpeg] ffmpeg not found"));
-      } else {
-        reject(err);
-      }
-    });
+  const framePaths = new Map<number, string>();
+  const files = readdirSync(videoOutputDir)
+    .filter((f) => f.startsWith(FRAME_FILENAME_PREFIX) && f.endsWith(`.${format}`))
+    .sort();
+  files.forEach((file, index) => {
+    framePaths.set(index, join(videoOutputDir, file));
  });
+
+  return {
+    videoId,
+    srcPath: videoPath,
+    outputDir: videoOutputDir,
+    framePattern,
+    fps,
+    totalFrames: framePaths.size,
+    metadata,
+    framePaths,
+  };
 }

 /**