From dcec31b468b85a03245045d7cbbae5a98dd741bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Mon, 26 Oct 2026 23:49:02 +0000 Subject: [PATCH] fix(engine): audio scratch files Windows refuses to delete no longer fail a render (#6474) * fix(engine): a scratch folder Windows refuses to delete no longer fails a finished audio mix * fix(engine): audio scratch files Windows refuses to delete no longer fail a render * fix(engine): a rename Windows keeps refusing falls back to a copy so the audio mix finishes * fix(engine): audio scratch left behind is counted in the render log * style(engine): wrap the long rename fallback test call * refactor(engine): split the rename fallback decision out of the retry loop * fix(engine): the rename copy fallback is opt-in and scratch leftovers are counted per render * style(engine): keep renameWithRetry's comment share at the base * style(engine): drop the blank line left by the comment cut --- packages/engine/src/index.ts | 1 + packages/engine/src/services/aacEncode.ts | 5 +- packages/engine/src/services/audioFxRender.ts | 4 +- packages/engine/src/services/audioMixer.ts | 20 ++++--- .../engine/src/utils/removeScratch.test.ts | 57 +++++++++++++++++++ packages/engine/src/utils/removeScratch.ts | 27 +++++++++ .../engine/src/utils/renameWithRetry.test.ts | 41 +++++++++++++ packages/engine/src/utils/renameWithRetry.ts | 56 +++++++++++++++--- .../src/services/render/audioPadTrim.ts | 9 +-- .../src/services/render/stages/audioStage.ts | 16 +++++- 10 files changed, 208 insertions(+), 28 deletions(-) create mode 100644 packages/engine/src/utils/removeScratch.test.ts create mode 100644 packages/engine/src/utils/removeScratch.ts diff --git a/packages/engine/src/index.ts b/packages/engine/src/index.ts index f0e0c1ae8a..fa77d00c70 100644 --- a/packages/engine/src/index.ts +++ b/packages/engine/src/index.ts @@ -396,6 +396,7 @@ export { export { trackChildProcess, killTrackedProcesses } from "./utils/processTracker.js"; export { renameWithRetry, renameWithRetryAsync } from "./utils/renameWithRetry.js"; +export { removeScratch, collectScratchLeftovers } from "./utils/removeScratch.js"; // drawElement self-verify comparison — shared by the streaming drain // (producer) and the parallel disk-path verify (parallelCoordinator). diff --git a/packages/engine/src/services/aacEncode.ts b/packages/engine/src/services/aacEncode.ts index f700615fcc..c2bdbd1000 100644 --- a/packages/engine/src/services/aacEncode.ts +++ b/packages/engine/src/services/aacEncode.ts @@ -4,7 +4,8 @@ */ import { execFile } from "node:child_process"; -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { removeScratch } from "../utils/removeScratch.js"; import { dirname, join } from "node:path"; import { getFfmpegBinary } from "../utils/ffmpegBinaries.js"; import { runFfmpeg, type RunFfmpegResult } from "../utils/runFfmpeg.js"; @@ -166,6 +167,6 @@ export async function encodeAac( const slowest = Math.max(...results.map((result) => result.durationMs)); return { ...muxed, durationMs: slowest + muxed.durationMs }; } finally { - rmSync(dir, { recursive: true, force: true }); + removeScratch(dir); } } diff --git a/packages/engine/src/services/audioFxRender.ts b/packages/engine/src/services/audioFxRender.ts index e8c9079c4d..fafed6eb4c 100644 --- a/packages/engine/src/services/audioFxRender.ts +++ b/packages/engine/src/services/audioFxRender.ts @@ -19,7 +19,6 @@ import { openSync, readFileSync, readSync, - rmSync, statSync, writeFileSync, writeSync, @@ -30,6 +29,7 @@ import { pathToFileURL } from "node:url"; import { getAudioFxRuntimeScript } from "@hyperframes/core/audio-fx-runtime"; import { enabledAudioFxNodes, type HfAudioFxChain } from "@hyperframes/core/audio-fx"; import { serializeAutomation, type HfAutomation } from "@hyperframes/core/audio-automation"; +import { removeScratch } from "../utils/removeScratch.js"; import { acquireBrowser } from "./browserManager.js"; import { createEnvelopeWalker } from "./audioVolumeEnvelope.js"; import { riffChunks, wavFormatTag } from "./wavChunks.js"; @@ -630,7 +630,7 @@ async function withFxBrowser( `Audio FX failed for track ${options.trackId}: ${(err as Error).message}`, ); } finally { - rmSync(hostDir, { recursive: true, force: true }); + removeScratch(hostDir); await lease?.release().catch(() => undefined); } } diff --git a/packages/engine/src/services/audioMixer.ts b/packages/engine/src/services/audioMixer.ts index 165cd886dd..fb80bee8e6 100644 --- a/packages/engine/src/services/audioMixer.ts +++ b/packages/engine/src/services/audioMixer.ts @@ -2,6 +2,7 @@ import { isSelfOrAncestorHidden, memberGroupKey, isMemberGroupHidden } from "./mediaHidden.js"; import { closeSync, existsSync, mkdirSync, mkdtempSync, openSync, rmSync, writeFileSync } from "fs"; import { renameWithRetry } from "../utils/renameWithRetry.js"; +import { removeScratch } from "../utils/removeScratch.js"; import { join, dirname, isAbsolute, relative } from "path"; import { parseHTML } from "linkedom"; import { extractAudioMetadata, type AudioMetadata } from "../utils/ffprobe.js"; @@ -751,7 +752,7 @@ async function extractAudioSegment( failure, }; } finally { - rmSync(cycleDir, { recursive: true, force: true }); + removeScratch(cycleDir); } } const inputSeek = mediaStart >= (metadata?.latestStreamLeadSeconds ?? 0); @@ -979,7 +980,7 @@ async function mixAudioTracksFlat( currentArgs[currentArgs.indexOf("-filter_complex_script")] = "-/filter_complex"; return await runFfmpeg(currentArgs, { signal, timeout: ffmpegProcessTimeout }); } finally { - rmSync(scriptDir, { recursive: true, force: true }); + removeScratch(scriptDir); } }; @@ -1163,7 +1164,7 @@ async function mixGroupMembersFlat( currentArgs[currentArgs.indexOf("-filter_complex_script")] = "-/filter_complex"; return await runFfmpeg(currentArgs, { signal, timeout: ffmpegProcessTimeout }); } finally { - rmSync(scriptDir, { recursive: true, force: true }); + removeScratch(scriptDir); } }; @@ -1249,8 +1250,8 @@ async function sumInSlices( signal, config, ); - rmSync(running.srcPath, { force: true }); - rmSync(slicePath, { force: true }); + removeScratch(running.srcPath); + removeScratch(slicePath); if (!folded.success) return folded; running = wavTrack(id, foldedPath); } @@ -1273,10 +1274,10 @@ async function mixGroupMembers( try { const summed = await sumInSlices(memberTracks, scratchDir, totalDuration, signal, config); if (!summed.success) return summed; - renameWithRetry(summed.track.srcPath, outputPath); + renameWithRetry(summed.track.srcPath, outputPath, { copyFallback: true }); return { success: true, degradedAutomation: summed.degradedAutomation }; } finally { - rmSync(scratchDir, { recursive: true, force: true }); + removeScratch(scratchDir); } } @@ -1321,7 +1322,7 @@ async function mixAudioTracks( } : mixed; } finally { - rmSync(scratchDir, { recursive: true, force: true }); + removeScratch(scratchDir); } } @@ -1374,7 +1375,8 @@ async function mixThroughFloatSum( failures: [failure], }; } - if (config?.keepEncoderInputAt) renameWithRetry(encoderInput, config.keepEncoderInputAt); + if (config?.keepEncoderInputAt) + renameWithRetry(encoderInput, config.keepEncoderInputAt, { copyFallback: true }); return { ...sum, outputPath, diff --git a/packages/engine/src/utils/removeScratch.test.ts b/packages/engine/src/utils/removeScratch.test.ts new file mode 100644 index 0000000000..a6e10715c1 --- /dev/null +++ b/packages/engine/src/utils/removeScratch.test.ts @@ -0,0 +1,57 @@ +import { mkdtempSync, readFileSync, writeFileSync, existsSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, it, vi } from "vitest"; + +vi.mock("node:fs", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, rmSync: vi.fn(actual.rmSync) }; +}); + +import * as fs from "node:fs"; +import { collectScratchLeftovers, removeScratch } from "./removeScratch.js"; +import { renameWithRetry } from "./renameWithRetry.js"; + +describe("removeScratch", () => { + it("removes a folder and what is in it", () => { + const dir = mkdtempSync(join(tmpdir(), "hf-scratch-")); + writeFileSync(join(dir, "a.wav"), "x"); + removeScratch(dir); + expect(existsSync(dir)).toBe(false); + }); + + it("does not fail when the system refuses the removal", () => { + vi.mocked(fs.rmSync).mockImplementationOnce(() => { + throw Object.assign(new Error("EPERM: operation not permitted, unlink"), { code: "EPERM" }); + }); + expect(() => removeScratch(join(tmpdir(), "hf-scratch-missing"))).not.toThrow(); + }); + + it("hands one run's leftovers to that run only, and a copy that cannot clear its source leaves it", async () => { + const refuse = () => { + throw Object.assign(new Error("EPERM: operation not permitted, unlink"), { code: "EPERM" }); + }; + const dir = mkdtempSync(join(tmpdir(), "hf-scratch-")); + const from = join(dir, "from.wav"); + const to = join(dir, "to.wav"); + writeFileSync(from, "audio"); + const reports: string[][] = []; + await Promise.all([ + collectScratchLeftovers( + async () => { + vi.mocked(fs.rmSync).mockImplementationOnce(refuse); + renameWithRetry(from, to, { platform: "win32", copyFallback: true, rename: refuse }); + }, + (left) => reports.push(left), + ), + collectScratchLeftovers( + async () => {}, + (left) => reports.push(left), + ), + ]); + removeScratch(join(dir, "outside-a-run")); + expect(readFileSync(to, "utf-8")).toBe("audio"); + expect(existsSync(from)).toBe(true); + expect(reports.sort()).toEqual([[], [from]]); + }); +}); diff --git a/packages/engine/src/utils/removeScratch.ts b/packages/engine/src/utils/removeScratch.ts new file mode 100644 index 0000000000..fb47d77b1f --- /dev/null +++ b/packages/engine/src/utils/removeScratch.ts @@ -0,0 +1,27 @@ +import { AsyncLocalStorage } from "node:async_hooks"; +import { rmSync } from "node:fs"; + +const collecting = new AsyncLocalStorage(); + +/** Removes a scratch folder or file the mix is already done with. A refusal (Windows temp folders that deny + * delete) leaves it for the render's own cleanup and never fails the audio it served. */ +export function removeScratch(path: string): void { + try { + rmSync(path, { recursive: true, force: true }); + } catch { + collecting.getStore()?.push(path); + } +} + +/** Runs `run`; whatever removeScratch left behind inside it (this run's only) goes to `report` once it ends. */ +export async function collectScratchLeftovers( + run: () => Promise, + report: (left: string[]) => void, +): Promise { + const left: string[] = []; + try { + return await collecting.run(left, run); + } finally { + report(left); + } +} diff --git a/packages/engine/src/utils/renameWithRetry.test.ts b/packages/engine/src/utils/renameWithRetry.test.ts index f7f79b7ba9..5ef825709e 100644 --- a/packages/engine/src/utils/renameWithRetry.test.ts +++ b/packages/engine/src/utils/renameWithRetry.test.ts @@ -45,4 +45,45 @@ describe("renameWithRetry", () => { await renameWithRetryAsync("a", "b", { platform: "win32", rename: async () => rename.fn() }); expect(rename.calls()).toBe(3); }); + + it("copies when win32 keeps refusing the rename, and fails with the rename error if the copy fails too", async () => { + const copied: string[] = []; + renameWithRetry("a", "b", { + platform: "win32", + copyFallback: true, + rename: failing("EPERM", 99).fn, + copy: (from, to) => void copied.push(`${from}>${to}`), + }); + expect(copied).toEqual(["a>b"]); + await renameWithRetryAsync("a", "c", { + platform: "win32", + copyFallback: true, + rename: async () => failing("EPERM", 99).fn(), + copy: async (from, to) => void copied.push(`${from}>${to}`), + }); + expect(copied).toEqual(["a>b", "a>c"]); + const broken = () => { + throw new Error("copy denied"); + }; + expect(() => + renameWithRetry("a", "b", { + platform: "win32", + copyFallback: true, + rename: failing("EPERM", 99).fn, + copy: broken, + }), + ).toThrow(/EPERM/); + }); + + it("never copies unless asked: an atomic publish keeps rename-or-throw", () => { + const copied: string[] = []; + expect(() => + renameWithRetry("a", "b", { + platform: "win32", + rename: failing("EPERM", 99).fn, + copy: (from, to) => void copied.push(`${from}>${to}`), + }), + ).toThrow(/EPERM/); + expect(copied).toEqual([]); + }); }); diff --git a/packages/engine/src/utils/renameWithRetry.ts b/packages/engine/src/utils/renameWithRetry.ts index 3c7307e36f..f0d62e1301 100644 --- a/packages/engine/src/utils/renameWithRetry.ts +++ b/packages/engine/src/utils/renameWithRetry.ts @@ -1,5 +1,6 @@ -import { renameSync } from "node:fs"; -import { rename } from "node:fs/promises"; +import { copyFileSync, renameSync } from "node:fs"; +import { copyFile, rename } from "node:fs/promises"; +import { removeScratch } from "./removeScratch.js"; // Windows reports EPERM/EACCES/EBUSY while antivirus or a just-exited process still holds the // file; the hold clears within milliseconds. Total wait before giving up: about 1.3 s. @@ -8,6 +9,8 @@ const BACKOFF_MS = [10, 20, 40, 80, 160, 320, 640]; interface Options { platform?: NodeJS.Platform; + /** Copy the bytes when Windows keeps refusing (a TEMP that denies delete). Not atomic: scratch files only. */ + copyFallback?: boolean; } function isRetryable(error: unknown, platform: NodeJS.Platform): boolean { @@ -15,6 +18,13 @@ function isRetryable(error: unknown, platform: NodeJS.Platform): boolean { return RETRYABLE.has((error as NodeJS.ErrnoException | undefined)?.code ?? ""); } +/** What a refused rename does next: wait and try again, fall back to a copy, or give up with the error. */ +function nextStep(error: unknown, attempt: number, options: Options, platform: NodeJS.Platform) { + if (!isRetryable(error, platform)) return "throw"; + if (attempt < BACKOFF_MS.length) return "wait"; + return options.copyFallback ? "copy" : "throw"; +} + function sleepSync(ms: number): void { Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms); } @@ -23,17 +33,32 @@ function sleepSync(ms: number): void { export function renameWithRetry( from: string, to: string, - options: Options & { rename?: (from: string, to: string) => void } = {}, + options: Options & { + rename?: (from: string, to: string) => void; + copy?: (from: string, to: string) => void; + } = {}, ): void { const doRename = options.rename ?? renameSync; + const doCopy = options.copy ?? copyFileSync; const platform = options.platform ?? process.platform; for (let attempt = 0; ; attempt++) { try { doRename(from, to); return; } catch (error) { - if (attempt >= BACKOFF_MS.length || !isRetryable(error, platform)) throw error; - sleepSync(BACKOFF_MS[attempt]!); + const step = nextStep(error, attempt, options, platform); + if (step === "throw") throw error; + if (step === "wait") { + sleepSync(BACKOFF_MS[attempt]!); + continue; + } + try { + doCopy(from, to); + } catch { + throw error; + } + removeScratch(from); + return; } } } @@ -42,17 +67,32 @@ export function renameWithRetry( export async function renameWithRetryAsync( from: string, to: string, - options: Options & { rename?: (from: string, to: string) => Promise } = {}, + options: Options & { + rename?: (from: string, to: string) => Promise; + copy?: (from: string, to: string) => Promise; + } = {}, ): Promise { const doRename = options.rename ?? rename; + const doCopy = options.copy ?? copyFile; const platform = options.platform ?? process.platform; for (let attempt = 0; ; attempt++) { try { await doRename(from, to); return; } catch (error) { - if (attempt >= BACKOFF_MS.length || !isRetryable(error, platform)) throw error; - await new Promise((resolve) => setTimeout(resolve, BACKOFF_MS[attempt])); + const step = nextStep(error, attempt, options, platform); + if (step === "throw") throw error; + if (step === "wait") { + await new Promise((resolve) => setTimeout(resolve, BACKOFF_MS[attempt])); + continue; + } + try { + await doCopy(from, to); + } catch { + throw error; + } + removeScratch(from); + return; } } } diff --git a/packages/producer/src/services/render/audioPadTrim.ts b/packages/producer/src/services/render/audioPadTrim.ts index ff42d02448..efcdea9c33 100644 --- a/packages/producer/src/services/render/audioPadTrim.ts +++ b/packages/producer/src/services/render/audioPadTrim.ts @@ -20,7 +20,7 @@ */ import { spawn } from "node:child_process"; -import { mkdtempSync, rmSync } from "node:fs"; +import { mkdtempSync } from "node:fs"; import { dirname, join } from "node:path"; import { DEFAULT_CONFIG, @@ -29,6 +29,7 @@ import { extractAudioMetadata, getFfprobeBinary, ManagedChildProcess, + removeScratch, renameWithRetry, runFfmpeg, trackChildProcess, @@ -333,7 +334,7 @@ export async function padOrTrimAudioToVideoFrameCount( }`, }; } finally { - for (const path of plan.cleanupPaths) rmSync(path, { force: true }); + for (const path of plan.cleanupPaths) removeScratch(path); } let loweredDb = 0; @@ -433,7 +434,7 @@ async function enforceAacTruePeak(input: EnforceAacTruePeakInput): Promise<{ } if (truePeakDbfs <= AAC_DELIVERY_TRUE_PEAK_DBFS) { if (measuredPath !== correctedPath) return { success: true }; - renameWithRetry(correctedPath, input.audioPath); + renameWithRetry(correctedPath, input.audioPath, { copyFallback: true }); return { success: true, loweredDb: Number((-attenuationDb).toFixed(2)) }; } if (pass === MAX_TRUE_PEAK_CORRECTION_PASSES) break; @@ -479,7 +480,7 @@ async function enforceAacTruePeak(input: EnforceAacTruePeakInput): Promise<{ )}`, }; } finally { - if (scratchDir) rmSync(scratchDir, { recursive: true, force: true }); + if (scratchDir) removeScratch(scratchDir); } } diff --git a/packages/producer/src/services/render/stages/audioStage.ts b/packages/producer/src/services/render/stages/audioStage.ts index 1df2e19cfc..3e98a8ab73 100644 --- a/packages/producer/src/services/render/stages/audioStage.ts +++ b/packages/producer/src/services/render/stages/audioStage.ts @@ -18,12 +18,13 @@ */ import { join } from "node:path"; -import { rmSync } from "node:fs"; import type { Fps } from "@hyperframes/core"; import { MIXED_AUDIO_FILENAME, audioFailureDiagnostics, processCompositionAudio, + removeScratch, + collectScratchLeftovers, type AudioProcessingFailure, type MixResult, } from "@hyperframes/engine"; @@ -89,7 +90,16 @@ function assertAudioNotInterrupted(failures: readonly AudioProcessingFailure[] | }); } -export async function runAudioStage(input: AudioStageInput): Promise { +export function runAudioStage(input: AudioStageInput): Promise { + return collectScratchLeftovers( + () => runAudioStageScoped(input), + (left) => { + if (left.length > 0) input.log?.warn("[audio] scratch left behind", { count: left.length }); + }, + ); +} + +async function runAudioStageScoped(input: AudioStageInput): Promise { const stageStart = Date.now(); const pcmSourcePath = join(input.workDir, "audio-source.wav"); const duration = (input.totalFrames * input.fps.den) / input.fps.num; @@ -111,7 +121,7 @@ export async function runAudioStage(input: AudioStageInput): Promise