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

[Peter Levart]

On 06/18/2018 04:47 PM, Claes Redestad wrote:
>
>
> On 2018-06-18 16:23, Peter Levart wrote:
>> Hi Claes,
>>
>> On 06/18/2018 03:54 PM, Claes Redestad wrote:
>>> I'd suggest something simple like this to ensure correctness, while 
>>> keeping the number of volatile reads to a minimum:
>>>
>>> http://cr.openjdk.java.net/~redestad/8199435.00/
>>>
>>> Then file a follow-up RFE to investigate if we can make these fields 
>>> non-volatile, e.g. using careful fencing as suggested
>>> by Peter.
>>
>> OK, but the constructor will still need a releaseFence at the end to 
>> prevent a potential write that unsafely publishes Properties instance 
>> to float before the volatile writes of 'map' and 'defaults'...
>>
>> You might want to use Unsafe directly for that since VarHandle could 
>> cause bootstrap issues.
>
> I don't follow.. which constructor needs a fence in the suggested patch?
>
> /Claes

The Properties constructor:


     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;

         UNSAFE.storeFence(); // <-- HERE!!
     }


Final fields have bigger guarantee than volatile fields in this respect.

Plain write that follows in program a volatile write, can in theory 
float before the volatile write. So if you publish a Properties instance 
via data race (via plain write), the observer may still see 
uninitialized 'map' and 'defaults' fields.

Regards, Peter