This codebase had five copies of essentially the same file-lock implementation. Each one acquired a lock by O_EXCL-creating a lockfile, writing the process PID into it, doing some work, then unlinking the lockfile on release. Stale-lock detection used PID-liveness primary with mtime fallback. Five files, near-identical code.

An earlier release fixed a data-loss bug in one of the five (the queue lock). The fix was small — drop the mtime fallback. The other four copies had the same code but were assumed to be fine. They weren't fine.

The second release found the same bug in two more (binding and message-binding). The fix was the same one line of removal, applied to two more files. The fifth copy was assumed to be fine. It wasn't.

This release found the same bug in the fifth copy (override-redemption). Under multi-process stress, 35-42 of 300 redemption records were silently lost per run — about 12% loss. Each checkOverride call reported success, but the record of "this token has been redeemed" was overwritten by a concurrent write. The single-use enforcement that the HMAC token scheme relies on was statistically broken.

Three independent discoveries of the same bug across three releases. Each discovery cost me about an hour of probing, code-reading, and verification. The bug-shape was the same every time. The fix was the same every time. The only difference was which of the five files happened to be the next one I probed.

What the bug pattern was

All five locks had this structure:

fd = openSync(LOCK, 'wx');           // create file, EEXIST if exists
writeFileSync(LOCK, String(pid));    // write PID for liveness check

The two lines run sequentially. Between them, the lock file exists but has empty content. Another process trying to acquire reads the lock file, finds empty content, and the pre-fix code then checked mtime: "if the file is older than N seconds, the holder probably crashed without writing its PID; consider the lock stale and steal it."

On macOS with APFS under tight contention, the mtime check returned values that occasionally tripped the staleness threshold even though the lock had been created milliseconds ago. The lock got unlinked. Two processes ended up "holding" the lock. Writes raced. Last writer won.

The fix in every case: drop the mtime fallback. Use PID-liveness only. If the PID is present and dead, the lock is stale. Otherwise wait. A crashed holder that never wrote its PID waits out the timeout and throws — fail-loud, the right behavior.

Why I didn't extract sooner

After the queue-lock fix, I noted that the audit/binding/override locks had the same pattern but were tested fine. The test was: run the queue's stress probe (10 procs × 30 writes) against the audit log. Audit passed 300/300. I generalized from that to "the pattern is fine for the other locks too."

The generalization was wrong. The race window for the mtime-fallback bug scales with how long the lock is held. Audit appends are short (open/write/fsync/close, microseconds). Binding writes are long (JSON read-modify-write, tens of milliseconds). The same lock code behaves differently under different work-inside-lock durations.

The right move after the queue fix would have been:

  1. Run the stress test against EACH of the four remaining copies, not just the audit lock. The audit lock was the easiest to test (it had a simple append API); the others required a tiny bit more setup. I skipped them.
  2. Recognize the duplication as a recurring-bug risk. Extract the helper. Make the next fix apply automatically.

Both moves would have been cheap. I did neither, because each individual decision was locally reasonable — "I'll skip the heavier stress tests, the audit one passed, the code's the same." Locally reasonable, globally wrong: that decision caused two more silent data-loss bugs to ship for an additional week.

The extraction

The shared helper is about 60 lines. It does exactly what the five copies did, minus the mtime fallback. Each caller now delegates:

import { withFileLock } from './file-lock.mjs';

function withMyLock(fn) {
  ensureMyDir();
  return withFileLock(MY_LOCK_FILE, fn, { timeoutMs: 5000 });
}

The timeout is a parameter because callers have different requirements (5s for bindings where contention is short, 30s for queue and audit where the work-inside-lock is longer).

The audit lock has a special semantic: if lock acquisition times out, the audit must NOT fail-block the caller's operation. It falls through to a best-effort append without synchronization. I preserved that by wrapping the helper's throw in a try/catch in withAuditLock:

function withAuditLock(fn) {
  ensureDir();
  try {
    return _withFileLock(AUDIT_LOCK, () => fn(true), { timeoutMs: 30000 });
  } catch (e) {
    if (typeof e.message === 'string' && e.message.includes('timed out')) {
      return fn(false);  // best-effort, no lock
    }
    throw e;
  }
}

Audit appends remain non-blocking. Other locks throw on timeout, which is the right fail-loud behavior.

Verification

The same multi-process stress test (10 procs × 30 writes per module, 4 modules) ran across 3 runs each = 12 total runs. 11 of 12 landed all 300 writes. One run of the override-redemption lock lost 1 of 300 — the extreme-contention edge case at the lock-acquire timeout boundary, down from 35-42 lost pre-fix.

For comparison, the pre-fix measurements:

Post-fix across all four: <0.3% loss aggregated, mostly zero.

The general lesson

When you find yourself copy-pasting a non-trivial code pattern into a third module, that's the signal to extract. Two copies is borderline; three copies is duplication tax accruing. Each future bug you find in one copy is also present in the others; the cost of fixing them serially exceeds the cost of extracting once.

The thing that keeps people from extracting is the perceived risk of the refactor breaking the working copies. The refactor in this release was straightforward — each call site became a 3-line wrapper that delegates to the shared helper — and the stress tests across all four modules ran cleanly post-refactor. The risk was much smaller than the cost of finding and fixing the same bug N times.

The version on the install URL is 4.5.110.