Refactor to handshake state and private

src/hotspot/share/prims/jvmtiRawMonitor.cpp

@@ -320,7 +320,7 @@ void JvmtiRawMonitor::raw_enter(Thread* self) {
       jt->handshake_state()->lock();
       // Suspend request flag can only be set in handshakes.
       // By blocking handshakes, suspend request flag cannot change its value.
-      if (!jt->is_suspend_requested()) {
+      if (!jt->handshake_state()->is_suspend_requested()) {
         contended = Atomic::cmpxchg(&_owner, (Thread*)NULL, jt);
         jt->handshake_state()->unlock();
         break;

src/hotspot/share/runtime/handshake.cpp

@@ -405,7 +405,9 @@ HandshakeState::HandshakeState(JavaThread* target) :
   _handshakee(target),
   _queue(),
   _lock(Monitor::leaf, "HandshakeState", Mutex::_allow_vm_block_flag, Monitor::_safepoint_check_never),
-  _active_handshaker()
+  _active_handshaker(),
+  _suspended(false),
+  _suspend_requested(false)
 {
 }
 
@@ -612,7 +614,7 @@ void HandshakeState::thread_exit() {
   while (true) {
     {
       MutexLocker ml(&_lock, Mutex::_no_safepoint_check_flag);
-      if (!_handshakee->is_suspend_requested()) {
+      if (!is_suspend_requested()) {
         _handshakee->set_exiting();
         // No more external suspends are allowed at this point.
         return;
@@ -630,37 +632,97 @@ bool HandshakeState::suspend_request_pending() {
   assert(JavaThread::current()->thread_state() != _thread_blocked, "should not be in a blocked state");
   assert(JavaThread::current()->thread_state() != _thread_in_native, "should not be in native");
   MutexLocker ml(&_lock, Mutex::_no_safepoint_check_flag);
-  return _handshakee->is_suspend_requested();
+  return is_suspend_requested();
 }
 
 void HandshakeState::suspend_in_handshake() {
   assert(Thread::current() == _handshakee, "should call from _handshakee");
   assert(_lock.owned_by_self(), "Lock must be held");
   assert(!_handshakee->has_last_Java_frame() || _handshakee->frame_anchor()->walkable(), "should have walkable stack");
   JavaThreadState jts = _handshakee->thread_state();
-  while (_handshakee->_suspended) {
+  while (is_suspended()) {
     _handshakee->set_thread_state(_thread_blocked);
     log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " suspended", p2i(_handshakee));
     _lock.wait_without_safepoint_check();
   }
   _handshakee->set_thread_state(jts);
-  _handshakee->set_suspend_requested(false);
+  set_suspend_requested(false);
   log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " resumed", p2i(_handshakee));
 }
 
+// This is the closure that prevents a suspended JavaThread from
+// escaping the suspend request.
+class ThreadSuspensionHandshake : public AsyncHandshakeClosure {
+ public:
+  ThreadSuspensionHandshake() : AsyncHandshakeClosure("ThreadSuspension") {}
+  void do_thread(Thread* thr) {
+    JavaThread* target = thr->as_Java_thread();
+    target->handshake_state()->suspend_in_handshake();
+  }
+};
+
+bool HandshakeState::handshake_suspend() {
+  if (_handshakee->is_exiting() ||
+     _handshakee->threadObj() == NULL) {
+    log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " exiting", p2i(_handshakee));
+    return false;
+  }
+  if (is_suspend_requested()) {
+    if (is_suspended()) {
+      // Target is already suspended.
+      log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " already suspended", p2i(_handshakee));
+      return false;
+    } else {
+      // Target is going to wake up and leave suspension.
+      // Let's just stop the thread from doing that.
+      log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " re-suspended", p2i(_handshakee));
+      set_suspend(true);
+      return true;
+    }
+  }
+  // no suspend request
+  assert(!is_suspended(), "cannot be suspended without a suspend request");
+  // Thread is safe, so it must execute the request, thus we can count it as suspended
+  // from this point.
+  set_suspend(true);
+  set_suspend_requested(true);
+  log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " suspended, arming ThreadSuspension", p2i(_handshakee));
+  ThreadSuspensionHandshake* ts = new ThreadSuspensionHandshake();
+  Handshake::execute(ts, _handshakee);
+  return true;
+}
+
+// This is the closure that synchronously honors the suspend request.
+class SuspendThreadHandshake : public HandshakeClosure {
+  bool _did_suspend;
+public:
+  SuspendThreadHandshake() : HandshakeClosure("SuspendThread"), _did_suspend(false) {}
+  void do_thread(Thread* thr) {
+    JavaThread* target = thr->as_Java_thread();
+    _did_suspend = target->handshake_state()->handshake_suspend();
+  }
+  bool did_suspend() { return _did_suspend; }
+};
+
+bool HandshakeState::suspend() {
+  SuspendThreadHandshake st;
+  Handshake::execute(&st, _handshakee);
+  return st.did_suspend();
+}
+
 bool HandshakeState::resume() {
   // Can't be suspended if there is no suspend request.
-  if (!_handshakee->is_suspend_requested()) {
+  if (!is_suspend_requested()) {
     return false;
   }
   MutexLocker ml(&_lock, Mutex::_no_safepoint_check_flag);
   // Can't be suspended if there is no suspend request.
-  if (!_handshakee->is_suspend_requested()) {
+  if (!is_suspend_requested()) {
     assert(!_handshakee->is_suspended(), "cannot be suspended without a suspend request");
     return false;
   }
   // Resume the thread.
-  _handshakee->set_suspend(false);
+  set_suspend(false);
   _lock.notify();
   return true;
 }

src/hotspot/share/runtime/handshake.hpp

@@ -33,6 +33,8 @@
 
 class HandshakeOperation;
 class JavaThread;
+class ThreadSuspensionHandshake;
+class SuspendThreadHandshake;
 
 // A handshake closure is a callback that is executed for a JavaThread
 // while it is in a safepoint/handshake-safe state. Depending on the
@@ -72,6 +74,9 @@ class JvmtiRawMonitor;
 // JavaThread or by the target JavaThread itself.
 class HandshakeState {
   friend JvmtiRawMonitor;
+  friend ThreadSuspensionHandshake;
+  friend SuspendThreadHandshake;
+  friend JavaThread;
   // This a back reference to the JavaThread,
   // the target for all operation in the queue.
   JavaThread* _handshakee;
@@ -132,12 +137,25 @@ class HandshakeState {
   Thread* active_handshaker() const { return _active_handshaker; }
 
   // Suspend/resume support
-  void thread_exit();
+ private:
+  volatile bool _suspended;
+  volatile bool _suspend_requested;
+
+  // Called from the suspend handshake.
+  bool handshake_suspend();
+  // Called from the async handshake (the trap)
+  // to stop a thread from continuing executing when suspended.
+  void suspend_in_handshake();
 
-  bool suspend_request_pending();
+  bool is_suspended()                 { return Atomic::load(&_suspended); }
+  void set_suspend(bool to)           { return Atomic::store(&_suspended, to); }
+  bool is_suspend_requested()         { return Atomic::load(&_suspend_requested); }
+  void set_suspend_requested(bool to) { return Atomic::store(&_suspend_requested, to); }
 
-  void suspend_in_handshake();
+  bool suspend();
   bool resume();
+  void thread_exit();
+  bool suspend_request_pending();
 };
 
 #endif // SHARE_RUNTIME_HANDSHAKE_HPP

src/hotspot/share/runtime/thread.cpp

@@ -1092,9 +1092,6 @@ JavaThread::JavaThread() :
 
   _handshake(this),
 
-  _suspended(false),
-  _suspend_requested(false),
-
   _popframe_preserved_args(nullptr),
   _popframe_preserved_args_size(0),
 
@@ -1773,57 +1770,6 @@ void JavaThread::send_thread_stop(oop java_throwable)  {
   this->interrupt();
 }
 
-// This is the closure that prevents a suspended JavaThread from
-// escaping the suspend request.
-class ThreadSuspensionHandshake : public AsyncHandshakeClosure {
- public:
-  ThreadSuspensionHandshake() : AsyncHandshakeClosure("ThreadSuspension") {}
-  void do_thread(Thread* thr) {
-    JavaThread* target = thr->as_Java_thread();
-    target->handshake_state()->suspend_in_handshake();
-  }
-};
-
-// This is the closure that synchronously honors the suspend request.
-class SuspendThreadHandshake : public HandshakeClosure {
-  bool _did_suspend;
-public:
-  SuspendThreadHandshake() : HandshakeClosure("SuspendThread"), _did_suspend(false) {}
-  void do_thread(Thread* thr) {
-    JavaThread* target = thr->as_Java_thread();
-    if (target->is_exiting() ||
-       target->threadObj() == NULL) {
-      log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " exiting", p2i(target));
-      return;
-    }
-    assert(java_lang_Thread::thread(target->threadObj()) != NULL, "BAD");
-    if (target->is_suspend_requested()) {
-      if (target->is_suspended()) {
-        // Target is already suspended.
-        log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " already suspended", p2i(target));
-        return;
-      } else {
-        // Target is going to wake up and leave suspension.
-        // Let's just stop the thread from doing that.
-        log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " re-suspended", p2i(target));
-        target->set_suspend(true);
-        _did_suspend = true;
-        return;
-      }
-    }
-    // no suspend request
-    assert(!target->is_suspended(), "cannot be suspended without a suspend request");
-    // Thread is safe, so it must execute the request, thus we can count it as suspended
-    // from this point.
-    target->set_suspend(true);
-    _did_suspend = true;
-    target->set_suspend_requested(true);
-    log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " suspended, arming ThreadSuspension", p2i(target));
-    ThreadSuspensionHandshake* ts = new ThreadSuspensionHandshake();
-    Handshake::execute(ts, target);
-  }
-  bool did_suspend() { return _did_suspend; }
-};
 
 // External suspension mechanism.
 //
@@ -1837,9 +1783,7 @@ bool JavaThread::java_suspend() {
     log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " not on ThreadsList, no suspension", p2i(this));
     return false;
   }
-  SuspendThreadHandshake st;
-  Handshake::execute(&st, this);
-  return st.did_suspend();
+  return this->handshake_state()->suspend();
 }
 
 bool JavaThread::java_resume() {

src/hotspot/share/runtime/thread.hpp

@@ -1121,20 +1121,9 @@ class JavaThread: public Thread {
   }
 
   // Suspend/resume support for JavaThread
- private:
-  volatile bool _suspended;
-  volatile bool _suspend_requested;
-
- public:
   bool java_suspend(); // higher-level suspension logic called by the public APIs
   bool java_resume();  // higher-level resume logic called by the public APIs
-
-  bool is_suspended()                 { return Atomic::load(&_suspended); }
-  void set_suspend(bool to)           { return Atomic::store(&_suspended, to); }
-
-  bool is_suspend_requested()         { return Atomic::load(&_suspend_requested); }
-  void set_suspend_requested(bool to) { return Atomic::store(&_suspend_requested, to); }
-
+  bool is_suspended()                 { return _handshake.is_suspended(); }
   bool suspend_request_pending()      { return _handshake.suspend_request_pending(); }
 
   static void check_safepoint_and_suspend_for_native_trans(JavaThread *thread);

src/hotspot/share/runtime/vmStructs.cpp

@@ -768,9 +768,6 @@ typedef HashtableEntry<InstanceKlass*, mtClass>  KlassHashtableEntry;
   /* JavaThread (NOTE: incomplete) */                                                                                                \
   /*********************************/                                                                                                \
                                                                                                                                      \
-  volatile_nonstatic_field(JavaThread,         _suspended,                                    bool)                                  \
-                                                                                                                                     \
-                                                                                                                                     \
   /************/                                                                                                                     \
   /* OSThread */                                                                                                                     \
   /************/                                                                                                                     \