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

Martin Buchholz martinrb at google.com
Thu May 25 18:55:58 UTC 2017

[+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:

--- hotspot/src/os_cpu/linux_x86/vm/orderAccess_linux_x86.inline.hpp 2016-11-22 15:30:39.000000000 -0800 +++ hotspot/src/os_cpu/linux_x86/vm/orderAccess_linux_x86.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 compiler_barrier() {

inline void OrderAccess::fence() { @@ -63,34 +66,34 @@ } }

-inline jbyte OrderAccess::load_acquire(volatile jbyte* p) { return p; } -inline jshort OrderAccess::load_acquire(volatile jshort p) { return p; } -inline jint OrderAccess::load_acquire(volatile jint p) { return p; } -inline jlong OrderAccess::load_acquire(volatile jlong p) { return Atomic::load(p); } -inline jubyte OrderAccess::load_acquire(volatile jubyte* p) { return p; } -inline jushort OrderAccess::load_acquire(volatile jushort p) { return p; } -inline juint OrderAccess::load_acquire(volatile juint p) { return p; } -inline julong OrderAccess::load_acquire(volatile julong p) { return Atomic::load((volatile jlong*)p); } -inline jfloat OrderAccess::load_acquire(volatile jfloat* p) { return p; } -inline jdouble OrderAccess::load_acquire(volatile jdouble p) { return jdouble_cast(Atomic::load((volatile jlong*)p)); }

-inline intptr_t OrderAccess::load_ptr_acquire(volatile intptr_t* p) { return p; } -inline void OrderAccess::load_ptr_acquire(volatile void* p) { return (void volatile )p; } -inline void OrderAccess::load_ptr_acquire(const volatile void* p) { return (void const volatile *)p; }

-inline void OrderAccess::release_store(volatile jbyte* p, jbyte v) { p = v; } -inline void OrderAccess::release_store(volatile jshort p, jshort v) { p = v; } -inline void OrderAccess::release_store(volatile jint p, jint v) { p = v; } -inline void OrderAccess::release_store(volatile jlong p, jlong v) { Atomic::store(v, p); } -inline void OrderAccess::release_store(volatile jubyte* p, jubyte v) { p = v; } -inline void OrderAccess::release_store(volatile jushort p, jushort v) { p = v; } -inline void OrderAccess::release_store(volatile juint p, juint v) { p = v; } -inline void OrderAccess::release_store(volatile julong p, julong v) { Atomic::store((jlong)v, (volatile jlong*)p); } -inline void OrderAccess::release_store(volatile jfloat* p, jfloat v) { p = v; } +inline jbyte OrderAccess::load_acquire(volatile jbyte p) { jbyte v = p; compiler_barrier(); return v; } +inline jshort OrderAccess::load_acquire(volatile jshort p) { jshort v = p; compiler_barrier(); return v; } +inline jint OrderAccess::load_acquire(volatile jint p) { jint v = p; compiler_barrier(); return v; } +inline jlong OrderAccess::load_acquire(volatile jlong p) { jlong v = Atomic::load(p); compiler_barrier(); return v; } +inline jubyte OrderAccess::load_acquire(volatile jubyte* p) { jubyte v = p; compiler_barrier(); return v; } +inline jushort OrderAccess::load_acquire(volatile jushort p) { jushort v = p; compiler_barrier(); return v; } +inline juint OrderAccess::load_acquire(volatile juint p) { juint v = p; compiler_barrier(); return v; } +inline julong OrderAccess::load_acquire(volatile julong p) { julong v = Atomic::load((volatile jlong*)p); compiler_barrier(); return v; } +inline jfloat OrderAccess::load_acquire(volatile jfloat* p) { jfloat v = p; compiler_barrier(); return v; } +inline jdouble OrderAccess::load_acquire(volatile jdouble p) { jdouble v = jdouble_cast(Atomic::load((volatile jlong*)p)); compiler_barrier(); return v; } + +inline intptr_t OrderAccess::load_ptr_acquire(volatile intptr_t* p) { intptr_t v = p; compiler_barrier(); return v; } +inline void OrderAccess::load_ptr_acquire(volatile void* p) { void* v = (void volatile )p; compiler_barrier(); return v; } +inline void OrderAccess::load_ptr_acquire(const volatile void* p) { void* v = (void const volatile )p; compiler_barrier(); return v; } + +inline void OrderAccess::release_store(volatile jbyte p, jbyte v) { compiler_barrier(); p = v; } +inline void OrderAccess::release_store(volatile jshort p, jshort v) { compiler_barrier(); p = v; } +inline void OrderAccess::release_store(volatile jint p, jint v) { compiler_barrier(); p = v; } +inline void OrderAccess::release_store(volatile jlong p, jlong v) { compiler_barrier(); Atomic::store(v, p); } +inline void OrderAccess::release_store(volatile jubyte* p, jubyte v) { compiler_barrier(); p = v; } +inline void OrderAccess::release_store(volatile jushort p, jushort v) { compiler_barrier(); p = v; } +inline void OrderAccess::release_store(volatile juint p, juint v) { compiler_barrier(); p = v; } +inline void OrderAccess::release_store(volatile julong p, julong v) { compiler_barrier(); Atomic::store((jlong)v, (volatile jlong*)p); } +inline void OrderAccess::release_store(volatile jfloat* p, jfloat v) { compiler_barrier(); p = v; } inline void OrderAccess::release_store(volatile jdouble p, jdouble v) { release_store((volatile jlong *)p, jlong_cast(v)); }

-inline void OrderAccess::release_store_ptr(volatile intptr_t* p, intptr_t v) { p = v; } -inline void OrderAccess::release_store_ptr(volatile void p, void* v) { (void volatile )p = v; } +inline void OrderAccess::release_store_ptr(volatile intptr_t p, intptr_t v) { compiler_barrier(); p = v; } +inline void OrderAccess::release_store_ptr(volatile void p, void* v) { compiler_barrier(); (void volatile *)p = v; }

inline void OrderAccess::store_fence(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