Ensure writes to VaultConfig and SslConfig are guaranteed to be visible to other threads #228
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.
#46
Hi Team,
This is just a minor change to ensure the variables are visible to other threads. (as per Java Memory Model: https://docs.oracle.com/javase/specs/jls/se8/html/jls-8.html#jls-8.3.1.4)
While I understand that the vault client is mostly not designed to be multi-threaded, there is a use case where this might still be useful:
The fix makes all the variables volatile to ensure that all threads always see a consistent value after any particular write (there will still be race conditions if the variables are mutated by different threads, but that's not an issue for this use case).
I think this is an easy fix for a use case, where I think might be quite common for long running services. It is also hard to detect, as different architectures and different JVMs might behave differently.
While as much as possible I would adopt the one vault per thread approach, it seems unavoidable to do so for this use case, hence the fix.
Thanks for the time and for creating this Library!