Improves thread safety of lazy initializations#641
Conversation
The Interlocked.Exchange calls will ensure that the same object is returned from each call even under a race. That's not necessarily required, but it should also make sure that no write re-ordering issues occur on platforms that allow it. And it also makes the possibility of thread safety issues explicit.
jbevain
left a comment
jbevain
left a comment
There was a problem hiding this comment.
Thank you for your contribution. I've only seen a bunch of style issues that need to be addressed but that's otherwise good to go.
|
Also, I'm not quite sure what is up with the failing test on Windows. That might be a by-product of the change, even though it looks unrelated to the PR functionality. |
scott-ferguson-unity
left a comment
scott-ferguson-unity
left a comment
There was a problem hiding this comment.
Updated the formatting issues. The tests do all pass when I run them locally.
Intialize it to null to match pattern for the other references. Fixes the case were the lazy initialization would break when public_key_token was Empty<byte>.Array.
|
The failing tests on Windows were my fault - I didn't think that breaks anything and keeps the pattern the same as the other cases. |
|
@scott-ferguson-unity thanks for getting this green. This is good to merge on my end. Do you want to add anything before I merge? |
|
No, I don't believe anything else needs to be added. |
|
Thanks for the PR! |
|
@scott-ferguson-unity , there is actually a dedicated |
|
@teo-tsirpanis using |
|
That one is much better, but I think it would need to be profiled (it uses either reflection or a lambda, depending on the overload you choose). |
4 participants
The Interlocked.Exchange calls will ensure that the same object is
returned from each call even under a race. That's not necessarily
required, but it should also make sure that no write re-ordering issues
occur on platforms that allow it. And it also makes the possibility
of thread safety issues explicit.