From 24de70a507909f8fa5f1995029c8cd58ea2aa67a Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sat, 10 Oct 2026 18:58:44 -0400 Subject: [PATCH] fix(fs): retry bounded Windows atomic rename sharing failures --- scripts/lib/atomic-write.js | 28 +++++- tests/lib/atomic-write.test.js | 156 +++++++++++++++++++++++++++++++++ 2 files changed, 181 insertions(+), 3 deletions(-) create mode 100644 tests/lib/atomic-write.test.js diff --git a/scripts/lib/atomic-write.js b/scripts/lib/atomic-write.js index 9e9e524fe..c85c7dad6 100644 --- a/scripts/lib/atomic-write.js +++ b/scripts/lib/atomic-write.js @@ -4,6 +4,30 @@ const crypto = require('crypto'); const fs = require('fs'); const path = require('path'); +function renameAtomic(tempPath, resolvedPath, options) { + const deadline = performance.now() + 750; + let attempts = 0; + let sleeper; + while (true) { + // A sharing violation may outlast the original validation. Recheck both + // the parent and destination ownership before every publication attempt. + if (options.validateParent) options.validateParent(); + if (options.beforeRename) options.beforeRename(); + try { + fs.renameSync(tempPath, resolvedPath); + return; + } catch (error) { + attempts++; + const remaining = deadline - performance.now(); + if (process.platform !== 'win32' + || !['EPERM', 'EACCES', 'EBUSY'].includes(error.code) + || attempts >= 21 || remaining <= 0) throw error; + sleeper ||= new Int32Array(new SharedArrayBuffer(4)); + Atomics.wait(sleeper, 0, 0, Math.min(25, remaining)); + } + } +} + function writeFileAtomic(filePath, content, options = {}) { const resolvedPath = path.resolve(filePath); const parentDir = path.dirname(resolvedPath); @@ -25,9 +49,7 @@ function writeFileAtomic(filePath, content, options = {}) { fs.fsyncSync(descriptor); fs.closeSync(descriptor); descriptor = undefined; - if (options.validateParent) options.validateParent(); - if (options.beforeRename) options.beforeRename(); - fs.renameSync(tempPath, resolvedPath); + renameAtomic(tempPath, resolvedPath, options); } catch (error) { if (descriptor !== undefined) { fs.closeSync(descriptor); diff --git a/tests/lib/atomic-write.test.js b/tests/lib/atomic-write.test.js new file mode 100644 index 000000000..7c4e955c7 --- /dev/null +++ b/tests/lib/atomic-write.test.js @@ -0,0 +1,156 @@ +'use strict'; + +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const vm = require('vm'); + +const filename = path.resolve(__dirname, '../../scripts/lib/atomic-write.js'); +const source = fs.readFileSync(filename, 'utf8'); + +// Load the actual helper with a local platform/filesystem facade. File creation, +// fsync, replacement and cleanup remain real; fault injection is module-local. +function loadHelper(platform, rename, { clockStep = 25 } = {}) { + let elapsed = 0; + let waits = 0; + const filesystem = Object.create(fs); + filesystem.renameSync = rename; + const module = { exports: {} }; + vm.runInNewContext(source, { + module, exports: module.exports, + require: name => name === 'fs' ? filesystem : require(name), + process: { platform, pid: process.pid }, + performance: { now: () => elapsed }, + Atomics: { wait: () => { elapsed += clockStep; waits++; } }, + SharedArrayBuffer, Int32Array, + }, { filename }); + return { ...module.exports, waits: () => waits }; +} + +let passed = 0; +let failed = 0; +function test(name, run) { + const root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-atomic-rename-'))); + const destination = path.join(root, 'state.json'); + fs.writeFileSync(destination, 'old'); + try { run({ root, destination }); passed++; console.log(`PASS ${name}`); } + catch (error) { failed++; console.error(`FAIL ${name}: ${error.stack}`); } + finally { fs.rmSync(root, { recursive: true, force: true }); } +} +const sharingError = code => Object.assign(new Error(`Injected sharing violation: ${code}`), { code }); + +for (const code of ['EPERM', 'EACCES', 'EBUSY']) { + test(`Windows ${code} retries atomic replacement without disturbing old bytes`, ({ root, destination }) => { + let attempts = 0; + let guards = 0; + const helper = loadHelper('win32', (temporary, target) => { + attempts++; + assert.strictEqual(fs.readFileSync(target, 'utf8'), 'old'); + assert.strictEqual(fs.readFileSync(temporary, 'utf8'), 'new'); + if (attempts < 3) throw sharingError(code); + fs.renameSync(temporary, target); + }); + helper.writeFileAtomic(destination, 'new', { beforeRename: () => { guards++; } }); + assert.strictEqual(attempts, 3); + assert.strictEqual(guards, attempts); + assert.strictEqual(helper.waits(), 2); + assert.strictEqual(fs.readFileSync(destination, 'utf8'), 'new'); + assert.deepStrictEqual(fs.readdirSync(root), ['state.json']); + }); + + test(`Windows permanent ${code} preserves destination and removes private staging`, ({ root, destination }) => { + let attempts = 0; + const expected = sharingError(code); + const helper = loadHelper('win32', (_temporary, target) => { + attempts++; + assert.strictEqual(fs.readFileSync(target, 'utf8'), 'old'); + throw expected; + }); + assert.throws(() => helper.writeFileAtomic(destination, 'new'), error => error === expected); + assert.strictEqual(attempts, 21, 'Hard attempt bound applies even before the deadline'); + assert.strictEqual(helper.waits(), 20); + assert.strictEqual(fs.readFileSync(destination, 'utf8'), 'old'); + assert.deepStrictEqual(fs.readdirSync(root), ['state.json']); + }); +} + +test('Windows retries also stop at the elapsed deadline', ({ root, destination }) => { + let attempts = 0; + const expected = sharingError('EPERM'); + const helper = loadHelper('win32', () => { attempts++; throw expected; }, { clockStep: 250 }); + assert.throws(() => helper.writeFileAtomic(destination, 'new'), error => error === expected); + assert.strictEqual(attempts, 4); + assert.deepStrictEqual(fs.readdirSync(root), ['state.json']); +}); + +for (const platform of ['linux', 'darwin']) { + for (const code of ['EPERM', 'EACCES', 'EBUSY']) { + test(`${platform} ${code} propagates immediately with original bytes intact`, ({ root, destination }) => { + let attempts = 0; + const expected = sharingError(code); + const helper = loadHelper(platform, () => { attempts++; throw expected; }); + assert.throws(() => helper.writeFileAtomic(destination, 'new'), error => error === expected); + assert.strictEqual(attempts, 1); + assert.strictEqual(helper.waits(), 0); + assert.strictEqual(fs.readFileSync(destination, 'utf8'), 'old'); + assert.deepStrictEqual(fs.readdirSync(root), ['state.json']); + }); + } +} + +test('Windows unrelated IO failure propagates immediately', ({ root, destination }) => { + let attempts = 0; + const expected = sharingError('EIO'); + const helper = loadHelper('win32', () => { attempts++; throw expected; }); + assert.throws(() => helper.writeFileAtomic(destination, 'new'), error => error === expected); + assert.strictEqual(attempts, 1); + assert.strictEqual(helper.waits(), 0); + assert.deepStrictEqual(fs.readdirSync(root), ['state.json']); +}); + +test('ownership revocation between sharing failures prevents another rename', ({ root, destination }) => { + let attempts = 0; + let guards = 0; + const revoked = new Error('Destination ownership revoked'); + const helper = loadHelper('win32', () => { attempts++; throw sharingError('EPERM'); }); + assert.throws(() => helper.writeFileAtomic(destination, 'new', { + beforeRename() { if (++guards === 2) throw revoked; }, + }), error => error === revoked); + assert.strictEqual(attempts, 1); + assert.strictEqual(guards, 2); + assert.strictEqual(fs.readFileSync(destination, 'utf8'), 'old'); + assert.deepStrictEqual(fs.readdirSync(root), ['state.json']); +}); + +test('parent replacement during retry preserves both destinations and original private staging', ({ root, destination }) => { + const parent = path.join(root, 'parent'); + const original = path.join(root, 'original'); + fs.mkdirSync(parent); + const target = path.join(parent, 'state.json'); + fs.writeFileSync(target, 'old'); + const identity = fs.statSync(parent).ino; + const revoked = new Error('Parent identity changed'); + let attempts = 0; + const helper = loadHelper('win32', () => { + attempts++; + fs.renameSync(parent, original); + fs.mkdirSync(parent); + fs.writeFileSync(target, 'user replacement'); + throw sharingError('EPERM'); + }); + assert.throws(() => helper.writeFileAtomic(target, 'new', { + validateParent() { if (fs.statSync(parent).ino !== identity) throw revoked; }, + }), error => error === revoked); + assert.strictEqual(attempts, 1); + assert.strictEqual(fs.readFileSync(target, 'utf8'), 'user replacement'); + assert.strictEqual(fs.readFileSync(path.join(original, 'state.json'), 'utf8'), 'old'); + const staging = fs.readdirSync(original).filter(name => name.endsWith('.tmp')); + assert.strictEqual(staging.length, 1, 'Do not clean a pathname through a substituted parent'); + assert.strictEqual(fs.readFileSync(path.join(original, staging[0]), 'utf8'), 'new'); + assert.deepStrictEqual(fs.readdirSync(parent), ['state.json']); + assert.strictEqual(fs.readFileSync(destination, 'utf8'), 'old'); +}); + +console.log(`Results: Passed: ${passed}, Failed: ${failed}`); +process.exitCode = failed ? 1 : 0;