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

[Daniel D. Daugherty]

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