RFR(XS) 8152846: Creation of ModuleEntryTable Investigate Need For OrderAccess::storestore()

[harold seigel]

Hi David,

Thanks for looking at this.  I agree that the storestore is needed and 
will open a new bug to address this.

Harold

On 4/16/2016 5:35 AM, David Holmes wrote:
> Hi Harold,
>
> On 6/04/2016 3:52 AM, harold seigel wrote:
>> Hi,
>>
>> Please review this small change to fix bug 8152846.
>>
>> This change removes an unneeded OrderAccess::storestore() call.
>>
>> Webrev: http://javaweb.us.oracle.com/~hseigel/webrev/bug_8152846/
>
> Please ensure you provide an accessible webrev link please.
>
> I have a concern with this code. I think the storeStore as written was 
> in the wrong place. The main concern with lock-free read of a field 
> like _packages is that if you see a non-null value then you also see 
> all the stores that were made as part of the construction of the 
> object _packages refers to. In that regard I think the original code:
>
>     // Ensure _packages is stable, since it is examined without a lock
>      OrderAccess::storestore();
>     _packages = new 
> PackageEntryTable(PackageEntryTable::_packagetable_entry_size);
>    }
>   return _packages;
>
> Should really have bee:
>
>  temp = new 
> PackageEntryTable(PackageEntryTable::_packagetable_entry_size);
>  OrderAccess::storeStore();
>  _packages = tmp;
>  ...
>
> though to be sure we need to examine the lock-free usage of _packages.
>
> David
> -----
>
>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8152846
>>
>> The fix was tested with JCK API, Lang, and VM tests, the UTE
>> non-colocated quick tests, and the hotspot, JDK vm, java/io, java/lang,
>> java/util, and java/security tests.  Testing with UTE colocated tests is
>> in progress.
>>
>> Thanks, Harold