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 {