Fwd: RFR: 8181085: Race condition in method resolution may produce spurious NullPointerException (original) (raw)

David Holmes david.holmes at oracle.com
Fri May 26 01:04:56 UTC 2017

On 26/05/2017 4:55 AM, Martin Buchholz wrote:

[+jdk8u-dev]

We've been hunting the elusive spurious NPEs as well; the following seems to be working for us (but we don't have any small repro recipe); something like this should be put into jdk8:

In other words you want a backport of: 8061964: Insufficient compiler barriers for GCC in OrderAccess functions …

https://bugs.openjdk.java.net/browse/JDK-8061964

IIRC what stopped this from being an 'automatic' backport candidate was the potential problem of older gcc's needing to be validated.

Cheers, David

--- hotspot/src/oscpu/linuxx86/vm/orderAccesslinuxx86.inline.hpp 2016-11-22 15:30:39.000000000 -0800 +++ hotspot/src/oscpu/linuxx86/vm/orderAccesslinuxx86.inline.hpp 2017-04-27 18:12:33.000000000 -0700 @@ -32,6 +32,11 @@

// Implementation of class OrderAccess. +// A compiler barrier, forcing the C++ compiler to invalidate all memory assumptions +static inline void compilerbarrier() { + asm volatile ("" : : : "memory"); +} + inline void OrderAccess::loadload() { acquire(); } inline void OrderAccess::storestore() { release(); } inline void OrderAccess::loadstore() { acquire(); } @@ -47,9 +52,7 @@ } inline void OrderAccess::release() { - // Avoid hitting the same cache-line from - // different threads. - volatile jint localdummy = 0; + compilerbarrier(); } inline void OrderAccess::fence() { @@ -63,34 +66,34 @@ } } -inline jbyte OrderAccess::loadacquire(volatile jbyte* p) { return *p; } -inline jshort OrderAccess::loadacquire(volatile jshort* p) { return *p; } -inline jint OrderAccess::loadacquire(volatile jint* p) { return *p; } -inline jlong OrderAccess::loadacquire(volatile jlong* p) { return Atomic::load(p); } -inline jubyte OrderAccess::loadacquire(volatile jubyte* p) { return *p; } -inline jushort OrderAccess::loadacquire(volatile jushort* p) { return *p; } -inline juint OrderAccess::loadacquire(volatile juint* p) { return *p; } -inline julong OrderAccess::loadacquire(volatile julong* p) { return Atomic::load((volatile jlong*)p); } -inline jfloat OrderAccess::loadacquire(volatile jfloat* p) { return *p; } -inline jdouble OrderAccess::loadacquire(volatile jdouble* p) { return jdoublecast(Atomic::load((volatile jlong*)p)); } - -inline intptrt OrderAccess::loadptracquire(volatile intptrt* p) { return *p; } -inline void* OrderAccess::loadptracquire(volatile void* p) { return (void volatile *)p; } -inline void* OrderAccess::loadptracquire(const volatile void* p) { return (void const volatile *)p; } - -inline void OrderAccess::releasestore(volatile jbyte* p, jbyte v) { *p = v; } -inline void OrderAccess::releasestore(volatile jshort* p, jshort v) { *p = v; } -inline void OrderAccess::releasestore(volatile jint* p, jint v) { *p = v; } -inline void OrderAccess::releasestore(volatile jlong* p, jlong v) { Atomic::store(v, p); } -inline void OrderAccess::releasestore(volatile jubyte* p, jubyte v) { *p = v; } -inline void OrderAccess::releasestore(volatile jushort* p, jushort v) { *p = v; } -inline void OrderAccess::releasestore(volatile juint* p, juint v) { *p = v; } -inline void OrderAccess::releasestore(volatile julong* p, julong v) { Atomic::store((jlong)v, (volatile jlong*)p); } -inline void OrderAccess::releasestore(volatile jfloat* p, jfloat v) { *p = v; } +inline jbyte OrderAccess::loadacquire(volatile jbyte* p) { jbyte v = *p; compilerbarrier(); return v; } +inline jshort OrderAccess::loadacquire(volatile jshort* p) { jshort v = *p; compilerbarrier(); return v; } +inline jint OrderAccess::loadacquire(volatile jint* p) { jint v = *p; compilerbarrier(); return v; } +inline jlong OrderAccess::loadacquire(volatile jlong* p) { jlong v = Atomic::load(p); compilerbarrier(); return v; } +inline jubyte OrderAccess::loadacquire(volatile jubyte* p) { jubyte v = *p; compilerbarrier(); return v; } +inline jushort OrderAccess::loadacquire(volatile jushort* p) { jushort v = *p; compilerbarrier(); return v; } +inline juint OrderAccess::loadacquire(volatile juint* p) { juint v = *p; compilerbarrier(); return v; } +inline julong OrderAccess::loadacquire(volatile julong* p) { julong v = Atomic::load((volatile jlong*)p); compilerbarrier(); return v; } +inline jfloat OrderAccess::loadacquire(volatile jfloat* p) { jfloat v = *p; compilerbarrier(); return v; } +inline jdouble OrderAccess::loadacquire(volatile jdouble* p) { jdouble v = jdoublecast(Atomic::load((volatile jlong*)p)); compilerbarrier(); return v; } + +inline intptrt OrderAccess::loadptracquire(volatile intptrt* p) { intptrt v = *p; compilerbarrier(); return v; } +inline void* OrderAccess::loadptracquire(volatile void* p) { void* v = (void volatile *)p; compilerbarrier(); return v; } +inline void* OrderAccess::loadptracquire(const volatile void* p) { void* v = (void const volatile *)p; compilerbarrier(); return v; } + +inline void OrderAccess::releasestore(volatile jbyte* p, jbyte v) { compilerbarrier(); *p = v; } +inline void OrderAccess::releasestore(volatile jshort* p, jshort v) { compilerbarrier(); *p = v; } +inline void OrderAccess::releasestore(volatile jint* p, jint v) { compilerbarrier(); *p = v; } +inline void OrderAccess::releasestore(volatile jlong* p, jlong v) { compilerbarrier(); Atomic::store(v, p); } +inline void OrderAccess::releasestore(volatile jubyte* p, jubyte v) { compilerbarrier(); *p = v; } +inline void OrderAccess::releasestore(volatile jushort* p, jushort v) { compilerbarrier(); *p = v; } +inline void OrderAccess::releasestore(volatile juint* p, juint v) { compilerbarrier(); *p = v; } +inline void OrderAccess::releasestore(volatile julong* p, julong v) { compilerbarrier(); Atomic::store((jlong)v, (volatile jlong*)p); } +inline void OrderAccess::releasestore(volatile jfloat* p, jfloat v) { compilerbarrier(); *p = v; } inline void OrderAccess::releasestore(volatile jdouble* p, jdouble v) { releasestore((volatile jlong *)p, jlongcast(v)); } -inline void OrderAccess::releasestoreptr(volatile intptrt* p, intptrt v) { *p = v; } -inline void OrderAccess::releasestoreptr(volatile void* p, void* v) { (void volatile *)p = v; } +inline void OrderAccess::releasestoreptr(volatile intptrt* p, intptrt v) { compilerbarrier(); *p = v; } +inline void OrderAccess::releasestoreptr(volatile void* p, void* v) { compilerbarrier(); (void volatile *)p = v; } inline void OrderAccess::storefence(jbyte* p, jbyte v) { asm volatile ( "xchgb (%2),%0"

