8240918: [REDO] Allow direct handshakes without VMThread intervention

REDO of 8230594

Reviewed-by: dholmes, rehn, mdoerr, coleenp, dcubed

src/hotspot/share/runtime/biasedLocking.cpp

@@ -622,7 +622,7 @@ BiasedLocking::Condition BiasedLocking::single_revoke_with_handshake(Handle obj,
                                      p2i(biaser), p2i(obj()));
 
   RevokeOneBias revoke(obj, requester, biaser);
-  bool executed = Handshake::execute(&revoke, biaser);
+  bool executed = Handshake::execute_direct(&revoke, biaser);
   if (revoke.status_code() == NOT_REVOKED) {
     return NOT_REVOKED;
   }
@@ -666,24 +666,24 @@ BiasedLocking::Condition BiasedLocking::single_revoke_with_handshake(Handle obj,
 
 // Caller should have instantiated a ResourceMark object before calling this method
 void BiasedLocking::walk_stack_and_revoke(oop obj, JavaThread* biased_locker) {
+  Thread* cur = Thread::current();
   assert(!SafepointSynchronize::is_at_safepoint(), "this should always be executed outside safepoints");
-  assert(Thread::current() == biased_locker || Thread::current()->is_VM_thread(), "wrong thread");
+  assert(cur == biased_locker || cur == biased_locker->active_handshaker(), "wrong thread");
 
   markWord mark = obj->mark();
   assert(mark.biased_locker() == biased_locker &&
          obj->klass()->prototype_header().bias_epoch() == mark.bias_epoch(), "invariant");
 
-  log_trace(biasedlocking)("%s(" INTPTR_FORMAT ") revoking object " INTPTR_FORMAT ", mark "
+  log_trace(biasedlocking)("JavaThread(" INTPTR_FORMAT ") revoking object " INTPTR_FORMAT ", mark "
                            INTPTR_FORMAT ", type %s, prototype header " INTPTR_FORMAT
                            ", biaser " INTPTR_FORMAT " %s",
-                           Thread::current()->is_VM_thread() ? "VMThread" : "JavaThread",
-                           p2i(Thread::current()),
+                           p2i(cur),
                            p2i(obj),
                            mark.value(),
                            obj->klass()->external_name(),
                            obj->klass()->prototype_header().value(),
                            p2i(biased_locker),
-                           Thread::current()->is_VM_thread() ? "" : "(walking own stack)");
+                           cur != biased_locker ? "" : "(walking own stack)");
 
   markWord unbiased_prototype = markWord::prototype().set_age(obj->mark().age());
 

src/hotspot/share/runtime/handshake.cpp

@@ -37,51 +37,43 @@
 #include "utilities/formatBuffer.hpp"
 #include "utilities/preserveException.hpp"
 
-class HandshakeOperation: public StackObj {
-public:
-  virtual void do_handshake(JavaThread* thread) = 0;
-};
 
-class HandshakeThreadsOperation: public HandshakeOperation {
-  static Semaphore _done;
+class HandshakeOperation: public StackObj {
   HandshakeClosure* _handshake_cl;
+  int32_t _pending_threads;
   bool _executed;
+  bool _is_direct;
 public:
-  HandshakeThreadsOperation(HandshakeClosure* cl) : _handshake_cl(cl), _executed(false) {}
+  HandshakeOperation(HandshakeClosure* cl, bool is_direct = false) :
+    _handshake_cl(cl),
+    _pending_threads(1),
+    _executed(false),
+    _is_direct(is_direct) {}
+
   void do_handshake(JavaThread* thread);
-  bool thread_has_completed() { return _done.trywait(); }
+  bool is_completed() {
+    int32_t val = Atomic::load(&_pending_threads);
+    assert(val >= 0, "_pending_threads=%d cannot be negative", val);
+    return val == 0;
+  }
+  void add_target_count(int count) { Atomic::add(&_pending_threads, count, memory_order_relaxed); }
   bool executed() const { return _executed; }
   const char* name() { return _handshake_cl->name(); }
 
-#ifdef ASSERT
-  void check_state() {
-    assert(!_done.trywait(), "Must be zero");
-  }
-#endif
+  bool is_direct() { return _is_direct; }
 };
 
-Semaphore HandshakeThreadsOperation::_done(0);
-
 class VM_Handshake: public VM_Operation {
   const jlong _handshake_timeout;
  public:
   bool evaluate_at_safepoint() const { return false; }
 
  protected:
-  HandshakeThreadsOperation* const _op;
+  HandshakeOperation* const _op;
 
-  VM_Handshake(HandshakeThreadsOperation* op) :
+  VM_Handshake(HandshakeOperation* op) :
       _handshake_timeout(TimeHelper::millis_to_counter(HandshakeTimeout)), _op(op) {}
 
-  void set_handshake(JavaThread* target) {
-    target->set_handshake_operation(_op);
-  }
-
-  // This method returns true for threads completed their operation
-  // and true for threads canceled their operation.
-  // A cancellation can happen if the thread is exiting.
-  bool poll_for_completed_thread() { return _op->thread_has_completed(); }
-
   bool handshake_has_timed_out(jlong start_time);
   static void handle_timeout();
 };
@@ -121,12 +113,10 @@ static void log_handshake_info(jlong start_time_ns, const char* name, int target
 class VM_HandshakeOneThread: public VM_Handshake {
   JavaThread* _target;
  public:
-  VM_HandshakeOneThread(HandshakeThreadsOperation* op, JavaThread* target) :
+  VM_HandshakeOneThread(HandshakeOperation* op, JavaThread* target) :
     VM_Handshake(op), _target(target) {}
 
   void doit() {
-    DEBUG_ONLY(_op->check_state();)
-
     jlong start_time_ns = 0;
     if (log_is_enabled(Info, handshake)) {
       start_time_ns = os::javaTimeNanos();
@@ -134,7 +124,7 @@ class VM_HandshakeOneThread: public VM_Handshake {
 
     ThreadsListHandle tlh;
     if (tlh.includes(_target)) {
-      set_handshake(_target);
+      _target->set_handshake_operation(_op);
     } else {
       log_handshake_info(start_time_ns, _op->name(), 0, 0, "(thread dead)");
       return;
@@ -147,9 +137,15 @@ class VM_HandshakeOneThread: public VM_Handshake {
       if (handshake_has_timed_out(timeout_start_time)) {
         handle_timeout();
       }
-      by_vm_thread = _target->handshake_try_process_by_vmThread();
-    } while (!poll_for_completed_thread());
-    DEBUG_ONLY(_op->check_state();)
+      by_vm_thread = _target->handshake_try_process(_op);
+    } while (!_op->is_completed());
+
+    // This pairs up with the release store in do_handshake(). It prevents future
+    // loads from floating above the load of _pending_threads in is_completed()
+    // and thus prevents reading stale data modified in the handshake closure
+    // by the Handshakee.
+    OrderAccess::acquire();
+
     log_handshake_info(start_time_ns, _op->name(), 1, by_vm_thread ? 1 : 0);
   }
 
@@ -160,11 +156,9 @@ class VM_HandshakeOneThread: public VM_Handshake {
 
 class VM_HandshakeAllThreads: public VM_Handshake {
  public:
-  VM_HandshakeAllThreads(HandshakeThreadsOperation* op) : VM_Handshake(op) {}
+  VM_HandshakeAllThreads(HandshakeOperation* op) : VM_Handshake(op) {}
 
   void doit() {
-    DEBUG_ONLY(_op->check_state();)
-
     jlong start_time_ns = 0;
     if (log_is_enabled(Info, handshake)) {
       start_time_ns = os::javaTimeNanos();
@@ -174,7 +168,7 @@ class VM_HandshakeAllThreads: public VM_Handshake {
     JavaThreadIteratorWithHandle jtiwh;
     int number_of_threads_issued = 0;
     for (JavaThread *thr = jtiwh.next(); thr != NULL; thr = jtiwh.next()) {
-      set_handshake(thr);
+      thr->set_handshake_operation(_op);
       number_of_threads_issued++;
     }
 
@@ -182,10 +176,11 @@ class VM_HandshakeAllThreads: public VM_Handshake {
       log_handshake_info(start_time_ns, _op->name(), 0, 0);
       return;
     }
+    // _op was created with a count == 1 so don't double count.
+    _op->add_target_count(number_of_threads_issued - 1);
 
     log_trace(handshake)("Threads signaled, begin processing blocked threads by VMThread");
     const jlong start_time = os::elapsed_counter();
-    int number_of_threads_completed = 0;
     do {
       // Check if handshake operation has timed out
       if (handshake_has_timed_out(start_time)) {
@@ -198,19 +193,18 @@ class VM_HandshakeAllThreads: public VM_Handshake {
       jtiwh.rewind();
       for (JavaThread *thr = jtiwh.next(); thr != NULL; thr = jtiwh.next()) {
         // A new thread on the ThreadsList will not have an operation,
-        // hence it is skipped in handshake_process_by_vmthread.
-        if (thr->handshake_try_process_by_vmThread()) {
+        // hence it is skipped in handshake_try_process.
+        if (thr->handshake_try_process(_op)) {
           handshake_executed_by_vm_thread++;
         }
       }
-      while (poll_for_completed_thread()) {
-        // Includes canceled operations by exiting threads.
-        number_of_threads_completed++;
-      }
+    } while (!_op->is_completed());
 
-    } while (number_of_threads_issued > number_of_threads_completed);
-    assert(number_of_threads_issued == number_of_threads_completed, "Must be the same");
-    DEBUG_ONLY(_op->check_state();)
+    // This pairs up with the release store in do_handshake(). It prevents future
+    // loads from floating above the load of _pending_threads in is_completed()
+    // and thus prevents reading stale data modified in the handshake closure
+    // by the Handshakee.
+    OrderAccess::acquire();
 
     log_handshake_info(start_time_ns, _op->name(), number_of_threads_issued, handshake_executed_by_vm_thread);
   }
@@ -218,7 +212,7 @@ class VM_HandshakeAllThreads: public VM_Handshake {
   VMOp_Type type() const { return VMOp_HandshakeAllThreads; }
 };
 
-void HandshakeThreadsOperation::do_handshake(JavaThread* thread) {
+void HandshakeOperation::do_handshake(JavaThread* thread) {
   jlong start_time_ns = 0;
   if (log_is_enabled(Debug, handshake, task)) {
     start_time_ns = os::javaTimeNanos();
@@ -236,80 +230,149 @@ void HandshakeThreadsOperation::do_handshake(JavaThread* thread) {
                                name(), p2i(thread), BOOL_TO_STR(Thread::current()->is_VM_thread()), completion_time);
   }
 
-  // Use the semaphore to inform the VM thread that we have completed the operation
-  _done.signal();
+  // 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 VMThread/Handshaker after it reads
+  // that the operation has completed.
+  Atomic::dec(&_pending_threads, memory_order_release);
 
-  // It is no longer safe to refer to 'this' as the VMThread may have destroyed this operation
+  // It is no longer safe to refer to 'this' as the VMThread/Handshaker may have destroyed this operation
 }
 
 void Handshake::execute(HandshakeClosure* thread_cl) {
-  HandshakeThreadsOperation cto(thread_cl);
+  HandshakeOperation cto(thread_cl);
   VM_HandshakeAllThreads handshake(&cto);
   VMThread::execute(&handshake);
 }
 
 bool Handshake::execute(HandshakeClosure* thread_cl, JavaThread* target) {
-  HandshakeThreadsOperation cto(thread_cl);
+  HandshakeOperation cto(thread_cl);
   VM_HandshakeOneThread handshake(&cto, target);
   VMThread::execute(&handshake);
   return handshake.executed();
 }
 
-HandshakeState::HandshakeState() : _operation(NULL), _semaphore(1), _thread_in_process_handshake(false) {
-  DEBUG_ONLY(_vmthread_processing_handshake = false;)
+bool Handshake::execute_direct(HandshakeClosure* thread_cl, JavaThread* target) {
+  JavaThread* self = JavaThread::current();
+  HandshakeOperation op(thread_cl, /*is_direct*/ true);
+
+  jlong start_time_ns = 0;
+  if (log_is_enabled(Info, handshake)) {
+    start_time_ns = os::javaTimeNanos();
+  }
+
+  ThreadsListHandle tlh;
+  if (tlh.includes(target)) {
+    target->set_handshake_operation(&op);
+  } else {
+    log_handshake_info(start_time_ns, op.name(), 0, 0, "(thread dead)");
+    return false;
+  }
+
+  bool by_handshaker = false;
+  while (!op.is_completed()) {
+    by_handshaker = target->handshake_try_process(&op);
+    // Check for pending handshakes to avoid possible deadlocks where our
+    // target is trying to handshake us.
+    if (SafepointMechanism::should_block(self)) {
+      ThreadBlockInVM tbivm(self);
+    }
+  }
+
+  // This pairs up with the release store in do_handshake(). It prevents future
+  // loads from floating above the load of _pending_threads in is_completed()
+  // and thus prevents reading stale data modified in the handshake closure
+  // by the Handshakee.
+  OrderAccess::acquire();
+
+  log_handshake_info(start_time_ns, op.name(), 1, by_handshaker ? 1 : 0);
+
+  return op.executed();
 }
 
-void HandshakeState::set_operation(JavaThread* target, HandshakeOperation* op) {
-  _operation = op;
-  SafepointMechanism::arm_local_poll_release(target);
+HandshakeState::HandshakeState() :
+  _operation(NULL),
+  _operation_direct(NULL),
+  _handshake_turn_sem(1),
+  _processing_sem(1),
+  _thread_in_process_handshake(false)
+{
+  DEBUG_ONLY(_active_handshaker = NULL;)
 }
 
-void HandshakeState::clear_handshake(JavaThread* target) {
-  _operation = NULL;
-  SafepointMechanism::disarm_if_needed(target, true /* release */);
+void HandshakeState::set_operation(HandshakeOperation* op) {
+  if (!op->is_direct()) {
+    assert(Thread::current()->is_VM_thread(), "should be the VMThread");
+    _operation = op;
+  } else {
+    assert(Thread::current()->is_Java_thread(), "should be a JavaThread");
+    // Serialize direct handshakes so that only one proceeds at a time for a given target
+    _handshake_turn_sem.wait_with_safepoint_check(JavaThread::current());
+    _operation_direct = op;
+  }
+  SafepointMechanism::arm_local_poll_release(_handshakee);
 }
 
-void HandshakeState::process_self_inner(JavaThread* thread) {
-  assert(Thread::current() == thread, "should call from thread");
-  assert(!thread->is_terminated(), "should not be a terminated thread");
-  assert(thread->thread_state() != _thread_blocked, "should not be in a blocked state");
-  assert(thread->thread_state() != _thread_in_native, "should not be in native");
+void HandshakeState::clear_handshake(bool is_direct) {
+  if (!is_direct) {
+    _operation = NULL;
+  } else {
+    _operation_direct = NULL;
+    _handshake_turn_sem.signal();
+  }
+}
+
+void HandshakeState::process_self_inner() {
+  assert(Thread::current() == _handshakee, "should call from _handshakee");
+  assert(!_handshakee->is_terminated(), "should not be a terminated thread");
+  assert(_handshakee->thread_state() != _thread_blocked, "should not be in a blocked state");
+  assert(_handshakee->thread_state() != _thread_in_native, "should not be in native");
+  JavaThread* self = _handshakee;
 
   do {
-    ThreadInVMForHandshake tivm(thread);
-    if (!_semaphore.trywait()) {
-      _semaphore.wait_with_safepoint_check(thread);
+    ThreadInVMForHandshake tivm(self);
+    if (!_processing_sem.trywait()) {
+      _processing_sem.wait_with_safepoint_check(self);
     }
-    HandshakeOperation* op = Atomic::load_acquire(&_operation);
-    if (op != NULL) {
-      HandleMark hm(thread);
-      CautiouslyPreserveExceptionMark pem(thread);
-      // Disarm before execute the operation
-      clear_handshake(thread);
-      op->do_handshake(thread);
+    if (has_operation()) {
+      HandleMark hm(self);
+      CautiouslyPreserveExceptionMark pem(self);
+      HandshakeOperation * op = _operation;
+      if (op != NULL) {
+        // Disarm before executing the operation
+        clear_handshake(/*is_direct*/ false);
+        op->do_handshake(self);
+      }
+      op = _operation_direct;
+      if (op != NULL) {
+        // Disarm before executing the operation
+        clear_handshake(/*is_direct*/ true);
+        op->do_handshake(self);
+      }
     }
-    _semaphore.signal();
+    _processing_sem.signal();
   } while (has_operation());
 }
 
-bool HandshakeState::vmthread_can_process_handshake(JavaThread* target) {
+bool HandshakeState::can_process_handshake() {
   // handshake_safe may only be called with polls armed.
-  // VM thread controls this by first claiming the handshake via claim_handshake_for_vmthread.
-  return SafepointSynchronize::handshake_safe(target);
+  // Handshaker controls this by first claiming the handshake via claim_handshake().
+  return SafepointSynchronize::handshake_safe(_handshakee);
 }
 
-static bool possibly_vmthread_can_process_handshake(JavaThread* target) {
+bool HandshakeState::possibly_can_process_handshake() {
   // Note that this method is allowed to produce false positives.
-  if (target->is_ext_suspended()) {
+  if (_handshakee->is_ext_suspended()) {
     return true;
   }
-  if (target->is_terminated()) {
+  if (_handshakee->is_terminated()) {
     return true;
   }
-  switch (target->thread_state()) {
+  switch (_handshakee->thread_state()) {
   case _thread_in_native:
     // native threads are safe if they have no java stack or have walkable stack
-    return !target->has_last_Java_frame() || target->frame_anchor()->walkable();
+    return !_handshakee->has_last_Java_frame() || _handshakee->frame_anchor()->walkable();
 
   case _thread_blocked:
     return true;
@@ -319,32 +382,40 @@ static bool possibly_vmthread_can_process_handshake(JavaThread* target) {
   }
 }
 
-bool HandshakeState::claim_handshake_for_vmthread() {
-  if (!_semaphore.trywait()) {
+bool HandshakeState::claim_handshake(bool is_direct) {
+  if (!_processing_sem.trywait()) {
     return false;
   }
-  if (has_operation()) {
+  if (has_specific_operation(is_direct)){
     return true;
   }
-  _semaphore.signal();
+  _processing_sem.signal();
   return false;
 }
 
-bool HandshakeState::try_process_by_vmThread(JavaThread* target) {
-  assert(Thread::current()->is_VM_thread(), "should call from vm thread");
+bool HandshakeState::try_process(HandshakeOperation* op) {
+  bool is_direct = op->is_direct();
 
-  if (!has_operation()) {
+  if (!has_specific_operation(is_direct)){
     // JT has already cleared its handshake
     return false;
   }
 
-  if (!possibly_vmthread_can_process_handshake(target)) {
+  if (!possibly_can_process_handshake()) {
     // JT is observed in an unsafe state, it must notice the handshake itself
     return false;
   }
 
   // Claim the semaphore if there still an operation to be executed.
-  if (!claim_handshake_for_vmthread()) {
+  if (!claim_handshake(is_direct)) {
+    return false;
+  }
+
+  // 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) {
+    _processing_sem.signal();
     return false;
   }
 
@@ -352,19 +423,19 @@ bool HandshakeState::try_process_by_vmThread(JavaThread* target) {
   // can observe a safe state the thread cannot possibly continue without
   // getting caught by the semaphore.
   bool executed = false;
-  if (vmthread_can_process_handshake(target)) {
-    guarantee(!_semaphore.trywait(), "we should already own the semaphore");
-    log_trace(handshake)("Processing handshake by VMThtread");
-    DEBUG_ONLY(_vmthread_processing_handshake = true;)
-    _operation->do_handshake(target);
-    DEBUG_ONLY(_vmthread_processing_handshake = false;)
-    // Disarm after VM thread have executed the operation.
-    clear_handshake(target);
+  if (can_process_handshake()) {
+    guarantee(!_processing_sem.trywait(), "we should already own the semaphore");
+    log_trace(handshake)("Processing handshake by %s", Thread::current()->is_VM_thread() ? "VMThread" : "Handshaker");
+    DEBUG_ONLY(_active_handshaker = Thread::current();)
+    op->do_handshake(_handshakee);
+    DEBUG_ONLY(_active_handshaker = NULL;)
+    // Disarm after we have executed the operation.
+    clear_handshake(is_direct);
     executed = true;
   }
 
   // Release the thread
-  _semaphore.signal();
+  _processing_sem.signal();
 
   return executed;
 }

src/hotspot/share/runtime/handshake.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -30,13 +30,17 @@
 #include "runtime/flags/flagSetting.hpp"
 #include "runtime/semaphore.hpp"
 
+class HandshakeOperation;
 class JavaThread;
 
 // A handshake closure is a callback that is executed for each JavaThread
 // while that thread is in a safepoint safe state. The callback is executed
-// either by the thread itself or by the VM thread while keeping the thread
-// in a blocked state. A handshake can be performed with a single
-// JavaThread as well.
+// either by the target JavaThread itself or by the VMThread while keeping
+// the target thread in a blocked state. A handshake can be performed with a
+// single JavaThread as well. In that case, the callback is executed either
+// by the target JavaThread itself or, depending on whether the operation is
+// a direct handshake or not, by the JavaThread that requested the handshake
+// or the VMThread respectively.
 class HandshakeClosure : public ThreadClosure {
   const char* const _name;
  public:
@@ -52,47 +56,51 @@ class Handshake : public AllStatic {
   // Execution of handshake operation
   static void execute(HandshakeClosure* hs_cl);
   static bool execute(HandshakeClosure* hs_cl, JavaThread* target);
+  static bool execute_direct(HandshakeClosure* hs_cl, JavaThread* target);
 };
 
-class HandshakeOperation;
-
-// The HandshakeState keep tracks of an ongoing handshake for one JavaThread.
-// VM thread and JavaThread are serialized with the semaphore making sure
-// the operation is only done by either VM thread on behalf of the JavaThread
-// or the JavaThread itself.
+// The HandshakeState keeps track of an ongoing handshake for this JavaThread.
+// VMThread/Handshaker and JavaThread are serialized with semaphore _processing_sem
+// making sure the operation is only done by either VMThread/Handshaker on behalf
+// of the JavaThread or by the target JavaThread itself.
 class HandshakeState {
+  JavaThread* _handshakee;
   HandshakeOperation* volatile _operation;
+  HandshakeOperation* volatile _operation_direct;
 
-  Semaphore _semaphore;
+  Semaphore _handshake_turn_sem;  // Used to serialize direct handshakes for this JavaThread.
+  Semaphore _processing_sem;
   bool _thread_in_process_handshake;
 
-  bool claim_handshake_for_vmthread();
-  bool vmthread_can_process_handshake(JavaThread* target);
+  bool claim_handshake(bool is_direct);
+  bool possibly_can_process_handshake();
+  bool can_process_handshake();
+  void clear_handshake(bool is_direct);
 
-  void clear_handshake(JavaThread* thread);
+  void process_self_inner();
 
-  void process_self_inner(JavaThread* thread);
 public:
   HandshakeState();
 
-  void set_operation(JavaThread* thread, HandshakeOperation* op);
+  void set_handshakee(JavaThread* thread) { _handshakee = thread; }
 
-  bool has_operation() const {
-    return _operation != NULL;
+  void set_operation(HandshakeOperation* op);
+  bool has_operation() const { return _operation != NULL || _operation_direct != NULL; }
+  bool has_specific_operation(bool is_direct) const {
+    return is_direct ? _operation_direct != NULL : _operation != NULL;
   }
 
-  void process_by_self(JavaThread* thread) {
+  void process_by_self() {
     if (!_thread_in_process_handshake) {
       FlagSetting fs(_thread_in_process_handshake, true);
-      process_self_inner(thread);
+      process_self_inner();
     }
   }
-
-  bool try_process_by_vmThread(JavaThread* target);
+  bool try_process(HandshakeOperation* op);
 
 #ifdef ASSERT
-  bool _vmthread_processing_handshake;
-  bool is_vmthread_processing_handshake() const { return _vmthread_processing_handshake; }
+  Thread* _active_handshaker;
+  Thread* active_handshaker() const { return _active_handshaker; }
 #endif
 
 };

src/hotspot/share/runtime/mutexLocker.cpp

@@ -188,7 +188,7 @@ void assert_lock_strong(const Mutex* lock) {
 }
 
 void assert_locked_or_safepoint_or_handshake(const Mutex* lock, const JavaThread* thread) {
-  if (Thread::current()->is_VM_thread() && thread->is_vmthread_processing_handshake()) return;
+  if (Thread::current() == thread->active_handshaker()) return;
   assert_locked_or_safepoint(lock);
 }
 #endif

src/hotspot/share/runtime/safepoint.cpp

@@ -463,8 +463,6 @@ void SafepointSynchronize::disarm_safepoint() {
       assert(!cur_state->is_running(), "Thread not suspended at safepoint");
       cur_state->restart(); // TSS _running
       assert(cur_state->is_running(), "safepoint state has not been reset");
-
-      SafepointMechanism::disarm_if_needed(current, false /* NO release */);
     }
   } // ~JavaThreadIteratorWithHandle
 
@@ -705,7 +703,6 @@ static bool safepoint_safe_with(JavaThread *thread, JavaThreadState state) {
 }
 
 bool SafepointSynchronize::handshake_safe(JavaThread *thread) {
-  assert(Thread::current()->is_VM_thread(), "Must be VMThread");
   if (thread->is_ext_suspended() || thread->is_terminated()) {
     return true;
   }

src/hotspot/share/runtime/safepointMechanism.inline.hpp

@@ -66,19 +66,6 @@ void SafepointMechanism::disarm_local_poll(JavaThread* thread) {
   thread->set_polling_page(poll_disarmed_value());
 }
 
-void SafepointMechanism::disarm_if_needed(JavaThread* thread, bool memory_order_release) {
-  JavaThreadState jts = thread->thread_state();
-  if (jts == _thread_in_native || jts == _thread_in_native_trans) {
-    // JavaThread will disarm itself and execute cross_modify_fence() before continuing
-    return;
-  }
-  if (memory_order_release) {
-    thread->set_polling_page_release(poll_disarmed_value());
-  } else {
-    thread->set_polling_page(poll_disarmed_value());
-  }
-}
-
 void SafepointMechanism::arm_local_poll_release(JavaThread* thread) {
   thread->set_polling_page_release(poll_armed_value());
 }

src/hotspot/share/runtime/thread.cpp

@@ -1690,6 +1690,7 @@ void JavaThread::initialize() {
   _SleepEvent = ParkEvent::Allocate(this);
   // Setup safepoint state info for this thread
   ThreadSafepointState::create(this);
+  _handshake.set_handshakee(this);
 
   debug_only(_java_call_counter = 0);
 
@@ -4465,12 +4466,21 @@ bool Threads::destroy_vm() {
   // exit_globals() will delete tty
   exit_globals();
 
-  // We are after VM_Exit::set_vm_exited() so we can't call
-  // thread->smr_delete() or we will block on the Threads_lock.
-  // Deleting the shutdown thread here is safe because another
-  // JavaThread cannot have an active ThreadsListHandle for
-  // this JavaThread.
-  delete thread;
+  // We are here after VM_Exit::set_vm_exited() so we can't call
+  // thread->smr_delete() or we will block on the Threads_lock. We
+  // must check that there are no active references to this thread
+  // before attempting to delete it. A thread could be waiting on
+  // _handshake_turn_sem trying to execute a direct handshake with
+  // this thread.
+  if (!ThreadsSMRSupport::is_a_protected_JavaThread(thread)) {
+    delete thread;
+  } else {
+    // Clear value for _thread_key in TLS to prevent, depending
+    // on pthreads implementation, possible execution of
+    // thread-specific destructor in infinite loop at thread
+    // exit.
+    Thread::clear_thread_current();
+  }
 
 #if INCLUDE_JVMCI
   if (JVMCICounterSize > 0) {

src/hotspot/share/runtime/thread.hpp

@@ -1348,7 +1348,7 @@ class JavaThread: public Thread {
   HandshakeState _handshake;
  public:
   void set_handshake_operation(HandshakeOperation* op) {
-    _handshake.set_operation(this, op);
+    _handshake.set_operation(op);
   }
 
   bool has_handshake() const {
@@ -1356,16 +1356,16 @@ class JavaThread: public Thread {
   }
 
   void handshake_process_by_self() {
-    _handshake.process_by_self(this);
+    _handshake.process_by_self();
   }
 
-  bool handshake_try_process_by_vmThread() {
-    return _handshake.try_process_by_vmThread(this);
+  bool handshake_try_process(HandshakeOperation* op) {
+    return _handshake.try_process(op);
   }
 
 #ifdef ASSERT
-  bool is_vmthread_processing_handshake() const {
-    return _handshake.is_vmthread_processing_handshake();
+  Thread* active_handshaker() const {
+    return _handshake.active_handshaker();
   }
 #endif
 

src/hotspot/share/runtime/threadSMR.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -129,7 +129,6 @@ class ThreadsSMRSupport : AllStatic {
   static void inc_deleted_thread_cnt();
   static void inc_java_thread_list_alloc_cnt();
   static void inc_tlh_cnt();
-  static bool is_a_protected_JavaThread(JavaThread *thread);
   static void release_stable_list_wake_up(bool is_nested);
   static void set_delete_notify();
   static void threads_do(ThreadClosure *tc);
@@ -143,6 +142,7 @@ class ThreadsSMRSupport : AllStatic {
  public:
   static void add_thread(JavaThread *thread);
   static ThreadsList* get_java_thread_list();
+  static bool is_a_protected_JavaThread(JavaThread *thread);
   static bool is_a_protected_JavaThread_with_lock(JavaThread *thread);
   static bool is_bootstrap_list(ThreadsList* list);
   static void remove_thread(JavaThread *thread);

test/hotspot/jtreg/runtime/handshake/HandshakeDirectTest.java

@@ -0,0 +1,132 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+/*
+ * @test HandshakeDirectTest
+ * @bug 8240918
+ * @summary This test tries to stress direct handshakes between threads while suspending them.
+ * @library /testlibrary /test/lib
+ * @build HandshakeDirectTest
+ * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:+UseBiasedLocking -XX:+SafepointALot -XX:BiasedLockingDecayTime=100000000 -XX:BiasedLockingBulkRebiasThreshold=1000000 -XX:BiasedLockingBulkRevokeThreshold=1000000 HandshakeDirectTest
+ * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:+UseBiasedLocking -XX:GuaranteedSafepointInterval=10 -XX:+HandshakeALot -XX:+SafepointALot -XX:BiasedLockingDecayTime=100000000 -XX:BiasedLockingBulkRebiasThreshold=1000000 -XX:BiasedLockingBulkRevokeThreshold=1000000 HandshakeDirectTest
+ */
+
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.concurrent.Semaphore;
+import java.io.*;
+
+public class HandshakeDirectTest  implements Runnable {
+    static final int WORKING_THREADS = 32;
+    static final int DIRECT_HANDSHAKES_MARK = 50000;
+    static Thread[] workingThreads = new Thread[WORKING_THREADS];
+    static Semaphore[] handshakeSem = new Semaphore[WORKING_THREADS];
+    static Object[] locks = new Object[WORKING_THREADS];
+    static boolean[] isBiased = new boolean[WORKING_THREADS];
+    static AtomicInteger handshakeCount = new AtomicInteger(0);
+
+    @Override
+    public void run() {
+        int me = Integer.parseInt(Thread.currentThread().getName());
+
+        while (true) {
+            try {
+                if (!isBiased[me]) {
+                    handshakeSem[me].acquire();
+                    synchronized(locks[me]) {
+                        isBiased[me] = true;
+                    }
+                    handshakeSem[me].release();
+                }
+
+                // Handshake directly some other worker
+                int handshakee = ThreadLocalRandom.current().nextInt(0, WORKING_THREADS - 1);
+                if (handshakee == me) {
+                    // Pick another thread instead of me.
+                    handshakee = handshakee != 0 ? handshakee - 1 : handshakee + 1;
+                }
+                handshakeSem[handshakee].acquire();
+                if (isBiased[handshakee]) {
+                    // Revoke biased lock
+                    synchronized(locks[handshakee]) {
+                        handshakeCount.incrementAndGet();
+                    }
+                    // Create new lock to be biased
+                    locks[handshakee] = new Object();
+                    isBiased[handshakee] = false;
+                }
+                handshakeSem[handshakee].release();
+                if (handshakeCount.get() >= DIRECT_HANDSHAKES_MARK) {
+                    break;
+                }
+            } catch(InterruptedException ie) {
+                throw new Error("Unexpected interrupt");
+            }
+        }
+    }
+
+    public static void main(String... args) throws Exception {
+        HandshakeDirectTest test = new HandshakeDirectTest();
+
+        // Initialize semaphores
+        for (int i = 0; i < WORKING_THREADS; i++) {
+            handshakeSem[i] = new Semaphore(1);
+        }
+
+        // Initialize locks
+        for (int i = 0; i < WORKING_THREADS; i++) {
+            locks[i] = new Object();
+        }
+
+        // Fire-up working threads.
+        for (int i = 0; i < WORKING_THREADS; i++) {
+            workingThreads[i] = new Thread(test, Integer.toString(i));
+            workingThreads[i].start();
+        }
+
+        // Fire-up suspend-resume thread
+        Thread suspendResumeThread = new Thread() {
+            @Override
+            public void run() {
+                while (true) {
+                    int i = ThreadLocalRandom.current().nextInt(0, WORKING_THREADS - 1);
+                    workingThreads[i].suspend();
+                    try {
+                        Thread.sleep(1); // sleep for 1 ms
+                    } catch(InterruptedException ie) {
+                    }
+                    workingThreads[i].resume();
+                }
+            }
+        };
+        suspendResumeThread.setDaemon(true);
+        suspendResumeThread.start();
+
+        // Wait until the desired number of direct handshakes is reached
+        // and check that all workers exited
+        for (int i = 0; i < WORKING_THREADS; i++) {
+            workingThreads[i].join();
+        }
+    }
+}