From f4aa894fe75a7517169fdb18eba69fb0778cef7c Mon Sep 17 00:00:00 2001 From: Bastiaan <17990055+bbassie@users.noreply.github.com> Date: Sun, 13 Sep 2026 02:37:26 +0200 Subject: [PATCH] Give each safeOutputFile call its own temporary file (#1026) Two writes to the same file at the same time shared one temporary name, so the second write clobbered the first's temporary and the first rename left the second call with nothing to rename, failing with ENOENT. The host does exactly that when it saves an instance config while the instance saves it too. The temporary now carries a random tag and is created exclusively, so concurrent writers never touch each other's file. Since the temporary is always new, the mode is applied on creation and the chmod for reused leftovers goes away. A failed rename removes its temporary instead of leaving it for the next write to overwrite. Co-authored-by: Claude Fable 5.1 --- packages/lib/src/file_ops.ts | 23 ++++++++++++----------- test/lib/file_ops.js | 30 ++++++++++++++++++++++++++---- 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/packages/lib/src/file_ops.ts b/packages/lib/src/file_ops.ts index 908f57a3..4b3028ca 100644 --- a/packages/lib/src/file_ops.ts +++ b/packages/lib/src/file_ops.ts @@ -137,11 +137,11 @@ export async function getTempFile(prefix = "tmp.", suffix = "", tmpdir = "./") { * file. If the operation fails because the parent directory does not exist, it * attempts to create the directory, including any ancestors and then retries * the operation. The name of the temporary file is the same as the target file - * with the suffix `.tmp` added before the extension. + * with a random tag and the suffix `.tmp` added before the extension, so + * concurrent writes to the same file do not interfere with each other. * - * If the operation fails it may leave behind the temporary file. This - * should not be too much of an issue as the next time the same file is - * written the temporary will be overwritten and renamed to the target file. + * If the operation fails the temporary file is removed. Should the process + * die in the middle of writing the temporary file it is left behind. * * @param file - Path to file to write. * @param data - Content to write. @@ -156,23 +156,24 @@ export async function safeOutputFile( >, ) { let { dir, name, ext } = path.parse(file); - let temporary = path.join(dir, `${name}.tmp${ext}`); + let temporary = path.join(dir, `${name}.${crypto.randomBytes(8).toString("hex")}.tmp${ext}`); try { - await fs.writeFile(temporary, data, { ...options, flush: true }); + await fs.writeFile(temporary, data, { ...options, flag: "wx", flush: true }); } catch (err: any) { if (err.code === "ENOENT") { // Try creating the folder and then retry the operation. await fs.mkdir(dir, { recursive: true }); - await fs.writeFile(temporary, data, { ...options, flush: true }); + await fs.writeFile(temporary, data, { ...options, flag: "wx", flush: true }); } else { throw err; } } - if (options?.mode !== undefined) { - // writeFile only applies mode when creating, a leftover temporary keeps its old mode. - await fs.chmod(temporary, options.mode); + try { + await fs.rename(temporary, file); + } catch (err: any) { + await fs.unlink(temporary).catch(() => {}); + throw err; } - await fs.rename(temporary, file); } // Reserved names by almost all filesystems diff --git a/test/lib/file_ops.js b/test/lib/file_ops.js index 2f4993e7..b338d927 100644 --- a/test/lib/file_ops.js +++ b/test/lib/file_ops.js @@ -87,29 +87,51 @@ describe("lib/file_ops", function() { }); describe("safeOutputFile()", function() { + async function assertNoTemporary(target) { + let { dir, name } = path.parse(target); + let entries = (await fs.readdir(dir)).filter( + entry => entry.startsWith(`${name}.`) && entry !== path.basename(target) + ); + assert.deepEqual(entries, [], "temporary was left behind"); + } it("should write new target file", async function() { let target = path.join(baseDir, "safe", "simple.txt"); await lib.safeOutputFile(target, "a text file", "utf8"); - await assert.rejects(fs.access(target.replace(".txt", ".tmp.txt")), "temporary was left behind"); + await assertNoTemporary(target); assert.equal(await fs.readFile(target, "utf8"), "a text file"); }); it("should overwrite existing target file", async function() { let target = path.join(baseDir, "safe", "exists.txt"); await fs.writeFile(target, "previous", "utf8"); await lib.safeOutputFile(target, "current", "utf8"); - await assert.rejects(fs.access(target.replace(".txt", ".tmp.txt")), "temporary was left behind"); + await assertNoTemporary(target); assert.equal(await fs.readFile(target, "utf8"), "current"); }); - it("should apply mode to a leftover temporary file", async function() { + it("should apply mode to the target file", async function() { if (process.platform === "win32") { this.skip(); } let target = path.join(baseDir, "safe", "mode.txt"); - await fs.writeFile(target.replace(".txt", ".tmp.txt"), "stale", { mode: 0o644 }); await lib.safeOutputFile(target, "private", { mode: 0o600 }); assert.equal((await fs.stat(target)).mode & 0o777, 0o600); assert.equal(await fs.readFile(target, "utf8"), "private"); }); + it("should not interfere between concurrent writes to the same file", async function() { + let target = path.join(baseDir, "safe", "concurrent.txt"); + let writes = []; + for (let i = 0; i < 20; i++) { + writes.push(lib.safeOutputFile(target, `write ${i}`, "utf8")); + } + await Promise.all(writes); + await assertNoTemporary(target); + assert.match(await fs.readFile(target, "utf8"), /^write \d+$/); + }); + it("should remove the temporary file when the rename fails", async function() { + let target = path.join(baseDir, "safe", "rename-fails"); + await fs.mkdir(target, { recursive: true }); + await assert.rejects(lib.safeOutputFile(target, "data", "utf8")); + await assertNoTemporary(target); + }); it("should handle creating file in current working directory", async function() { let target = "temporary-file-made-to-test-cwd.txt"; try {