RFR 8199435 : Unsafe publication of java.util.Properties.map

[David Holmes]

Hi Brent,

On 16/06/2018 1:44 AM, Brent Christian wrote:
> Hi,
> 
> In JDK 9, 8029891[1] refactored java.util.Properties to store its values 
> in an internal ConcurrentHashMap, and removed synchronization from 
> "reader" methods in order to avoid potential hangs/deadlocks during 
> classloading.
> 
> Claes has noticed that there is the possibility of the new 'map' field 
> being observed with its default value (null), before being set.
> 
> After looking at the JSR 133 FAQ[2], I agree with Claes that we should 
> make 'map' a field final.

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.

I think making the actual field(s) volatile is the only JMM compliant 
way to correctly address this.

David
-----

> Please review my change to do this:
> 
> Webrev:
> http://cr.openjdk.java.net/~bchristi/8199435/webrev/
> Issue:
> https://bugs.openjdk.java.net/browse/JDK-8199435
> 
> Thanks,
> -Brent
> 
> 1. https://bugs.openjdk.java.net/browse/JDK-8029891
> 2. 
> https://www.cs.umd.edu/~pugh/java/memoryModel/jsr-133-faq.html#finalRight