src/hotspot/share/runtime/handshake.cpp

@@ -51,22 +51,22 @@
     _is_direct(is_direct) {}
 
   void do_handshake(JavaThread* thread);
   bool is_completed() {
     int32_t val = Atomic::load(&_pending_threads);
-    assert(val >= 0, "_pending_threads cannot be negative");
+    assert(val >= 0, "_pending_threads=%d cannot be negative", val);
     return val == 0;
   }
   void add_target_count(int count) { Atomic::add(&_pending_threads, count); }
   bool executed() const { return _executed; }
   const char* name() { return _handshake_cl->name(); }
 
   bool is_direct() { return _is_direct; }
 
 #ifdef ASSERT
   void check_state() {
-    assert(_pending_threads == 0, "Must be zero");
+    assert(_pending_threads == 0, "Must be zero: %d", _pending_threads);
   }
 #endif
 };
 
 class VM_Handshake: public VM_Operation {

@@ -223,11 +223,16 @@
     jlong completion_time = os::javaTimeNanos() - start_time_ns;
     log_debug(handshake, task)("Operation: %s for thread " PTR_FORMAT ", is_vm_thread: %s, completed in " JLONG_FORMAT " ns",
                                name(), p2i(thread), BOOL_TO_STR(Thread::current()->is_VM_thread()), completion_time);
   }
 
-  // Inform VMThread/Handshaker that we have completed the operation
+  // Inform VMThread/Handshaker that we have completed the operation.
+  // When this is executed by the Handshakee we need a release store
+  // here to make sure memory operations executed in the handshake
+  // closure are visible to the Handshaker after it reads that the
+  // operation has completed. Atomic::dec() already provides a full
+  // memory fence.
   Atomic::dec(&_pending_threads);
 
   // It is no longer safe to refer to 'this' as the VMThread/Handshaker may have destroyed this operation
 }
 

@@ -271,10 +276,15 @@
     }
   }
   DEBUG_ONLY(op.check_state();)
   log_handshake_info(start_time_ns, op.name(), 1, by_handshaker ? 1 : 0);
 
+  // Prevent future loads from floating above the load of _pending_threads
+  // in is_completed(). This prevents reading stale data modified in the
+  // handshake closure by the Handshakee.
+  OrderAccess::acquire();
+
   return op.executed();
 }
 
 HandshakeState::HandshakeState() :
   _operation(NULL),

@@ -397,11 +407,11 @@
   }
 
   // Check if the handshake operation is the same as the one we meant to execute. The
   // handshake could have been already processed by the handshakee and a new handshake
   // by another JavaThread might be in progress.
-  if ( (is_direct && op != _operation_direct)) {
+  if (is_direct && op != _operation_direct) {
     _processing_sem.signal();
     return false;
   }
 
   // If we own the semaphore at this point and while owning the semaphore