소스 검색

feat(atomic-write): state the writer-lock wait limit per call

How long a contender waits is a property of the operation the lock holder
runs, not of the write protocol. The 2s default was sized for the
render-and-rename cycle every call site had; a credential mutation that
refreshes an expired token performs a network round trip while holding the
lock, and leaving the default in place would fail every other writer of that
file for the duration.

`withFileLock` takes an optional `waitMs`; the retry cadence stays fixed
because it governs how often a contender asks, which no caller varies. Every
existing call site keeps the default.
Yichen Jiang 1 개월 전
부모
커밋
26a8e6a555

+ 2 - 2
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: 4d0b55291955c9d37f4788c7d37ad8e6ce728f70
-README.zh.md: c2d7f0b49fa123befbb663ac43862a40b4ef19b4
+README.md: 0e6501c0ae35bf26d7df67f28ce0ae24c85ecca6
+README.zh.md: 377f85f5c4aab74e8c04068b616de32a8ba063d6

+ 2 - 0
packages/util/atomic-write/README.md

@@ -30,6 +30,8 @@ await withFileLock('/home/u/.dsh/settings.yaml', async () => {
 
 `withFileLock` serializes the writers of one file across processes, for the read-render-commit cycles a bare atomic commit cannot make safe on its own. The lock is a `wx`-created `<filename>.lock` sibling, so readers never contend; waiters back off exponentially and fail with a timeout rather than block forever. `EEXIST` identifies contention directly; `EPERM` does so only when a fresh `lstat` confirms that the lock path exists, covering Windows exclusive-create behavior without hiding an unrelated permission failure. A contender never removes the existing lock: age cannot distinguish a crashed owner from a paused live writer.
 
+How long a contender waits is a property of the operation the holder runs, so it is stated per call through `waitMs`. The default is sized for file work alone; a holder whose cycle includes a network round trip — a credential mutation that refreshes an expired token — states a longer one, because leaving the default would fail every other writer of that file for the duration. The retry cadence stays fixed: it governs how often a contender asks, which no caller has a reason to vary.
+
 ## Model Experience
 
 None, as this is a pure filesystem primitive; nothing here reaches a model request.

+ 2 - 0
packages/util/atomic-write/README.zh.md

@@ -30,6 +30,8 @@ await withFileLock('/home/u/.dsh/settings.yaml', async () => {
 
 `withFileLock` 跨进程串行化同一文件的写入方,服务于单靠原子提交无法保证安全的读-渲染-提交循环。锁是以 `wx` 创建的同目录 `<filename>.lock`,因此读取方从不参与竞争;等待方按指数退避,超时即失败而非无限阻塞。`EEXIST` 直接表示竞争;只有一次新的 `lstat` 确认锁路径存在时,`EPERM` 才表示竞争,从而兼容 Windows 的独占创建行为,又不掩盖无关的权限故障。竞争者绝不移除现有锁:锁龄无法区分已经崩溃的所有者与被暂停但仍存活的写入方。
 
+等待多久是持锁方所跑操作的属性,因此由每次调用经 `waitMs` 声明。默认值只按纯文件工作量级选定;若持锁方的循环包含一次网络往返——例如刷新过期 token 的凭据变更——就应声明更长的值,否则该文件的其他写入方会在这段时间内全部失败。退避节奏保持固定:它决定竞争者多久问一次,调用方没有理由改变它。
+
 ## 模型体验
 
 无:本包是纯文件系统原语,此处没有任何内容会到达模型请求。

+ 30 - 6
packages/util/atomic-write/src/index.ts

@@ -78,14 +78,36 @@ async function isLockContention(error: unknown, lockPath: string): Promise<boole
 }
 
 /**
- * Writer-lock protocol constants. These are robustness invariants of the
- * cross-process write protocol, not deployment tunables: contention normally
- * resolves within the retry deadline, while expiry fails the contender without
- * guessing whether the existing lock still has an owner.
+ * Retry cadence for a contended lock. These stay robustness invariants of the
+ * cross-process write protocol rather than deployment tunables: they govern how
+ * often a contender asks, which no caller has a reason to vary.
  */
 const LOCK_RETRY_INITIAL_MS = 20
 const LOCK_RETRY_MAX_MS = 200
-const LOCK_TIMEOUT_MS = 2_000
+
+/**
+ * How long a contender waits when the caller states no limit — sized for the
+ * render-and-rename cycle every call site had when this package was written.
+ * Expiry fails the contender rather than guessing whether the existing lock
+ * still has an owner. How long is *worth* waiting is a property of the
+ * operation the lock holder runs, which is why {@link FileLockOptions.waitMs}
+ * exists; the value here is the floor for an operation that does file work
+ * alone.
+ */
+const DEFAULT_LOCK_WAIT_MS = 2_000
+
+/** Options for one {@link withFileLock} acquisition. */
+export interface FileLockOptions {
+  /**
+   * Maximum time to wait for the lock, in milliseconds. State one when the
+   * holder's operation legitimately runs longer than file work — a credential
+   * mutation that refreshes a token performs a network round trip while
+   * holding the lock, and leaving the default in place would fail every other
+   * writer of the same file for the duration. Waiting is productive: a
+   * contender that acquires the lock afterwards re-reads the committed state.
+   */
+  waitMs?: number
+}
 
 /**
  * Hold the cross-process writer lock for `filename` around one operation. The
@@ -100,14 +122,16 @@ const LOCK_TIMEOUT_MS = 2_000
  * action. The parent directory must exist.
  * @param filename - the file whose writers this lock serializes.
  * @param operation - the read-render-commit cycle to run while holding the lock.
+ * @param options - acquisition options; omitted waits {@link DEFAULT_LOCK_WAIT_MS}.
  * @returns the operation's result; the lock releases on both outcomes.
  */
 export async function withFileLock<T>(
   filename: string,
   operation: () => Promise<T>,
+  options?: FileLockOptions,
 ): Promise<T> {
   const lockPath = `${filename}.lock`
-  const deadline = Date.now() + LOCK_TIMEOUT_MS
+  const deadline = Date.now() + (options?.waitMs ?? DEFAULT_LOCK_WAIT_MS)
   let delay = LOCK_RETRY_INITIAL_MS
   for (;;) {
     try {

+ 41 - 0
packages/util/atomic-write/tests/atomic-write.spec.ts

@@ -28,6 +28,18 @@ async function scratch(): Promise<string> {
   return mkdtemp(join(tmpdir(), 'dsh-atomic-write-'))
 }
 
+/** Resolve once the lockfile exists, so contention is measured against a held lock. */
+async function waitForLock(lockPath: string): Promise<void> {
+  for (;;) {
+    try {
+      await stat(lockPath)
+      return
+    } catch {
+      await new Promise(resolve => setTimeout(resolve, 5))
+    }
+  }
+}
+
 describe('writeFileAtomic', () => {
   it('creates the file and its parents with exactly the stated mode', async () => {
     const dir = await scratch()
@@ -105,4 +117,33 @@ describe('withFileLock', () => {
     })).rejects.toThrow(/ENOENT|ENOTDIR|not a directory/i)
     expect(called).toBe(false)
   })
+
+  it('waits for the caller-stated limit rather than the protocol default', async () => {
+    // An operation whose work includes a network round trip legitimately holds
+    // the lock far longer than the render-and-rename the default was sized
+    // for. The limit is per call so one such operation cannot fail every other
+    // writer of the same file, and a caller that states a short one still
+    // fails fast.
+    const dir = await scratch()
+    const target = join(dir, 'document')
+    let release = (): void => {}
+    const held = new Promise<void>((resolve) => { release = resolve })
+    const holder = withFileLock(target, () => held)
+    // The holder owns the lock once its lockfile exists; contending before
+    // that would measure nothing.
+    await waitForLock(`${target}.lock`)
+
+    // Elapsed time is the assertion that distinguishes a honoured limit from
+    // the ignored argument: without it the contender simply waits out the
+    // protocol default and fails with the same message.
+    const startedAt = Date.now()
+    await expect(withFileLock(target, async () => 'impatient', { waitMs: 50 }))
+      .rejects.toThrow(/timed out waiting for the writer lock/)
+    expect(Date.now() - startedAt).toBeLessThan(1_000)
+
+    const patient = withFileLock(target, async () => 'patient', { waitMs: 10_000 })
+    release()
+    await holder
+    expect(await patient).toBe('patient')
+  })
 })