Skip to content

Fix stale-lock takeover race in file store - #180

Open
levineam wants to merge 1 commit into
mainfrom
codex/propose-fix-for-stale-lock-recovery-issue
Open

Fix stale-lock takeover race in file store#180
levineam wants to merge 1 commit into
mainfrom
codex/propose-fix-for-stale-lock-recovery-issue

Conversation

@levineam

Copy link
Copy Markdown
Owner

Motivation

  • Stale-lock recovery previously renamed the canonical state.lock before validating owner liveness or age, which temporarily removed live locks and allowed other contenders to acquire the canonical path.
  • That behavior could let a subsequent lock be overwritten during restoration or allow a releaser to remove a lock it no longer owned, breaking the file-store mutex and risking corrupted/store state.

Description

  • Validate the observed lock file contents, owner liveness (lockOwnerIsRunning) and age (staleLockMs) before moving the canonical state.lock in recoverStaleLock so live owners are never displaced.
  • For inode-mismatch restoration, use fs.linkSync(takeoverPath, lockPath) instead of renaming to avoid overwriting a newly-created canonical lock when restoring from a takeover file.
  • Read and stash an ownership token when creating the lock and verify that token on release so a releaser only removes the lock if it still owns it (avoid blind rm by path).
  • Add a regression test stale-lock recovery does not move a live lock proving a live lock is not renamed even when its mtime appears stale, while preserving existing race/regression tests.

Testing

  • Ran node --test modules/jarvos-control-plane/test/storage.test.js and all control-plane storage tests passed (15/15).
  • Ran npm test --prefix modules/jarvos-control-plane and the suite passed mostly (93 passed, 1 unrelated ownership test failed due to the environment running as root which affects an ownership-specific assertion).
  • Verified local diffs and test-run output with git diff --check (no issues reported).

Codex Task

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant