fix: remove read lock from config get #5114
Merged
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.
What this PR does / why we need it:
This removes the lock when reading from a global or local config file. The read lock can lead to dead-lock in situations where we trigger concurrent reads, e.g. in projects having multiple modules with linked sources. @nilium has provided a reproducible example here: https://github.com/nilium/garden-locker, see discord thread for more details: https://discord.com/channels/817392104711651328/1151269956240031826
Is it safe to remove the read lock?
Concurrent reads are safe since they don't modify the state of the config.
Writes need the lock since they perform a read + write of the config, which in the case of concurrent writes could result in a trace with R1, R2, W2, W1. Process 1 overwrites the values from process 2 leading to a potentially incorrect state.
Write operations are atomic, we either read the state before or after the write. A concurrent read should not be able to read a partial state of the config.
Config data is always validated against the schema and throws an error in case of failure.
Note of caution
repositoryUrl
).Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer: