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