Open
Conversation
Some race conditions appear to still exist in the field. The existing mechanism tries to create a random-enough temporary filename to use for the copye of the object to be cached, before doing an atomic rename to the final filename in the cache. The temporary filename uses a uuid suffix which has a miniscule chance of not being unique across SCons invocations, but which is unchanging within a single incovation. The new proposal is to use Python's tempfile module to create a unqiue temporary directory for each cache write, copy the file there, then do the rename. One could further imagine wrapping critical code sections in filelock calls but that's not done in this change (FileLock is currently used for initial cachedir creation operations but not for individual cache writes or reads). Signed-off-by: Mats Wichmann <mats@linux.com>
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.
Some
CacheDirrace conditions appear to still exist in the field. The existing mechanism tries to create a random-enough temporary filename to use for the copy of the object to be cached, before doing an atomic rename to the final filename in the cache. The temporary filename uses a uuid suffix which has a miniscule chance of not being unique across SCons invocations, but which is unchanging within a single invocation. The new proposal is to use Python'stempfilemodule to create a unqiue temporary directory for each cache write, copy the file there, then do the rename. One could further imagine wrapping critical code sections in filelock calls but that's not done in this change (FileLockis currently to protect initialCacheDircreation operations as that had been a source of conflicts in a multi-threaded build, but not for individual cache writes or reads).There is no test: I spent a fair bit of time with AI assistance trying to create a reliable reproducer without success - through ever more contrived scenarios it still wouldn't fail. So while this may help, it's still a bit of "flying blind".
Contributor Checklist:
CHANGES.txtandRELEASE.txt(and read theREADME.rst).