On Fri, 8 Jan 2021 13:32:08 GMT, Peter Levart <plev...@openjdk.org> wrote:

>>> IIUC, your changes mostly all flow from the decision to declare the fields 
>>> as non-volatile; if they were still declared as volatile then it'd be 
>>> impossible to observe null in them, I think (correct me if I'm wrong; it 
>>> seems like you thought through this quite thoroughly) as then I don't see 
>>> how could a volatile read happen before the initial volatile writes as the 
>>> writes are part of the ClassValues constructor invocation and the reference 
>>> to the ClassValues object is unavailable externally before the constructor 
>>> completes. In any case, your approach definitely avoids any of these 
>>> concerns so I'm inclined to go with it.
>> 
>> It depends entirely on the guarantees of ClassValue and not on whether the 
>> fields are volatile or not. If ClassValue publishes the BiClassValues object 
>> via data race then even if the fields in BiClassValues are volatile and 
>> initialized in constructor, the publishing write in ClassValue could get 
>> reordered before the volatile writes of the fields, so you could observe the 
>> fields uninitialized.
>> I can't find in the spec of ClassValue any guarantees of ordering, but I 
>> guess the implementation does guarantee safe publication. So if you want to 
>> rely on ClassValue guaranteeing safe publication, you can pre-initialized 
>> the fields in constructor and code can assume they are never null even if 
>> they are not volatile.
>
> I checked the code of ClassValue and it can be assumed that it publishes 
> associated values safely. The proof is that it keeps values that it publishes 
> assigned to the final field `java.lang.ClassValue.Entry#value`.

So, are you saying the solution where I kept the fields volatile and 
initialized them with `Map.of()` is safe? If so, that'd be good news; I'm 
inclined these days to write as much null-free code as possible :-)

-------------

PR: https://git.openjdk.java.net/jdk/pull/1918

Reply via email to