RFR: 8158854: Ensure release_store is paired with load_acquire in lock-free code

[David Holmes]

Thanks Dan!

David

On 19/08/2016 12:02 PM, Daniel D. Daugherty wrote:
> On 8/17/16 8:50 PM, David Holmes wrote:
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8158854
>>
>> webrev: http://cr.openjdk.java.net/~dholmes/8158854/webrev/
>
> src/share/vm/classfile/classLoader.hpp
>     No comments.
>
> src/share/vm/classfile/verifier.cpp
>     No comments.
>
> src/share/vm/oops/arrayKlass.hpp
>     No comments.
>
> src/share/vm/oops/instanceKlass.cpp
>     No comments.
>
> src/share/vm/oops/instanceKlass.hpp
>     No comments.
>
> src/share/vm/oops/instanceKlass.inline.hpp
>     No comments.
>
> src/share/vm/oops/objArrayKlass.cpp
>     No comments.
>
> src/share/vm/oops/typeArrayKlass.cpp
>     No comments.
>
> src/share/vm/runtime/vmStructs.cpp
>     No comments.
>
> Thumbs up.
>
> Dan
>
>
>>
>>
>> Generally speaking release_store should be paired with load_acquire to
>> ensure correct memory visibility and ordering in lock-free code (often
>> the read path is what is lock-free). So based on some observations
>> from earlier bug fixes this bug was intended to examine the use of
>> release_store and see if we have the appropriate load_acquire as well.
>> The bug report lists all of the cases that were examined - some clear
>> cut correct, some complex correct, some fixed here and some split out
>> into separate issues.
>>
>> Here's a summary of the actual changes in the webrev:
>>
>>  src/share/vm/classfile/classLoader.hpp
>>
>> - next() accessor needs to use load_acquire.
>>
>> ---
>>
>> src/share/vm/classfile/verifier.cpp
>>
>> - load of _verify_byte_codes_fn needs to load_acquire to pair with use
>> of release_store
>> - release_store of _is_new_verify_byte_codes_fn is not needed
>>
>> ---
>>
>> src/share/vm/oops/arrayKlass.hpp
>> src/share/vm/oops/instanceKlass.cpp
>> src/share/vm/oops/instanceKlass.hpp
>> src/share/vm/oops/instanceKlass.inline.hpp
>> src/share/vm/oops/objArrayKlass.cpp
>> src/share/vm/oops/typeArrayKlass.cpp
>>
>> The logic for storing dimensions values was using a storeStore barrier
>> between the lower and higher dimensions. This is converted to use a
>> release-store setter for higher-dimension, with paired load-acquire
>> accessor. Plus the accessed fields are declared volatile.
>>
>> The methods_jmethod_ids_acquire() and its paired
>> release_set_methods_jmethod_ids(), are moved to the .inline.hpp file
>> where they belong.
>>
>> ---
>>
>> src/share/vm/runtime/vmStructs.cpp
>>
>> Updated declaration for _array_klasses  now it is volatile.
>>
>> ---
>>
>> Thanks,
>> David
>