RFR (S) 8182397: Race in field updates when creating ArrayKlasses can lead to crash

[coleen.phillimore at oracle.com]

On 7/21/17 11:23 AM, Andrew Dinn wrote:
> On 21/07/17 15:23, coleen.phillimore at oracle.com wrote:
>> Andrew,  Thank you for your comments and suggestions.
> All jus' part of the job, ma'am ... :-)
>
>> I copied this from
>>
>> void oopDesc::obj_field_put_volatile(int offset, oop value) {
>>    OrderAccess::release();
>>    obj_field_put(offset, value);
>>    OrderAccess::fence();
>> }
>>
>> Is it overkill also in the obj_field_put_volatile case?
> Well, I would assume from the name that obj_field_put_volatile is used
> to do a volatile write internally from the JVM with the intention that
> it be equivalent to doing a volatile write from JDK code.
>
> If so then, in general, I /guess/ that might perhaps demand a full fence
> even though on some platforms (like AArch64) we /can/ and /do/ implement
> volatile writes in JITted code using only a releasing store. Andrew
> Haley might know better on this score.
>
> That said, the method you have defined is only (currently) used to
> ensure that the two stores for the mirror and array klass are ordered
> correctly as regards a read through from klass to mirror. That usage
> definitely only needs a releasing store (assuming the memory system
> architecture respects address dependencies).
>
> In which case it might be more appropriate to rename the method you
> inserted to metadata_field_put_release -- highlighting the fact that
> this is all the order guarantee it provides.

Thank you.  I'll rename it to release to be more clear.  I'm not 
comfortable messing around with the other function, I was just curious.
>
>>> Alternatively, I believe you could implement the assign of using a call
>>> to OrderAccess::store_release_ptr which would achieve the same outcome
>>> on AArch64.
>> This is nicer.  Is this what you mean?
>>
>> void oopDesc::metadata_field_put_volatile(int offset, Metadata* value) {
>>    OrderAccess::release_store_ptr(metadata_field_addr(offset), value);
>> }
> Yes, that was what I was thinking of.

Thank you for the code review.
Coleen

>
> regards,
>
>
> Andrew Dinn
> -----------
> Senior Principal Software Engineer
> Red Hat UK Ltd
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander