On 2018-06-18 05:45, David Holmes wrote:
Making it "final" to fix unsafe publication only works with actual publication after construction. You're making it "final" but then not setting it at construction time and instead using UNSAFE.putVolatile to get around the fact that it is final! The "final" serves no purpose in that regard. Further a putVolatile without corresponding getVolatile does not achieve safe publication under the JMM.
Adding a volatile read on every read through Properties is likely to have some performance impact, but it could still be better than before 8029891 where such operations were synchronized. Regarding Peter's comments about defaults, I think accesses to that field was implicitly guarded by the synchronized super.get calls before 8029891 and now needs to be re-examined too.
I think making the actual field(s) volatile is the only JMM compliant way to correctly address this.
The only issue is serialization-related readHashTable method (clone could be implemented without Unsafe as clone.map.putAll(map)). Is there no sanctioned way (using e.g. VarHandles) to safely construct and publish a transient "final" field in the scope of a readObject? I've been shying away from experimenting with VarHandles in places like Properties as I'm certain it could lead to bootstrap issues, but for something like a deserialization hook it might very well be fine.
/Claes
