Make ConverterManager.getInstance()
init thread-safe.
#776
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.
We encountered a TSAN error because 2 threads were racing to call
ConverterManager.getInstance()
. My read is that the current code couldactually be unsafe, since one thread might see
INSTANCE
as non-nullbefore the other thread has initialized its fields.
Fortunately, this looks like a good case for the
initialization-on-demand holder
idiom.
(There would be additional thread-safety concerns if anyone were to try
to mutate the
ConverterManager
instance concurrently with usage. ButI'm not sure there's a solution to that short of spraying
synchronized
around. That seems like probably overkill, given the possible
performance impact for what I'd hope is an uncommon use case.)