Repository navigation
Close the lock file when lock() fails to get the lock - #58
Open
gaborcsardi wants to merge 2 commits into
Open
gaborcsardi wants to merge 2 commits into
gaborcsardi wants to merge 2 commits into
Conversation
filelock_lock() opens the lock file before it tries to lock it, but it only kept or closed the file descriptor when the lock succeeded. Every failed lock() call leaked one: a timeout = 0 call that finds the file locked, a timeout > 0 call that times out, a locking error, and on Unix an interrupt while waiting. A process that polls a busy lock accumulates descriptors until it hits the fd limit or select()'s FD_SETSIZE. On Unix, close the descriptor on the no-lock return paths and before the error() calls in filelock_lock(). The wait in filelock__interruptible() can longjmp out through error() or R_CheckUserInterrupt(), so it now runs under R_UnwindProtect(), which closes the descriptor and then continues the jump. The interrupt is still an interrupt. R_UnwindProtect() needs R 3.5.0, so Depends is now R (>= 3.5). On Windows, close the handle on the timeout return path and before each filelock__error() call after CreateFileW(). The interrupt paths already closed it. The WAIT_FAILED paths now read GetLastError() before the cleanup calls, so the reported error code is not clobbered. The new tests hold the lock from a child process and check that failed lock() calls do not leave the lock file open, for timeouts and for an interrupt. ps is added to Suggests for this. Fixes r-lib#28. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CwcjqqK59FmTJscyi7xUqv
The Windows wait loop closes the handle and raises "Locking interrupted for file ..." when an interrupt is pending, so the condition is a simpleError there. Check that on Windows and the interrupt class on Unix; the file-closed check applies to both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
eliotmcintire
added a commit
to PredictiveEcology/filelock
that referenced
this pull request
Oct 16, 2026
Merge fix/close-fd-on-failed-lock (r-lib#58) and take its files whole, dropping the fork's earlier fix a2fe838: r-lib#58 closes the lock file on the same failure paths and also on an error or interrupt while waiting. The fork's main now differs from r-lib#58 only in Version (1.0.3.9001), and will equal upstream once r-lib#58 is merged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011oFSqD7YBJZdkFfStooVtz
eliotmcintire
added a commit
to PredictiveEcology/filelock
that referenced
this pull request
Oct 16, 2026
main now carries r-lib#58 instead of a2fe838. The retry loop in filelock_lock() (start over when the locked file is no longer the one at `path`, or on ESTALE) now wraps r-lib#58's code, including its R_UnwindProtect() around the wait, so a failed or interrupted attempt still closes the file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011oFSqD7YBJZdkFfStooVtz
This branch has not been deployed
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
2 participants
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.
filelock_lock() opens the lock file before it tries to lock it, but it only kept or closed the file descriptor when the lock succeeded. Every failed lock() call leaked one: a timeout = 0 call that finds the file locked, a timeout > 0 call that times out, a locking error, and on Unix an interrupt while waiting. A process that polls a busy lock accumulates descriptors until it hits the fd limit or select()'s FD_SETSIZE.
On Unix, close the descriptor on the no-lock return paths and before the error() calls in filelock_lock(). The wait in
filelock__interruptible() can longjmp out through error() or R_CheckUserInterrupt(), so it now runs under R_UnwindProtect(), which closes the descriptor and then continues the jump. The interrupt is still an interrupt. R_UnwindProtect() needs R 3.5.0, so Depends is now R (>= 3.5).
On Windows, close the handle on the timeout return path and before each filelock__error() call after CreateFileW(). The interrupt paths already closed it. The WAIT_FAILED paths now read GetLastError() before the cleanup calls, so the reported error code is not clobbered.
The new tests hold the lock from a child process and check that failed lock() calls do not leave the lock file open, for timeouts and for an interrupt. ps is added to Suggests for this.
Fixes #28.
Claude-Session: https://claude.ai/code/session_01CwcjqqK59FmTJscyi7xUqv