From 5a937d19c7a63c7d6e13f41237c72f12743e510f Mon Sep 17 00:00:00 2001 From: ttbombadil Date: Fri, 9 Oct 2026 12:01:14 +0200 Subject: [PATCH] fix(temp): remove compile build paths and sweep the shared temp directory Every distinct compile left its build directory behind: the compile worker's build-output/ was never evicted, and main-thread compiles without --build-path made arduino-cli keep one directory per random sketch path. The periodic sweeper scanned process.cwd()/temp, so finished sandbox runs renamed to /unosim-temp/.cleanup were never removed either. - ArduinoCompiler uses one build directory per compile (the caller's or its own sibling of the sketch directory) and removes it after the compile, also on failure and cache hits; the HEX stays in the caches. - sweepTempArtifacts removes old .cleanup items in the legacy and the shared sandbox temp directory and stale worker build-output entries. - SandboxRunner, compile worker and sweeper share one path definition. Co-Authored-By: Claude Opus 5.5 --- docs/INSTALL_SERVER.md | 5 + server/index.ts | 47 ++------ server/services/arduino-compiler.ts | 12 ++- server/services/sandbox-runner.ts | 5 +- server/services/temp-artifact-sweep.ts | 66 ++++++++++++ server/services/workers/compile-worker.ts | 4 +- .../compile-build-path-cleanup.test.ts | 101 ++++++++++++++++++ .../services/temp-artifact-sweep.test.ts | 92 ++++++++++++++++ 8 files changed, 283 insertions(+), 49 deletions(-) create mode 100644 server/services/temp-artifact-sweep.ts create mode 100644 tests/server/services/compile-build-path-cleanup.test.ts create mode 100644 tests/server/services/temp-artifact-sweep.test.ts diff --git a/docs/INSTALL_SERVER.md b/docs/INSTALL_SERVER.md index f62d0356c..2b9240870 100644 --- a/docs/INSTALL_SERVER.md +++ b/docs/INSTALL_SERVER.md @@ -92,6 +92,11 @@ die aktuelle Entscheidung zur optionalen Benutzeranmeldung steht in - Der Docker-Socket ist nur für den UnoSim-Server verfügbar. - Temporäre Build-Pfade werden unter `UNOSIM_SHARED_TEMP_DIR` am identischen Host- und Containerpfad gemountet. +- Jeder Compile entfernt sein Build-Verzeichnis danach selbst; das HEX bleibt + in den Caches unter `storage/`. Der Server entfernt minütlich, was älter als + 5 Minuten ist: Sketch-Verzeichnisse beendeter Läufe (`unosim-temp/*.cleanup`) + und liegengebliebene Compile-Build-Verzeichnisse + (`unosim-worker-build/worker_*/build-output/*`). - Jeder Sandbox-Container trägt das Label `unosim.owner`: mit `UNOSIM_INSTANCE_ID` den Wert `instance.` (Compose setzt `unosim-server`), sonst `:`. Beim Start und am Ende eines diff --git a/server/index.ts b/server/index.ts index 59146c05b..6ba1769ee 100644 --- a/server/index.ts +++ b/server/index.ts @@ -17,6 +17,7 @@ import { getStartupConfigurationEntries, } from "./startup-access"; import { apiRateLimitKey, shouldSkipApiRateLimit } from "./rate-limit-policy"; +import { sweepTempArtifacts } from "./services/temp-artifact-sweep"; import { safeErrorLogMetadata, safeErrorStackFrames, @@ -28,50 +29,16 @@ function parseAllowedFrameAncestors(): string[] { return Array.from(new Set(config.server.allowedFrameAncestors)); } -// Cleanup service: Delete old .cleanup.json files and .cleanup directories (> 5 minutes old) +// Cleanup service: removes abandoned run and compile artifacts (> 5 minutes old) +// from the shared temp directory that sandbox runs and compile workers use. function startCleanupService(): NodeJS.Timeout { const CLEANUP_INTERVAL_MS = 60 * 1000; // Check every minute - const CLEANUP_AGE_MS = 5 * 60 * 1000; // Delete files/dirs older than 5 minutes + const legacyTempDir = path.join(process.cwd(), "temp"); return setInterval(async () => { - try { - const tempDir = path.join(process.cwd(), "temp"); - try { - await fs.promises.access(tempDir); - } catch { - return; // tempDir doesn't exist - } - - const items = await fs.promises.readdir(tempDir); - const now = Date.now(); - let deletedCount = 0; - - for (const item of items) { - const itemPath = path.join(tempDir, item); - const stats = await fs.promises.stat(itemPath); - const age = now - stats.mtimeMs; - - // Delete old .cleanup.json files - if (item.endsWith(".cleanup.json") && age > CLEANUP_AGE_MS) { - await fs.promises.unlink(itemPath); - deletedCount++; - } - // Delete old .cleanup directories - else if ( - item.endsWith(".cleanup") && - stats.isDirectory() && - age > CLEANUP_AGE_MS - ) { - await fs.promises.rm(itemPath, { recursive: true, force: true }); - deletedCount++; - } - } - - if (deletedCount > 0) { - console.log(`[Cleanup] Deleted ${deletedCount} old temp items`); - } - } catch { - // Silently handle cleanup errors + const deletedCount = await sweepTempArtifacts({ legacyTempDir }); + if (deletedCount > 0) { + console.log(`[Cleanup] Deleted ${deletedCount} old temp items`); } }, CLEANUP_INTERVAL_MS); } diff --git a/server/services/arduino-compiler.ts b/server/services/arduino-compiler.ts index b1fe3edfe..abbc5b7e7 100644 --- a/server/services/arduino-compiler.ts +++ b/server/services/arduino-compiler.ts @@ -18,6 +18,7 @@ import { resolvePathWithinRoot } from "../security/safe-paths"; import { ensureTempDirs, cleanupSketchDirs, + robustCleanupDir, } from "./compiler/temp-fs"; import { writeBinaryToStorage, @@ -181,6 +182,10 @@ export class ArduinoCompiler { const sketchDir = resolvePathWithinRoot(baseTempDir, sketchId); const sketchFile = resolvePathWithinRoot(sketchDir, `${sketchId}.ino`); + // One build directory per compile, removed afterwards: the HEX lives on in the + // caches. Without --build-path arduino-cli would keep a directory per (random) + // sketch path in its own cache. + const buildPath = options?.buildPath ?? resolvePathWithinRoot(baseTempDir, `${sketchId}-build`); // Pre-compilation validation and parsing const parser = new CodeParser(); @@ -248,9 +253,7 @@ export class ArduinoCompiler { // 3. Create directories and process headers await mkdir(sketchDir, { recursive: true }); - if (options?.buildPath) { - await mkdir(options.buildPath, { recursive: true }).catch(() => {}); - } + await mkdir(buildPath, { recursive: true }).catch(() => {}); if (options?.buildCachePath) { await mkdir(options.buildCachePath, { recursive: true }).catch(() => {}); } @@ -266,7 +269,7 @@ export class ArduinoCompiler { // 4. Run Arduino CLI compilation const cliConfig: CLICompileConfig = { fqbn: options?.fqbn || this.defaultFqbn, - buildPath: options?.buildPath, + buildPath, buildCachePath: options?.buildCachePath || this.defaultBuildCachePath, }; const cliResult = await compileWithArduinoCli(sketchFile, cliConfig, this.processExecutor); @@ -319,6 +322,7 @@ export class ArduinoCompiler { ioRegistry, }; } finally { + await robustCleanupDir(buildPath); await this._cleanupSketchDirs(sketchDir, baseTempDir, tempRoot); } } diff --git a/server/services/sandbox-runner.ts b/server/services/sandbox-runner.ts index 851b2066f..26b8ac0f1 100644 --- a/server/services/sandbox-runner.ts +++ b/server/services/sandbox-runner.ts @@ -4,9 +4,8 @@ import { ProcessController, type IProcessController } from "./process-controller"; import { mkdir } from "node:fs/promises"; import { existsSync } from "node:fs"; -import { join } from "node:path"; import { Logger } from "@shared/logger"; -import { getFastTmpBaseDir } from "@shared/utils/temp-paths"; +import { sandboxTempDir } from "./temp-artifact-sweep"; import { ArduinoOutputParser as StderrParser } from "./arduino-output-parser"; import { RegistryManager } from "./registry-manager"; import { SimulationTimeoutManager } from "./simulation-timeout-manager"; @@ -63,7 +62,7 @@ export class SandboxRunner { constructor(options?: { tempDir?: string; processController?: IProcessController }) { this.processController = options?.processController ?? new ProcessController(); - this.tempDir = options?.tempDir ?? join(getFastTmpBaseDir(), "unosim-temp"); + this.tempDir = options?.tempDir ?? sandboxTempDir(); this.timeoutManager = new SimulationTimeoutManager(); this.fileBuilder = new SketchFileBuilder(this.tempDir); this.localCompiler = new LocalCompiler(); diff --git a/server/services/temp-artifact-sweep.ts b/server/services/temp-artifact-sweep.ts new file mode 100644 index 000000000..692c12cc7 --- /dev/null +++ b/server/services/temp-artifact-sweep.ts @@ -0,0 +1,66 @@ +import { readdir, rm, stat } from "node:fs/promises"; +import { join } from "node:path"; +import { getFastTmpBaseDir } from "../../shared/utils/temp-paths.ts"; + +/** Abandoned temp artifacts are removed once they are older than this. */ +export const TEMP_ARTIFACT_MAX_AGE_MS = 5 * 60 * 1000; + +/** Sketch directories of simulation runs; finished ones are renamed to `.cleanup`. */ +export function sandboxTempDir(): string { + return join(getFastTmpBaseDir(), "unosim-temp"); +} + +/** Per-worker directories of the compile workers. */ +export function workerBuildRoot(): string { + return join(getFastTmpBaseDir(), "unosim-worker-build"); +} + +interface SweepOptions { + /** The process-relative temp directory swept since before the shared temp directory existed. */ + legacyTempDir: string; + now?: number; + maxAgeMs?: number; +} + +/** + * Removes temp artifacts that no run or compile uses any more, in the + * directories that sandbox runs and compile workers actually write to: + * `.cleanup` directories and `.cleanup.json` files of finished runs, and + * per-sketch build directories that a compile left in `build-output`. + * Returns the number of removed entries; never throws. + */ +export async function sweepTempArtifacts(options: SweepOptions): Promise { + const cutoff = (options.now ?? Date.now()) - (options.maxAgeMs ?? TEMP_ARTIFACT_MAX_AGE_MS); + const isCleanupItem = (name: string) => name.endsWith(".cleanup") || name.endsWith(".cleanup.json"); + let removed = await removeStaleEntries(options.legacyTempDir, cutoff, isCleanupItem); + removed += await removeStaleEntries(sandboxTempDir(), cutoff, isCleanupItem); + for (const worker of await listEntries(workerBuildRoot())) { + if (!worker.startsWith("worker_")) continue; + removed += await removeStaleEntries(join(workerBuildRoot(), worker, "build-output"), cutoff, () => true); + } + return removed; +} + +async function listEntries(dir: string): Promise { + try { + return await readdir(dir); + } catch { + return []; + } +} + +async function removeStaleEntries(dir: string, cutoff: number, matches: (name: string) => boolean): Promise { + let removed = 0; + for (const name of await listEntries(dir)) { + if (!matches(name)) continue; + const path = join(dir, name); + try { + if ((await stat(path)).mtimeMs >= cutoff) continue; + await rm(path, { recursive: true, force: true }); + removed++; + } catch { + // Removed concurrently or not accessible; the next sweep retries. + } + } + return removed; +} diff --git a/server/services/workers/compile-worker.ts b/server/services/workers/compile-worker.ts index 427ced5d4..c367a73b2 100644 --- a/server/services/workers/compile-worker.ts +++ b/server/services/workers/compile-worker.ts @@ -15,7 +15,7 @@ import { parentPort, workerData } from "node:worker_threads"; import { Logger } from "../../../shared/logger.ts"; import { safeErrorLogMetadata } from "../safe-error-log-metadata.ts"; -import { getFastTmpBaseDir } from "../../../shared/utils/temp-paths.ts"; +import { workerBuildRoot } from "../temp-artifact-sweep.ts"; import { type CompileRequestPayload, type AnyWorkerMessage, @@ -64,7 +64,7 @@ const CORE_METADATA_TTL_MS = 5 * 60 * 1000; const resolvedWorkerId = Number(workerData?.workerId || 1); // Safe usage of writable temp directory: worker-specific isolation by concurrent worker_${workerId}, // and all files are temporary build artifacts with automatic cleanup after compilation. -const WORKER_BUILD_DIR = join(getFastTmpBaseDir(), "unosim-worker-build", `worker_${resolvedWorkerId}`); +const WORKER_BUILD_DIR = join(workerBuildRoot(), `worker_${resolvedWorkerId}`); const BINARY_STORAGE_DIR = join(process.cwd(), "storage", "binaries"); let cachedLibFingerprint: { value: string; expiresAt: number } | null = null; diff --git a/tests/server/services/compile-build-path-cleanup.test.ts b/tests/server/services/compile-build-path-cleanup.test.ts new file mode 100644 index 000000000..7160030ae --- /dev/null +++ b/tests/server/services/compile-build-path-cleanup.test.ts @@ -0,0 +1,101 @@ +import { randomUUID } from "node:crypto"; +import { mkdir, mkdtemp, readdir, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ArduinoCompiler } from "../../../server/services/arduino-compiler"; +import * as cliRunner from "../../../server/services/compiler/cli-runner"; + +const HEX = Buffer.from(":00000001FF\n"); +let root: string; + +/** Stands in for arduino-cli: writes typical build artifacts into the build path. */ +function fakeCli(success: boolean) { + return vi.spyOn(cliRunner, "compileWithArduinoCli").mockImplementation(async (_sketchFile, cliConfig) => { + if (cliConfig.buildPath) { + await mkdir(join(cliConfig.buildPath, "sketch"), { recursive: true }); + await writeFile(join(cliConfig.buildPath, "sketch", "sketch.ino.cpp.o"), "object"); + await writeFile(join(cliConfig.buildPath, "sketch.ino.hex"), HEX); + } + return success + ? { success: true, output: "Sketch uses 1 bytes.\n\nBoard: Arduino UNO", binary: HEX } + : { success: false, output: "", errors: "sketch.ino:1:1: error: boom" }; + }); +} + +function uniqueSketch(): string { + return `// ${randomUUID()}\nvoid setup(){}\nvoid loop(){}\n`; +} + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), "unosim-build-path-")); +}); + +afterEach(async () => { + vi.restoreAllMocks(); + await rm(root, { recursive: true, force: true }); +}); + +describe("compile build paths", () => { + it("leaves no per-sketch build directory behind after many distinct compiles", async () => { + fakeCli(true); + const compiler = await ArduinoCompiler.create(); + const buildOutput = join(root, "build-output"); + + for (let index = 0; index < 20; index++) { + // The compile worker's layout: one build path per sketch hash, created before the compile. + const buildPath = join(buildOutput, `hash-${index}`); + await mkdir(buildPath, { recursive: true }); + const result = await compiler.compile(uniqueSketch(), undefined, root, { + buildPath, + hexCacheDir: join(root, "hex-cache"), + }); + expect(result.success).toBe(true); + expect(result.binary).toEqual(HEX); + } + + expect(await readdir(buildOutput)).toEqual([]); + // The HEX cache keeps the useful artifact. + expect((await readdir(join(root, "hex-cache"))).some((entry) => entry.endsWith(".hex"))).toBe(true); + }); + + it("removes the build path of a failed compile", async () => { + fakeCli(false); + const compiler = await ArduinoCompiler.create(); + const buildPath = join(root, "build-output", "failed"); + await mkdir(buildPath, { recursive: true }); + + const result = await compiler.compile(uniqueSketch(), undefined, root, { buildPath, hexCacheDir: join(root, "hex-cache") }); + + expect(result.success).toBe(false); + expect(await readdir(join(root, "build-output"))).toEqual([]); + }); + + it("removes the build path when the compile is answered from the cache", async () => { + const cli = fakeCli(true); + const compiler = await ArduinoCompiler.create(); + const code = uniqueSketch(); + const hexCacheDir = join(root, "hex-cache"); + await compiler.compile(code, undefined, root, { buildPath: join(root, "build-output", "first"), hexCacheDir }); + const cachedPath = join(root, "build-output", "cached"); + await mkdir(cachedPath, { recursive: true }); + + await compiler.compile(code, undefined, root, { buildPath: cachedPath, hexCacheDir }); + + expect(cli).toHaveBeenCalledTimes(1); + expect(await readdir(join(root, "build-output"))).toEqual([]); + }); + + it("gives a compile without an explicit build path its own temporary one and removes it", async () => { + const cli = fakeCli(true); + const compiler = await ArduinoCompiler.create(); + + await compiler.compile(uniqueSketch(), undefined, root, { hexCacheDir: join(root, "hex-cache") }); + + // Without a build path arduino-cli keeps one directory per (random) sketch path in its own cache. + const buildPath = cli.mock.calls[0][1].buildPath; + expect(buildPath).toBeDefined(); + expect(buildPath!.startsWith(root)).toBe(true); + expect((await readdir(root)).filter((entry) => entry !== "hex-cache")).toEqual([]); + }); +}); diff --git a/tests/server/services/temp-artifact-sweep.test.ts b/tests/server/services/temp-artifact-sweep.test.ts new file mode 100644 index 000000000..bb6bbc7f7 --- /dev/null +++ b/tests/server/services/temp-artifact-sweep.test.ts @@ -0,0 +1,92 @@ +import { mkdir, mkdtemp, readdir, rm, utimes, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { FilesystemHelper } from "../../../server/services/sandbox/filesystem-helper"; +import { + sandboxTempDir, + sweepTempArtifacts, + TEMP_ARTIFACT_MAX_AGE_MS, + workerBuildRoot, +} from "../../../server/services/temp-artifact-sweep"; + +let base: string; +const ageAll = async (path: string, ageMs: number) => { + const time = new Date(Date.now() - ageMs); + await utimes(path, time, time); +}; + +beforeEach(async () => { + base = await mkdtemp(join(tmpdir(), "unosim-sweep-")); + vi.stubEnv("UNOSIM_SHARED_TEMP_DIR", base); +}); + +afterEach(async () => { + vi.unstubAllEnvs(); + await rm(base, { recursive: true, force: true }); +}); + +describe("temp artifact sweep", () => { + it("uses the shared temp directory that sandbox runs and compile workers write to", () => { + expect(sandboxTempDir()).toBe(join(base, "unosim-temp")); + expect(workerBuildRoot()).toBe(join(base, "unosim-worker-build")); + }); + + it("removes the sketch directories of many finished runs once they are old enough", async () => { + const helper = new FilesystemHelper({ clearCreatedSketchDir: vi.fn() } as never, {} as never); + for (let run = 0; run < 25; run++) { + const sketchDir = join(sandboxTempDir(), `run-${run}`); + await mkdir(sketchDir, { recursive: true }); + await writeFile(join(sketchDir, "sketch.cpp"), "int main(){}"); + // What a finished run leaves behind: the directory renamed for deferred removal. + expect(helper.attemptCleanupDir(sketchDir)).toBe(true); + await ageAll(`${sketchDir}.cleanup`, TEMP_ARTIFACT_MAX_AGE_MS + 1_000); + } + const fresh = join(sandboxTempDir(), "fresh.cleanup"); + const active = join(sandboxTempDir(), "active-run"); + await mkdir(fresh); + await mkdir(active); + await ageAll(active, TEMP_ARTIFACT_MAX_AGE_MS + 1_000); + + const removed = await sweepTempArtifacts({ legacyTempDir: join(base, "legacy") }); + + expect(removed).toBe(25); + expect((await readdir(sandboxTempDir())).sort()).toEqual(["active-run", "fresh.cleanup"]); + }); + + it("removes stale per-sketch compile build directories but keeps recent ones and the caches", async () => { + const buildOutput = join(workerBuildRoot(), "worker_1", "build-output"); + for (let index = 0; index < 10; index++) { + const dir = join(buildOutput, `hash-${index}`); + await mkdir(dir, { recursive: true }); + await ageAll(dir, TEMP_ARTIFACT_MAX_AGE_MS + 1_000); + } + await mkdir(join(buildOutput, "in-progress")); + const sketchDir = join(workerBuildRoot(), "worker_1", "some-sketch"); + await mkdir(sketchDir); + await ageAll(sketchDir, TEMP_ARTIFACT_MAX_AGE_MS + 1_000); + + const removed = await sweepTempArtifacts({ legacyTempDir: join(base, "legacy") }); + + expect(removed).toBe(10); + expect(await readdir(buildOutput)).toEqual(["in-progress"]); + expect((await readdir(join(workerBuildRoot(), "worker_1"))).sort()).toEqual(["build-output", "some-sketch"]); + }); + + it("keeps sweeping the legacy temp directory", async () => { + const legacy = join(base, "legacy"); + await mkdir(join(legacy, "old.cleanup"), { recursive: true }); + await writeFile(join(legacy, "old.cleanup.json"), "{}"); + await writeFile(join(legacy, "keep.json"), "{}"); + await ageAll(join(legacy, "old.cleanup"), TEMP_ARTIFACT_MAX_AGE_MS + 1_000); + await ageAll(join(legacy, "old.cleanup.json"), TEMP_ARTIFACT_MAX_AGE_MS + 1_000); + await ageAll(join(legacy, "keep.json"), TEMP_ARTIFACT_MAX_AGE_MS + 1_000); + + expect(await sweepTempArtifacts({ legacyTempDir: legacy })).toBe(2); + expect(await readdir(legacy)).toEqual(["keep.json"]); + }); + + it("tolerates missing directories", async () => { + await expect(sweepTempArtifacts({ legacyTempDir: join(base, "missing") })).resolves.toBe(0); + }); +});