On Thu, May 25, 2017 at 9:25 AM, Andrew Dinn <adinn at redhat.com> wrote: Apologies but this RFR is retracted -- the problem only applies to jdk8.

I will be posting a revised RFR to jdk8u. 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 On 25/05/17 14:16, Andrew Dinn wrote: Forwarding this to hotpsot-dev which is probably the more appropriate destination.

-------- Forwarded Message -------- Subject: RFR: 8181085: Race condition in method resolution may produce spurious NullPointerException Date: Thu, 25 May 2017 14:12:53 +0100 From: Andrew Dinn <adinn at redhat.com> To: jdk10-dev <jdk10-dev at openjdk.java.net> The following webrev fixes a race condition that is present in jdk10 and also jdk9 and jdk8. It is caused by a misplaced volatile keyword that faild to ensure correct ordering of writes by the compiler. Reviews welcome. http://cr.openjdk.java.net/~adinn/8181085/webrev.00/ Backporting: This same fix is required in jdk9 and jdk8. Testing: The reproducer posted with the original issue manifests the NPE reliably on jdk8. It does not manifest on jdk9/10 but that is only thanks to changes introduced into the resolution process in jdk9 which change the timing of execution. However, without this fix the out-of-order write problem is still present in jdk9/10, as can be seen by eyeballing the compiled code for ConstantPoolCacheEntry::setdirectorvtablecall. The patch has been validated on jdk8 by running the reproducer. It stops any resulting NPEs. The code for ConstantPoolCacheEntry::setdirectorvtablecall on jdk8-10 has been eyeballed to ensure that post-patch the assignments now occur in the correct order. 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

More information about the hotspot-dev mailing list