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

[Peter Levart]

Hi Brent,

If 'map' field could be observed as null (which I think is only 
possible, if Properties object is unsafely published), then so can 
'defaults' field. You could make it final as well if it was not protected.

So I'm thinking what kind of memory fences would make those fields 
behave so that they would not be observed uninitialized even when the 
instance is published via data race. I think that the following would be 
enough, for example:

     private Properties(Properties defaults, int initialCapacity) {
         // use package-private constructor to
         // initialize unused fields with dummy values
         super((Void) null);
         map = new ConcurrentHashMap<>(initialCapacity);
         this.defaults = defaults;
         VarHandle.storeStoreFence();
     }

...

     public String getProperty(String key) {
         VarHandle.loadLoadFence();
         Object oval = map.get(key);
         String sval = (oval instanceof String) ? (String)oval : null;
         return ((sval == null) && (defaults != null)) ? 
defaults.getProperty(key) : sval;
     }


...of course, you would have to put loadLoadFence() at the beggining of 
each method that uses any of those two fields. Accesses to 'defaults' in 
subclasses wouldn't be included, but at least accesses in Properties 
class would be covered.


Regards, Peter

On 06/15/18 17:44, 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.
>
> 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