From 3e56eaaa0fd8b6e248c06aca439a8c337e4f143c Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Sat, 29 Aug 2026 13:04:06 +0800 Subject: [PATCH] fix(atomic-write): retry transient Windows replacement --- ...-29-windows-atomic-replace-retry.i18n.yaml | 6 ++ ...2026-08-29-windows-atomic-replace-retry.md | 27 +++++ ...6-08-29-windows-atomic-replace-retry.zh.md | 27 +++++ packages/util/atomic-write/README.i18n.yaml | 4 +- packages/util/atomic-write/README.md | 4 +- packages/util/atomic-write/README.zh.md | 4 +- packages/util/atomic-write/src/index.ts | 35 +++++- .../atomic-write/tests/atomic-write.spec.ts | 101 ++++++++++++++++-- 8 files changed, 188 insertions(+), 20 deletions(-) create mode 100644 .agents/notes/implemented/bug-fix/2026-08-29-windows-atomic-replace-retry.i18n.yaml create mode 100644 .agents/notes/implemented/bug-fix/2026-08-29-windows-atomic-replace-retry.md create mode 100644 .agents/notes/implemented/bug-fix/2026-08-29-windows-atomic-replace-retry.zh.md diff --git a/.agents/notes/implemented/bug-fix/2026-08-29-windows-atomic-replace-retry.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-08-29-windows-atomic-replace-retry.i18n.yaml new file mode 100644 index 0000000000..694e117443 --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-08-29-windows-atomic-replace-retry.i18n.yaml @@ -0,0 +1,6 @@ +# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each +# side as of the last confirmed-consistent state. Both languages carry equal authority; +# after editing either side, bring the other along and re-record with: +# pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-08-29-windows-atomic-replace-retry.md +2026-08-29-windows-atomic-replace-retry.md: 4db5de6403be7ec39a1568a11d8877cba1ed5838 +2026-08-29-windows-atomic-replace-retry.zh.md: 0138727ac0fe12af51b5a383b60859977300353c diff --git a/.agents/notes/implemented/bug-fix/2026-08-29-windows-atomic-replace-retry.md b/.agents/notes/implemented/bug-fix/2026-08-29-windows-atomic-replace-retry.md new file mode 100644 index 0000000000..4db5de6403 --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-08-29-windows-atomic-replace-retry.md @@ -0,0 +1,27 @@ +# Agent Note: Retry transient Windows atomic replacements + +Status: implemented + +English | [中文](2026-08-29-windows-atomic-replace-retry.zh.md) + +## Problem + +Windows can temporarily reject a rename that replaces an existing file with `EACCES`, `EBUSY`, or `EPERM` while another system component holds the target. The cross-process writer lock orders cooperating application writers but cannot release that external handle, so treating the first error as permanent makes an otherwise valid settings or credentials update fail nondeterministically. + +## Decision + +`writeFileAtomic` owns replacement retry because every file-backed store needs the same guarantee. On Windows only, it retries `EACCES`, `EBUSY`, and `EPERM` up to eight times with exponential delays from 20 to 200 milliseconds. The same fully written temporary sibling remains the rename source throughout, and a caller-held writer lock remains held until `writeFileAtomic` settles. + +Other error codes and other operating systems fail immediately. Exhausting the retry budget rethrows the final filesystem error after removing the temporary sibling; the existing target remains unchanged because no attempt deletes or truncates it. + +## Alternatives considered + +**Retry the credentials mutation.** A consumer-level retry would leave settings and future stores exposed, and replaying a read-modify-write operation can repeat work outside the atomic replacement. The shared primitive is the narrow owner of replacement-only retry. + +**Delete the target before rename.** Removing the target can make readers observe an absent file and forfeits atomic replacement, so it cannot be a recovery step. + +**Retry indefinitely.** A permanent permission error would then hang the writer and any lock contender. A bounded delay absorbs transient file use while preserving a predictable failure outcome. + +## Consequences + +A transient Windows handle can delay one replacement by at most 1.1 seconds before the final attempt fails. During that interval readers continue to see the complete old target, and success still consists of one atomic rename. Regression tests inject every retried code, permanent and non-Windows failures, and retry exhaustion; they observe rename attempts and advance fake timers rather than depending on wall-clock sleeps. diff --git a/.agents/notes/implemented/bug-fix/2026-08-29-windows-atomic-replace-retry.zh.md b/.agents/notes/implemented/bug-fix/2026-08-29-windows-atomic-replace-retry.zh.md new file mode 100644 index 0000000000..0138727ac0 --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-08-29-windows-atomic-replace-retry.zh.md @@ -0,0 +1,27 @@ +# Agent Note: 重试 Windows 上的瞬时原子替换失败 + +Status: implemented + +[English](2026-08-29-windows-atomic-replace-retry.md) | 中文 + +## 问题 + +当另一个系统组件持有目标文件时，Windows 可能以 `EACCES`、`EBUSY` 或 `EPERM` 暂时拒绝替换已有文件的 rename。跨进程写锁能够排序应用内互相协作的写入方，却无法释放该外部句柄，因此把第一次错误当作永久失败会让本来有效的设置或凭据更新随机失败。 + +## 决策 + +`writeFileAtomic` 负责替换重试，因为每个文件型存储都需要相同保证。它仅在 Windows 上重试 `EACCES`、`EBUSY` 与 `EPERM`，最多八次，延迟从 20 毫秒指数增长至 200 毫秒。整个过程中，同一份已经完整写入的临时兄弟文件始终作为 rename 来源；调用方持有的写锁也会保持到 `writeFileAtomic` 结束。 + +其他错误码和其他操作系统会立即失败。重试预算耗尽后，函数移除临时兄弟文件并重新抛出最后一个文件系统错误；由于任何尝试都不会删除或截断现有目标，目标内容保持不变。 + +## 考虑过的替代方案 + +**重试凭据变更。** 消费方级重试仍会让设置和未来存储暴露于同一问题，而且重放一次读-修改-写操作可能重复原子替换之外的工作。共享原语是只负责替换重试的最窄所有者。 + +**在 rename 前删除目标。** 删除目标会让读取方观察到文件缺失，并放弃原子替换，因此不能作为恢复步骤。 + +**无限重试。** 永久权限错误会由此挂住写入方与所有锁竞争者。有界延迟可以吸收瞬时文件占用，同时保留可预测的失败结果。 + +## 后果 + +一个瞬时 Windows 句柄最多会让单次替换多等待 1.1 秒，随后最终尝试失败。在此期间，读取方继续看到完整的旧目标；成功仍由一次原子 rename 完成。回归测试注入每种可重试错误、永久错误、非 Windows 错误与重试耗尽，并观察 rename 尝试和推进伪时钟，而不依赖真实时间 sleep。 diff --git a/packages/util/atomic-write/README.i18n.yaml b/packages/util/atomic-write/README.i18n.yaml index 84dba6fd7b..ffb53e092a 100644 --- a/packages/util/atomic-write/README.i18n.yaml +++ b/packages/util/atomic-write/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write packages/util/atomic-write/README.md -README.md: 22806b539faaa7668c37d863c20ffced2576bde6 -README.zh.md: 1468f06aa4d46d9ea7c471bbb045314b67ae595e +README.md: 69daf671ba9d1269643533a6bb6e64462b8bee05 +README.zh.md: 8a8613c673c4d12634c686cda7f2ced9957492b2 diff --git a/packages/util/atomic-write/README.md b/packages/util/atomic-write/README.md index 22806b539f..69daf671ba 100644 --- a/packages/util/atomic-write/README.md +++ b/packages/util/atomic-write/README.md @@ -36,7 +36,7 @@ declare const text: string await writeFileAtomic('/home/u/.dsh/settings.yaml', text, { mode: 0o600 }) ``` -Parent directories are created as needed, and readers observe either the old or the new complete content. On any failure the temporary file is removed and the failure is rethrown, so a failed replacement leaves the target untouched. +Parent directories are created as needed, and readers observe either the old or the new complete content. On Windows, transient replacement interference reported as `EACCES`, `EBUSY`, or `EPERM` is retried for a bounded interval; any remaining failure removes the temporary file and leaves the target untouched. ### Coordinating writers @@ -79,7 +79,7 @@ The package is built on one separation: the atomic commit owns the swap, and the ### Write path -`writeFileAtomic` writes a random-suffix sibling opened with exclusive create (`wx`), then renames it over the target. The exclusive open refuses to follow a symlink planted at a guessable temp path; the same-directory sibling keeps the rename on one filesystem; and the rename replaces a symlinked target itself instead of writing through to its referent. +`writeFileAtomic` writes a random-suffix sibling opened with exclusive create (`wx`), then renames it over the target. The exclusive open refuses to follow a symlink planted at a guessable temp path; the same-directory sibling keeps the rename on one filesystem; and the rename replaces a symlinked target itself instead of writing through to its referent. A Windows retry keeps the same complete sibling and uses bounded exponential backoff, so temporary use of the target by software outside the cooperative writer lock cannot turn a safe replacement into an immediate failure; the [retry decision](../../../.agents/notes/implemented/bug-fix/2026-08-29-windows-atomic-replace-retry.md) owns the rationale and rejected alternatives. `withFileLock` creates a `.lock` sibling with `wx`. `EEXIST` identifies contention directly; `EPERM` does so only when a fresh `lstat` confirms the lock path exists, covering Windows exclusive-create behavior without hiding an unrelated permission failure. The lock records its creator's PID and is removed by the holder in a `finally`; contention backs off exponentially and fails when the per-call `waitMs` deadline (default two seconds) passes. diff --git a/packages/util/atomic-write/README.zh.md b/packages/util/atomic-write/README.zh.md index 1468f06aa4..8a8613c673 100644 --- a/packages/util/atomic-write/README.zh.md +++ b/packages/util/atomic-write/README.zh.md @@ -36,7 +36,7 @@ declare const text: string await writeFileAtomic('/home/u/.dsh/settings.yaml', text, { mode: 0o600 }) ``` -父目录会按需创建，读取方只会观察到旧内容或完整的新内容。任何失败都会移除临时文件并重新抛出该失败，因此一次失败的替换不会改动目标文件。 +父目录会按需创建，读取方只会观察到旧内容或完整的新内容。在 Windows 上，报告为 `EACCES`、`EBUSY` 或 `EPERM` 的瞬时替换干扰会在有界时间内重试；任何剩余失败都会移除临时文件，并保持目标文件不变。 ### 协调写入方 @@ -79,7 +79,7 @@ await withFileLock('/home/u/.dsh/settings.yaml', async () => { ### 写入路径 -`writeFileAtomic` 先以独占创建（`wx`）打开一个随机后缀的同级文件并写入内容，然后 rename 到目标上。独占打开拒绝跟随预先埋在可猜测临时路径上的符号链接；同目录兄弟文件保证 rename 落在同一文件系统上；rename 替换的是符号链接目标本身，绝不写穿到其指向的文件。 +`writeFileAtomic` 先以独占创建（`wx`）打开一个随机后缀的同级文件并写入内容，然后 rename 到目标上。独占打开拒绝跟随预先埋在可猜测临时路径上的符号链接；同目录兄弟文件保证 rename 落在同一文件系统上；rename 替换的是符号链接目标本身，绝不写穿到其指向的文件。Windows 重试会保留同一份完整的兄弟文件，并采用有界指数退避，因此协作式写锁之外的软件瞬时占用目标时，不会让安全替换立即失败；[重试决策](../../../.agents/notes/implemented/bug-fix/2026-08-29-windows-atomic-replace-retry.zh.md)记录了理由与被拒绝的替代方案。 `withFileLock` 以 `wx` 创建 `.lock` 同级文件。`EEXIST` 直接表示竞争；只有一次新的 `lstat` 确认锁路径存在时，`EPERM` 才表示竞争，从而兼容 Windows 的独占创建行为，又不掩盖无关的权限故障。锁记录创建者的 PID，由持有者在 `finally` 中移除；竞争按指数退避，在每次调用声明的 `waitMs` 期限（默认两秒）过后失败。 diff --git a/packages/util/atomic-write/src/index.ts b/packages/util/atomic-write/src/index.ts index 3e5764a329..467c3c29b5 100644 --- a/packages/util/atomic-write/src/index.ts +++ b/packages/util/atomic-write/src/index.ts @@ -14,6 +14,33 @@ import { randomBytes } from 'node:crypto' import { lstat, mkdir, rename, rm, writeFile } from 'node:fs/promises' import { dirname } from 'node:path' +const WINDOWS_TRANSIENT_RENAME_ERRORS: ReadonlySet = new Set(['EACCES', 'EBUSY', 'EPERM']) +const WINDOWS_RENAME_RETRY_INITIAL_MS = 20 +const WINDOWS_RENAME_RETRY_MAX_MS = 200 +const WINDOWS_RENAME_RETRY_LIMIT = 8 + +/** Whether Windows reported temporary interference with an atomic replacement. */ +function isTransientWindowsRenameError(error: unknown): boolean { + if (process.platform !== 'win32') return false + return WINDOWS_TRANSIENT_RENAME_ERRORS.has((error as NodeJS.ErrnoException | null)?.code ?? '') +} + +/** Replace the target after bounded retries for transient Windows interference. */ +async function renameAtomicTemp(temp: string, filename: string): Promise { + let delay = WINDOWS_RENAME_RETRY_INITIAL_MS + for (let retries = 0;; retries += 1) { + try { + await rename(temp, filename) + return + } catch (error) { + if (!isTransientWindowsRenameError(error)) throw error + if (retries >= WINDOWS_RENAME_RETRY_LIMIT) throw error + } + await new Promise(resolve => setTimeout(resolve, delay)) + delay = Math.min(delay * 2, WINDOWS_RENAME_RETRY_MAX_MS) + } +} + /** * Filesystem options for {@link writeFileAtomic}; `mode` is required so the * permission decision stays visible at every call site. @@ -40,8 +67,10 @@ export interface WriteFileAtomicOptions { * rename, so replacing a wider-permission file narrows it without a chmod * race. The rename also replaces a symlinked target itself instead of writing * through to its referent, and the same-directory sibling keeps the rename on - * one filesystem. On any failure the temp file is removed and the failure - * rethrown. Crash durability (fsync) is out of scope. + * one filesystem. Windows replacement retries transient `EACCES`, `EBUSY`, + * and `EPERM` failures for a bounded interval while the complete temp file + * remains the rename source. On any remaining failure the temp file is + * removed and the failure rethrown. Crash durability (fsync) is out of scope. * @param filename - final path receiving the content. * @param content - complete next file content. * @param options - permission bits for the replacement inode. @@ -56,7 +85,7 @@ export async function writeFileAtomic(filename: string, content: string, options const temp = `${filename}.${randomBytes(6).toString('hex')}.tmp` try { await writeFile(temp, content, { mode: options.mode, flag: 'wx' }) - await rename(temp, filename) + await renameAtomicTemp(temp, filename) } catch (error) { await rm(temp, { force: true }) throw error diff --git a/packages/util/atomic-write/tests/atomic-write.spec.ts b/packages/util/atomic-write/tests/atomic-write.spec.ts index 683abe51bc..ff3a2a4e6a 100644 --- a/packages/util/atomic-write/tests/atomic-write.spec.ts +++ b/packages/util/atomic-write/tests/atomic-write.spec.ts @@ -1,15 +1,28 @@ -import { lstat, mkdir, mkdtemp, readFile, readdir, rm, stat, symlink, writeFile } from 'node:fs/promises' +import { lstat, mkdtemp, readFile, readdir, rm, stat, symlink, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { dirname, join } from 'node:path' import { afterEach, describe, expect, it, vi } from 'vitest' import { withFileLock, writeFileAtomic } from '../src/index.ts' -const state = vi.hoisted(() => ({ failLockCreateWithEPERM: false })) +const state = vi.hoisted(() => ({ + failLockCreateWithEPERM: false, + renameAttempts: 0, + renameFailures: [] as string[], +})) vi.mock('node:fs/promises', async (importOriginal) => { const actual = await importOriginal() return { ...actual, + rename: (async (...args: Parameters) => { + state.renameAttempts += 1 + const code = state.renameFailures.shift() + if (code !== undefined) { + if (code === 'NO_CODE') throw new Error('injected rename failure without a code') + throw Object.assign(new Error(`${code}: injected rename failure`), { code }) + } + return actual.rename(...args) + }), writeFile: (async (path: unknown, ...rest: never[]) => { if (state.failLockCreateWithEPERM && String(path).endsWith('.lock')) { state.failLockCreateWithEPERM = false @@ -20,12 +33,26 @@ vi.mock('node:fs/promises', async (importOriginal) => { } }) -afterEach(() => { +const scratchDirs: string[] = [] + +afterEach(async () => { + vi.useRealTimers() + vi.restoreAllMocks() state.failLockCreateWithEPERM = false + state.renameAttempts = 0 + state.renameFailures.length = 0 + await Promise.all(scratchDirs.splice(0).map(dir => rm(dir, { + force: true, + maxRetries: 10, + recursive: true, + retryDelay: 20, + }))) }) async function scratch(): Promise { - return mkdtemp(join(tmpdir(), 'dsh-atomic-write-')) + const dir = await mkdtemp(join(tmpdir(), 'dsh-atomic-write-')) + scratchDirs.push(dir) + return dir } /** Resolve once the lockfile exists, so contention is measured against a held lock. */ @@ -44,9 +71,12 @@ describe('writeFileAtomic', () => { it('creates the file and its parents with exactly the stated mode', async () => { const dir = await scratch() const target = join(dir, 'nested', 'deep', 'doc.yaml') - await writeFileAtomic(target, 'a: 1\n', { mode: 0o600 }) + await writeFileAtomic(target, 'a: 1\n', { dirMode: 0o700, mode: 0o600 }) expect(await readFile(target, 'utf8')).toBe('a: 1\n') - if (process.platform !== 'win32') expect((await stat(target)).mode & 0o777).toBe(0o600) + if (process.platform !== 'win32') { + expect((await stat(dirname(target))).mode & 0o777).toBe(0o700) + expect((await stat(target)).mode & 0o777).toBe(0o600) + } }) it('replaces existing content and narrows a wider-permission file to the stated mode', async () => { @@ -70,13 +100,62 @@ describe('writeFileAtomic', () => { expect(await readFile(victim, 'utf8')).toBe('victim-content') }) - it('leaves no temp sibling and rethrows when the rename fails', async () => { + it('retries transient Windows rename interference and commits the replacement', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + vi.useFakeTimers() const dir = await scratch() - const target = join(dir, 'occupied') - await mkdir(target) - await expect(writeFileAtomic(target, 'content', { mode: 0o600 })).rejects.toThrow() + const target = join(dir, 'document') + await writeFile(target, 'old') + state.renameFailures.push('EACCES', 'EBUSY', 'EPERM') + + const replacement = writeFileAtomic(target, 'new', { mode: 0o600 }) + await vi.waitFor(() => { expect(state.renameAttempts).toBeGreaterThan(0) }) + await vi.runAllTimersAsync() + await replacement + + expect(state.renameAttempts).toBe(4) + expect(await readFile(target, 'utf8')).toBe('new') expect((await readdir(dir)).filter(entry => entry.includes('.tmp'))).toEqual([]) }) + + it('leaves no temp sibling after bounded Windows rename retries expire', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + vi.useFakeTimers() + const dir = await scratch() + const target = join(dir, 'document') + await writeFile(target, 'old') + state.renameFailures.push(...Array.from({ length: 9 }, () => 'EPERM')) + + const replacement = writeFileAtomic(target, 'new', { mode: 0o600 }) + await vi.waitFor(() => { expect(state.renameAttempts).toBeGreaterThan(0) }) + await vi.runAllTimersAsync() + await expect(replacement).rejects.toMatchObject({ code: 'EPERM' }) + + expect(state.renameAttempts).toBe(9) + expect(await readFile(target, 'utf8')).toBe('old') + expect((await readdir(dir)).filter(entry => entry.includes('.tmp'))).toEqual([]) + }) + + it('does not retry a Windows rename failure without a transient code', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + const dir = await scratch() + const target = join(dir, 'document') + state.renameFailures.push('NO_CODE') + + await expect(writeFileAtomic(target, 'new', { mode: 0o600 })).rejects.toThrow(/without a code/) + expect(state.renameAttempts).toBe(1) + expect((await readdir(dir)).filter(entry => entry.includes('.tmp'))).toEqual([]) + }) + + it('does not retry rename permission failures outside Windows', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('linux') + const dir = await scratch() + const target = join(dir, 'document') + state.renameFailures.push('EPERM') + + await expect(writeFileAtomic(target, 'new', { mode: 0o600 })).rejects.toMatchObject({ code: 'EPERM' }) + expect(state.renameAttempts).toBe(1) + }) }) describe('withFileLock', () => {