(S) RFR: 8157904: Atomic::cmpxchg for jbyte is missing a fence on initial failure

David Holmes david.holmes at oracle.com
Mon Aug 22 07:54:03 UTC 2016


Bug: https://bugs.openjdk.java.net/browse/JDK-8157904

webrev: http://cr.openjdk.java.net/~dholmes/8157904/webrev/


An earlier code review noticed that the default shared implementation of 
Atomic::cmpxchg(jbyte*) was missing the required post-memory-barrier in 
case of an initial failure:

   while (cur_as_bytes[offset] == comparand) {
      jint res = cmpxchg(new_val, dest_int, cur, order);
       ...
   }

if we never enter the while loop we don't do a cmpxchg and so we have no 
memory barrier.

The simple fix is to invert things and use a do { } while () loop. That 
way we always execute at least one real cmpxchg and so get the required 
memory barrier. For that to work we also have to preload comparand into 
the initial jint value we expect to see - as Kim pointed out.

Additionally Kim updated the code to get rid of C-style casts and direct 
pointer arithmetic. I kept some of the intermediate locals for improved 
readability.

Testing:

The only platform that potentially uses the shared implementation is 
solaris_sparc (the others all have specialized asm variants). However as 
I don't have a sparc system to readily test on I had to tweak an 
atomic_linux_x86.hpp to use this on linux-x86.

Then it turns out that we don't have any existing callers of the 
cmpxchg(byte) variant, because at the Java-level in Unsafe it has also 
been written in terms of cmpxchg(int) [this will probably be fixed so 
that intrinsics can be used]. So I had to modify Unsafe.java and 
unsafe.cpp to add in a variant that uses Atomic::cmpxchg(jbyte*), and 
then I modified the VarHandles code to use those new variants and then 
ran the VarHandles tests. Those tests passed with both the updated 
shared variant and the linux-x86 specific version.

Thanks,
David


More information about the hotspot-runtime-dev mailing list