JDK-7100935 : win32: memmove is not atomic but is used for pd_conjoint_*_atomic operations
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 7
  • Priority: P2
  • Status: Closed
  • Resolution: Fixed
  • OS: windows
  • CPU: x86
  • Submitted: 2011-10-14
  • Updated: 2014-03-14
  • Resolved: 2012-01-23
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 6 JDK 7 JDK 8 Other
6u60Fixed 7Resolved 8Fixed hs23Fixed
Related Reports
Duplicate :  
JDK-8037373 - win32: memmove is not atomic but is used for pd_conjoint_*_atomic operations
Description
http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2011-October/002473.html

Axel Siebenhorn reports:

on windows-amd64 the interpreter copies compressed oops using memmove.
However, memmove is not thread safe and might copy bytewise.
Another thread can see a partly copied compressed oop.
I added a small test, where a second java thread reads from the array, while the first copies.

The second thread could also be the parallel marking of the CMS.
I prepared  the following webrev:
http://www.sapjvm.com/as/webrevs/atomic_array_copy/

The suggested fix replaces the memmove by a simple copy loop.
Volker Simonis reports:

Oh, it's not just Windows. Just try the attached test on Solaris/SPARC
like this:

java -Xint  ArraycopyTest

Depending on your machine, you'll see more or less errors.
The program does unaligned short array copies which suffer from the
same memmove problem.
In the case of short arrays that only leads to incorrect results and
not to crashes - but thats bad enough!
Comments
EVALUATION See main CR
28-10-2011
EVALUATION http://hg.openjdk.java.net/hsx/hotspot-comp/hotspot/rev/16f9fa2bf76c
19-10-2011
PUBLIC COMMENTS Crash does not happen with -XX:-UseCompressedOops
14-10-2011
PUBLIC COMMENTS I slightly modified the test program to print the incorrect class name: public class TestConjointAtomicArraycopy { static volatile Object [] testArray = new Object [4]; public static void main(String[] args ) throws InterruptedException{ for (int i = 0; i < testArray.length; i++){ testArray[i] = new String("A"); } Thread writer = new Thread (new Runnable(){ public void run(){ for (int i = 0 ; i < 1000000; i++) { System.arraycopy(testArray, 1, testArray, 0, 3); testArray[2] = new String("a"); } } }); Thread reader = new Thread( new Runnable(){ public void run(){ while ( true){ String name = testArray[2].getClass().getName(); if(!(name.endsWith("String"))){ throw new RuntimeException("got wrong class name:" + name); } } } }); reader.start(); writer.start(); writer.join(); System.out.println("Test passed."); System.exit(0); } } and on 64-bit windows it crashed the VM! # # A fatal error has been detected by the Java Runtime Environment: # # EXCEPTION_ACCESS_VIOLATION (0xc0000005) at pc=0x000000006d72057a, pid=5660, tid=5020 # # JRE version: 8.0-b02 # Java VM: Java HotSpot(TM) 64-Bit Server VM (22.0-b01 mixed mode windows-amd64 compressed oops) # Problematic frame: # V [jvm.dll+0xf057a] # # Failed to write core dump. Minidumps are not enabled by default on client versions of Windows # # If you would like to submit a bug report, please visit: # http://bugreport.sun.com/bugreport/crash.jsp # --------------- T H R E A D --------------- Current thread (0x00000000066a5000): JavaThread "Thread-1" [_thread_in_vm, id=5020, stack(0x0000000007bb0000,0x0000000007cb0000)] siginfo: ExceptionCode=0xc0000005, reading address 0x0000000000000078 Registers: RAX=0x0000000000000000, RBX=0x00000000066a5000, RCX=0x0000000000000003, RDX=0x0000000000000000 RSP=0x0000000007caf740, RBP=0x0000000007caf7f8, RSI=0x00000000066a51d0, RDI=0x0000000007caf818 R8 =0x0000000000000004, R9 =0x00000007d5ef9450, R10=0x00000000028021bc, R11=0x000000006d7e96b0 R12=0x0000000000000000, R13=0x000000077ca022f8, R14=0x0000000007caf818, R15=0x00000000066a5000 RIP=0x000000006d72057a, EFLAGS=0x0000000000010246 Top of Stack: (sp=0x0000000007caf740) 0x0000000007caf740: 0000000007caf7b8 000000077caade48 0x0000000007caf750: 0000000000000000 000000077ca07d38 0x0000000007caf760: 00000000066a5000 0000000000000000 0x0000000007caf770: 0000000007caf800 00000000027f6330 0x0000000007caf780: 00000000027f6213 00000000028021e9 0x0000000007caf790: 000000077ca022f8 0000000007caf7f8 0x0000000007caf7a0: 00000000066a5000 000000000000000a 0x0000000007caf7b0: 00000007d5f51c68 0000000007caf7b8 0x0000000007caf7c0: 0000000000000000 0000000007caf818 0x0000000007caf7d0: 000000077caa9a88 0000000000000000 0x0000000007caf7e0: 000000077ca022f8 0000000000000000 0x0000000007caf7f0: 0000000007caf818 0000000007caf860 0x0000000007caf800: 00000000027f6213 0000000000000000 0x0000000007caf810: 00000000027ff01b 00000007d5ffa0c8 0x0000000007caf820: 0000000007caf820 000000077cc4cd55 0x0000000007caf830: 0000000007caf878 000000077cc4d0a0 Instructions: (pc=0x000000006d72057a) 0x000000006d72055a: 00 00 48 8b 17 74 15 8b 52 08 8b 0d be 8b 52 00 0x000000006d72056a: 48 d3 e2 48 03 15 ac 8b 52 00 eb 04 48 8b 52 08 0x000000006d72057a: 48 8b 52 78 48 8b ce e8 6a 4b 0b 00 48 83 7c 24 0x000000006d72058a: 28 00 48 8b f0 74 0a 48 8d 4c 24 20 e8 65 1d 10 Register to memory mapping: RAX=0x0000000000000000 is an unknown value RBX=0x00000000066a5000 is a thread RCX=0x0000000000000003 is an unknown value RDX=0x0000000000000000 is an unknown value RSP=0x0000000007caf740 is pointing into the stack for thread: 0x00000000066a5000 RBP=0x0000000007caf7f8 is pointing into the stack for thread: 0x00000000066a5000 RSI=0x00000000066a51d0 is an unknown value RDI=0x0000000007caf818 is pointing into the stack for thread: 0x00000000066a5000 R8 =0x0000000000000004 is an unknown value R9 =0x00000007d5ef9450 is an oop java.lang.Class - klass: 'java/lang/Class' R10=0x00000000028021bc is an Interpreter codelet method entry point (kind = native) [0x0000000002801f40, 0x00000000028027c0] 2176 bytes R11=0x000000006d7e96b0 is an unknown value R12=0x0000000000000000 is an unknown value R13=0x000000077ca022f8 is an oop {method} - klass: {other class} R14=0x0000000007caf818 is pointing into the stack for thread: 0x00000000066a5000 R15=0x00000000066a5000 is a thread Stack: [0x0000000007bb0000,0x0000000007cb0000], sp=0x0000000007caf740, free space=1021k Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code) V [jvm.dll+0xf057a] j TestConjointAtomicArraycopy$2.run()V+5 j java.lang.Thread.run()V+11 v ~StubRoutines::call_stub V [jvm.dll+0x1a3926] Java frames: (J=compiled Java code, j=interpreted, Vv=VM code) j java.lang.Object.getClass()Ljava/lang/Class;+0 j TestConjointAtomicArraycopy$2.run()V+5 j java.lang.Thread.run()V+11 v ~StubRoutines::call_stub
14-10-2011
SUGGESTED FIX http://www.sapjvm.com/as/webrevs/atomic_array_copy/hg_hsx_hotspotmain.patch --- old/src/os_cpu/windows_x86/vm/copy_windows_x86.inline.hpp 2011-10-13 12:08:44.918387900 +0200 +++ new/src/os_cpu/windows_x86/vm/copy_windows_x86.inline.hpp 2011-10-13 12:08:44.236319700 +0200 @@ -85,13 +85,13 @@ } static void pd_conjoint_jshorts_atomic(jshort* from, jshort* to, size_t count) { - // FIXME - (void)memmove(to, from, count << LogBytesPerShort); + // SAPJVM AS 2011-09-30. memmove might copy byte-wise. + copy_conjoint_atomic<jshort>( from, to, count); } static void pd_conjoint_jints_atomic(jint* from, jint* to, size_t count) { - // FIXME - (void)memmove(to, from, count << LogBytesPerInt); + // SAPJVM AS 2011-09-30. memmove might copy byte-wise. + copy_conjoint_atomic<jint>( from, to, count); } static void pd_conjoint_jlongs_atomic(jlong* from, jlong* to, size_t count) { @@ -128,20 +128,7 @@ } static void pd_conjoint_oops_atomic(oop* from, oop* to, size_t count) { - // Do better than this: inline memmove body NEEDS CLEANUP - if (from > to) { - while (count-- > 0) { - // Copy forwards - *to++ = *from++; - } - } else { - from += count - 1; - to += count - 1; - while (count-- > 0) { - // Copy backwards - *to-- = *from--; - } - } + copy_conjoint_atomic<oop>( from, to, count); } static void pd_arrayof_conjoint_bytes(HeapWord* from, HeapWord* to, size_t count) { --- old/src/share/vm/utilities/copy.hpp 2011-10-13 12:08:52.253121300 +0200 +++ new/src/share/vm/utilities/copy.hpp 2011-10-13 12:08:51.779073900 +0200 @@ -54,6 +54,25 @@ } class Copy : AllStatic { + + // SAPJVM AS 2011-09-20. Template for atomic copy. + template <class T> static void copy_conjoint_atomic(T* from, T* to, size_t count) + { + if (from > to) { + while (count-- > 0) { + // Copy forwards + *to++ = *from++; + } + } else { + from += count - 1; + to += count - 1; + while (count-- > 0) { + // Copy backwards + *to-- = *from--; + } + } + } + public: // Block copy methods have four attributes. We don't define all possibilities. // alignment: aligned to BytesPerLong
14-10-2011
EVALUATION Compressed oops utilizes pd_conjoint_jints_atomic which should atomically copy jints from the src to dest locations. But on win32 we have the following in copy_windows_x96.inline.hpp: static void pd_conjoint_jshorts_atomic(jshort* from, jshort* to, size_t count) { // FIXME (void)memmove(to, from, count << LogBytesPerShort); } static void pd_conjoint_jints_atomic(jint* from, jint* to, size_t count) { // FIXME (void)memmove(to, from, count << LogBytesPerInt); } It is unclear whether the FIXME refers to the atomicity issue or relates to earlier performance work (5039509) but memmove is well known to not be guaranteed to be atomic (its use was replaced on solaris for example). According to the submitter on win32 it will copy byte-by-byte until it reaches an 8-byte aligned address and then copy 64-bits at a time (presumably only on 64-bit systems).
14-10-2011