Skip to content

withFileLock is not atomic: concurrent processes can enter the critical section together and delete each other's locks #195

Description

@ivnmood

Summary

withFileLock (projects/core/src/utils/file-utils.ts, compiled utils/file-utils.js) is not a real mutex: lock acquisition and release both race. The symptoms were reported before in #142 (Could not acquire lock to write file under Jest), but the underlying races are still present in v3.3.0 through v4.0.1/master.

The races

1. Check-then-write acquisition (TOCTOU). waitForLockRelease() polls fs.existsSync(lockFilePath), then the caller creates the lock with a plain non-exclusive write:

waitForLockRelease(lockFilePath);   // waits until the file does not exist
fs.writeFileSync(lockFilePath, ''); // any process can do this concurrently

Two processes can both observe "no lock" between the check and the write, both "acquire", and both enter the critical section concurrently.

2. Unconditional removal in finally. The lock is deleted whether or not this process owns it:

finally {
  if (fs.existsSync(lockFilePath)) fs.rmSync(lockFilePath, { force: true });
}

Combined with (1): process A finishes and deletes the lock that process B created, letting process C enter while B is still inside fn().

3. No stale-lock recovery. If a process dies while holding the lock (e.g. a killed Jest worker), every other process throws after the fixed 2s lockFileWaitMs and the user has to run ts-patch clear-cache by hand.

Reproduction

Run N processes concurrently incrementing a counter file under withFileLock:

// worker.js — run 8 of these concurrently, 25 iterations each
const fs = require('fs');
const { withFileLock } = require('ts-patch/utils/file-utils');

for (let i = 0; i < 25; i++) {
  withFileLock(process.env.TARGET, () => {
    const value = Number(fs.readFileSync(process.env.TARGET, 'utf8'));
    const end = Date.now() + 2; while (Date.now() < end) {} // widen the race window
    fs.writeFileSync(process.env.TARGET, String(value + 1));
  });
}

Expected final counter: 200. Actual: lost updates and/or Could not acquire lock to write file throws.

Real-world trigger: ts-jest with compiler: 'ts-patch/compiler' and parallel Jest workers on a cold cache — all workers write the same cached files at startup. Many users work around it with maxWorkers: 1, which serializes the whole suite.

Suggested fix

  • Acquire atomically: fs.openSync(lockFilePath, 'wx') in a retry loop on EEXIST (O_CREAT|O_EXCL is atomic on local filesystems, and works on Windows).
  • Write { pid, token, createdAt } into the lock file; on release, delete only if the token still matches (owner-only release).
  • Stale-lock recovery instead of a 2s hard failure: if the lock is older than a threshold and its pid is dead — or older than a larger backstop age (protects against pid reuse) — steal it. Steal via fs.renameSync to a unique temp name first, so two concurrent stealers cannot delete a lock the winner just re-created.
  • Keep an overall acquisition deadline with the current descriptive error message.
  • Optional: replace the busy-wait sleep with Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms) so waiters don't burn a CPU core.

I have this working as a local patch (all of the above, ~100 lines in the compiled utils/file-utils.js); an 8-process increment stress test that fails on the current implementation passes with it, and concurrent-stealer interleaving stays strictly serialized. Happy to open a PR if you're open to the approach.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions