From a4d9297da10da64e372fc181ed05db2f885f8c44 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sun, 11 Oct 2026 13:47:57 -0400 Subject: [PATCH] fix: allow bounded sustained Windows atomic replacement contention --- scripts/lib/atomic-write.js | 6 +++-- tests/lib/atomic-write.test.js | 44 +++++++++++++++++++++++++++++++--- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/scripts/lib/atomic-write.js b/scripts/lib/atomic-write.js index d3f2d14ac..76ff40c4a 100644 --- a/scripts/lib/atomic-write.js +++ b/scripts/lib/atomic-write.js @@ -26,7 +26,9 @@ function readFileWithSharingRetry(filePath, options) { } function renameAtomic(tempPath, resolvedPath, options) { - const deadline = performance.now() + 750; + // Concurrent Windows installers can hold replacement sharing locks beyond + // the reader's shorter retry budget. Keep publication bounded to two seconds. + const deadline = performance.now() + 2000; let attempts = 0; let sleeper; while (true) { @@ -42,7 +44,7 @@ function renameAtomic(tempPath, resolvedPath, options) { const remaining = deadline - performance.now(); if (process.platform !== 'win32' || !['EPERM', 'EACCES', 'EBUSY'].includes(error.code) - || attempts >= 21 || remaining <= 0) throw error; + || attempts >= 81 || remaining <= 0) throw error; sleeper ||= new Int32Array(new SharedArrayBuffer(4)); Atomics.wait(sleeper, 0, 0, Math.min(25, remaining)); } diff --git a/tests/lib/atomic-write.test.js b/tests/lib/atomic-write.test.js index 390ce5772..cfe499ef0 100644 --- a/tests/lib/atomic-write.test.js +++ b/tests/lib/atomic-write.test.js @@ -69,11 +69,34 @@ for (const code of ['EPERM', 'EACCES', 'EBUSY']) { 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(attempts, 81, 'Hard attempt bound applies even before the deadline'); + assert.strictEqual(helper.waits(), 80); assert.strictEqual(fs.readFileSync(destination, 'utf8'), 'old'); assert.deepStrictEqual(fs.readdirSync(root), ['state.json']); }); + + test(`Windows ${code} recovers sustained sharing contention with ownership rechecked`, ({ root, destination }) => { + let attempts = 0; + let parents = 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 <= 50) throw sharingError(code); + fs.renameSync(temporary, target); + }); + helper.writeFileAtomic(destination, 'new', { + validateParent: () => { parents++; }, + beforeRename: () => { guards++; }, + }); + assert.strictEqual(attempts, 51); + assert.strictEqual(guards, attempts); + assert.ok(parents >= attempts); + assert.strictEqual(helper.waits(), 50); + assert.strictEqual(fs.readFileSync(destination, 'utf8'), 'new'); + assert.deepStrictEqual(fs.readdirSync(root), ['state.json']); + }); } test('Windows retries also stop at the elapsed deadline', ({ root, destination }) => { @@ -81,7 +104,7 @@ test('Windows retries also stop at the elapsed deadline', ({ root, destination } 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.strictEqual(attempts, 9); assert.deepStrictEqual(fs.readdirSync(root), ['state.json']); }); @@ -124,6 +147,21 @@ test('ownership revocation between sharing failures prevents another rename', ({ assert.deepStrictEqual(fs.readdirSync(root), ['state.json']); }); +test('late ownership revocation stops sustained contention without publication', ({ root, destination }) => { + let attempts = 0; + let guards = 0; + const revoked = new Error('Destination ownership revoked after sustained contention'); + const helper = loadHelper('win32', () => { attempts++; throw sharingError('EPERM'); }); + assert.throws(() => helper.writeFileAtomic(destination, 'new', { + beforeRename() { if (++guards === 26) throw revoked; }, + }), error => error === revoked); + assert.strictEqual(attempts, 25); + assert.strictEqual(guards, 26); + assert.strictEqual(helper.waits(), 25); + 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');