diff --git a/workspaces/libnpmversion/lib/write-json.js b/workspaces/libnpmversion/lib/write-json.js index 2f19953d75d28..24e58411ba6f6 100644 --- a/workspaces/libnpmversion/lib/write-json.js +++ b/workspaces/libnpmversion/lib/write-json.js @@ -1,5 +1,6 @@ // write the json back, preserving the line breaks and indent -const { writeFile } = require('node:fs/promises') +const { writeFile, rename, unlink } = require('node:fs/promises') +const { randomBytes } = require('node:crypto') const kIndent = Symbol.for('indent') const kNewline = Symbol.for('newline') @@ -11,5 +12,16 @@ module.exports = async (path, pkg) => { delete pkg._id const raw = JSON.stringify(pkg, null, indent) + '\n' const data = newline === '\n' ? raw : raw.split('\n').join(newline) - return writeFile(path, data) + + // Write to a temp file and rename into place so a concurrent reader (e.g. + // another `npm version` process resolving workspaces) never observes a + // truncated or partially written package.json. + const tmpPath = `${path}.${randomBytes(6).toString('hex')}.tmp` + try { + await writeFile(tmpPath, data) + await rename(tmpPath, path) + } catch (err) { + await unlink(tmpPath).catch(() => {}) + throw err + } } diff --git a/workspaces/libnpmversion/test/write-json.js b/workspaces/libnpmversion/test/write-json.js index 98c67897c53c9..4d7c3a8e1132f 100644 --- a/workspaces/libnpmversion/test/write-json.js +++ b/workspaces/libnpmversion/test/write-json.js @@ -1,7 +1,7 @@ const t = require('tap') const path = require('node:path') const writeJson = require('../lib/write-json.js') -const { readFile } = require('node:fs/promises') +const { readFile, readdir } = require('node:fs/promises') const kIndent = Symbol.for('indent') const kNewline = Symbol.for('newline') @@ -52,3 +52,44 @@ t.test('write json with newlines and indent set', async t => { t.end() }) + +t.test('writes atomically, never leaving a truncated file for concurrent readers', async t => { + const dir = t.testdir() + const file = path.join(dir, 'package.json') + + // A concurrent reader must always see either the old complete contents or + // the new complete contents, never a partial/empty file mid-write. + await writeJson(file, { a: 1 }) + let sawTruncated = false + const reads = [] + for (let i = 0; i < 20; i++) { + reads.push(readFile(file, 'utf8').then(str => { + if (str.length === 0 || !str.trim().endsWith('}')) { + sawTruncated = true + } + return null + }).catch(() => {})) + } + await writeJson(file, { a: 2, b: [1, 2, 3] }) + await Promise.all(reads) + + t.equal(sawTruncated, false, 'never observed a truncated file') + const final = JSON.parse(await readFile(file, 'utf8')) + t.same(final, { a: 2, b: [1, 2, 3] }) + + const leftover = (await readdir(dir)).filter(f => f.endsWith('.tmp')) + t.same(leftover, [], 'no leftover temp files') +}) + +t.test('cleans up the temp file when the write fails', async t => { + const dir = t.testdir() + // path is a directory, not a file, so renaming onto it will fail + const file = path.join(dir, 'package.json') + const { mkdir } = require('node:fs/promises') + await mkdir(file) + + await t.rejects(writeJson(file, { a: 1 })) + + const leftover = (await readdir(dir)).filter(f => f.endsWith('.tmp')) + t.same(leftover, [], 'temp file was cleaned up after failure') +})