Fwd: RFR: 8181085: Race condition in method resolution may produce spurious NullPointerException

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() {
+  __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 local_dummy = 0;
+  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::set_direct_or_vtable_call.
> >
> > The patch has been validated on jdk8 by running the reproducer. It stops
> > any resulting NPEs.
> >
> > The code for ConstantPoolCacheEntry::set_direct_or_vtable_call 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 jdk8u-dev mailing list