Repository navigation
fix(locking): take the driver lock with an exclusive create - #802
Merged
Merged
Conversation
LockFile.aquirelock checked islocked and then wrote the pid. Two drivers started together both passed the check and both wrote the file. Create the file with O_CREAT|O_EXCL|O_WRONLY, write the pid, and fsync. On EEXIST, a dead pid or a truncated file is removed and the create is tried once more. Where fcntl is available, hold an flock on the descriptor so a reused pid is not treated as the owner. removelock unlinks the file only when it still contains the pid this object wrote.
The exclusive lock lives on acquirelock. aquirelock is a deprecated alias. The akmc, basin hopping, parallel replica, and escape rate drivers, and the library body test, call the spelled method.
register_results removed the incoming job directory and created it again without exist_ok. Two drivers that both passed the lock both created it, and the second raised FileExistsError. add_state had the same race on the new state directory. Create both with exist_ok. The replica drivers and the process-search explorer share the incoming job directory pattern and get the same call.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Develop took the fork exit guard and the results directory create after this branch. Keep the exclusive lock, disarm the exit hook in a forked child, and create the results directory before the log and the lock.
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.
Two server drivers started in one directory could both pass the lock check and write their pid. They then raced over the shared job directories, and one run aborted with
FileExistsError.The lock is now created with an exclusive open. The pid is written and flushed, and a dead, empty or truncated lock is removed and retried once. Where
fcntlis available the owner holds an flock, so a recycled pid cannot look live.removelockdeletes the file only when it still holds the pid this process wrote.acquirelockis the name to call, andaquirelockremains as a deprecated alias. The akmc, basin hopping, parallel replica and escape rate drivers now callacquirelock.Basin hopping also raised
FileExistsErrorwhen the state directory or the incoming job directory already existed. Those creates, and the same incoming job directory in parallel replica, escape rate and the explorer, now accept an existing directory.Tests in
tests/test_locking.pystart two processes and two basin hopping drivers together and check that exactly one takes the lock. They also cover stale and truncated lock files, and existing state and job directories. Each test fails on develop d243519 and passes here. The full pytest run gives 670 passed and 43 skipped.