fix: don't break a live lock on staleness alone (#432) - #445
Open
arzaanxeng wants to merge 2 commits into
Open
Conversation
Enhance file lock mechanism with heartbeat and mtime refresh to prevent premature lock expiration.
Contributor
🟠 Maintainer review suggested — low confidenceThe automated review could not reach a fully supported conclusion. Limitations
This review is advisory and does not block merging. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #432
Bug
breakIfAbandoned()decided a lock was abandoned ifexpired OR ownerGone.That meant a same-host owner that was still alive and running could have its
lock stolen just because its critical section took longer than
staleMs(big file, slow disk, loaded machine). Two processes would then race the same
file, and whichever
rename()landed last silently discarded the other's work.Fix
breakIfAbandoned()now checks owner liveness first: if the owner's pid ischeckable and confirmed alive on the same host, the lock is never broken on
staleness alone — liveness is decisive, not just one of two OR'd signals.
withFileLock()now runs a heartbeat while the critical section is active,refreshing the lock file's mtime every
staleMs / 3. This covers the casewhere liveness can't be checked (cross-host lock files), so
staleMsstillreflects real inactivity rather than just how long the section has run.
breaks regardless" behavior.
Testing
Added two regression tests in
file-lock.test.tsthat reproduce the exactscenario from the issue: a live same-host holder whose lock is older than
staleMs, and a second acquirer racing it mid-critical-section. Both newtests fail against the old code and pass with this fix. Full suite:
8/8 passing.