Fix race condition when trying to fetch the same project from the registry more than once #131
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.
Closes #128
This PR updates the
fetch_project_from_registry
function to use a mutex to ensure that the same project doesn't get cloned more than once at a time. This works by using aBTreeMap
(wrapped in aMutex
), where each key-value pair is a project hash and another mutexAs far as implementation, there are a few other places in the code where we have a similar problem of wanting to ensure that a piece of code only runs once for a particular key: baking recipes, and creating outputs. For baking, we use a map that contains channels, which allows parallel calls to get the result immediately without doing more work. For creating outputs, we use a lazier option of using a single mutex to guard the whole section-- it's not (currently) fine-grained. This new implementation is kind of like a mix between these two other implementations, where we do have fine-grained locking, but each invocation ends up doing a bit of work to read the filesystem to figure out where the output goes
One downside with this implementation is that the mutex map uses
Arc
s, and it never gets keys removed. We could switch to usingWeak
values then add a "remove" phase, but I thought it'd be unlikely that enough keys could ever be added to the map to become an issue