8272526: Cleanup ThreadStateTransition class

Reviewed-by: dholmes, rehn, coleenp

src/hotspot/share/prims/jni.cpp

@@ -3658,7 +3658,7 @@ static jint JNI_CreateJavaVM_inner(JavaVM **vm, void **penv, void *args) {
 #endif
 
     // Since this is not a JVM_ENTRY we have to set the thread state manually before leaving.
-    ThreadStateTransition::transition(thread, _thread_in_vm, _thread_in_native);
+    ThreadStateTransition::transition_from_vm(thread, _thread_in_native);
     MACOS_AARCH64_ONLY(thread->enable_wx(WXExec));
   } else {
     // If create_vm exits because of a pending exception, exit with that
@@ -3878,11 +3878,8 @@ static jint attach_current_thread(JavaVM *vm, void **penv, void *_args, bool dae
   *(JNIEnv**)penv = thread->jni_environment();
 
   // Now leaving the VM, so change thread_state. This is normally automatically taken care
-  // of in the JVM_ENTRY. But in this situation we have to do it manually. Notice, that by
-  // using ThreadStateTransition::transition, we do a callback to the safepoint code if
-  // needed.
-
-  ThreadStateTransition::transition(thread, _thread_in_vm, _thread_in_native);
+  // of in the JVM_ENTRY. But in this situation we have to do it manually.
+  ThreadStateTransition::transition_from_vm(thread, _thread_in_native);
   MACOS_AARCH64_ONLY(thread->enable_wx(WXExec));
 
   // Perform any platform dependent FPU setup

src/hotspot/share/prims/jvmtiExport.cpp

@@ -117,7 +117,7 @@ class JvmtiThreadEventTransition : StackObj {
        if (_saved_state == _thread_in_Java) {
          ThreadStateTransition::transition_from_java(_jthread, _thread_in_native);
        } else {
-         ThreadStateTransition::transition(_jthread, _saved_state, _thread_in_native);
+         ThreadStateTransition::transition_from_vm(_jthread, _thread_in_native);
        }
     } else {
       _jthread = NULL;

src/hotspot/share/prims/universalUpcallHandler.cpp

@@ -83,20 +83,12 @@ JavaThread* ProgrammableUpcallHandler::on_entry(OptimizedEntryBlob::FrameData* c
   // since it can potentially block.
   context->new_handles = JNIHandleBlock::allocate_block(thread);
 
+  // clear any pending exception in thread (native calls start with no exception pending)
+  thread->clear_pending_exception();
+
   // After this, we are officially in Java Code. This needs to be done before we change any of the thread local
   // info, since we cannot find oops before the new information is set up completely.
-  ThreadStateTransition::transition_from_native(thread, _thread_in_Java);
-
-  // Make sure that we handle asynchronous stops and suspends _before_ we clear all thread state
-  // in OptimizedEntryBlob::FrameData. This way, we can decide if we need to do any pd actions
-  // to prepare for stop/suspend (cache sp, or other state).
-  bool clear_pending_exception = true;
-  if (thread->has_special_runtime_exit_condition()) {
-    thread->handle_special_runtime_exit_condition();
-    if (thread->has_pending_exception()) {
-      clear_pending_exception = false;
-    }
-  }
+  ThreadStateTransition::transition_from_native(thread, _thread_in_Java, true /* check_asyncs */);
 
   context->old_handles = thread->active_handles();
 
@@ -111,11 +103,6 @@ JavaThread* ProgrammableUpcallHandler::on_entry(OptimizedEntryBlob::FrameData* c
   debug_only(thread->inc_java_call_counter());
   thread->set_active_handles(context->new_handles);     // install new handle block and reset Java frame linkage
 
-  // clear any pending exception in thread (native calls start with no exception pending)
-  if (clear_pending_exception) {
-    thread->clear_pending_exception();
-  }
-
   MACOS_AARCH64_ONLY(thread->enable_wx(WXExec));
 
   return thread;

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

@@ -76,59 +76,54 @@ class InterfaceSupport: AllStatic {
 class ThreadStateTransition : public StackObj {
  protected:
   JavaThread* _thread;
- public:
-  ThreadStateTransition(JavaThread *thread) {
-    _thread = thread;
-    assert(thread != NULL, "must be active Java thread");
-    assert(thread == Thread::current(), "must be current thread");
-  }
 
-  // Change threadstate in a manner, so safepoint can detect changes.
-  // Time-critical: called on exit from every runtime routine
-  static inline void transition(JavaThread *thread, JavaThreadState from, JavaThreadState to) {
-    assert(from != _thread_in_Java, "use transition_from_java");
-    assert(from != _thread_in_native, "use transition_from_native");
-    assert((from & 1) == 0 && (to & 1) == 0, "odd numbers are transitions states");
-    assert(thread->thread_state() == from, "coming from wrong thread state");
-
-    // Check NoSafepointVerifier
-    // This also clears unhandled oops if CheckUnhandledOops is used.
+ private:
+  static inline void transition_and_process(JavaThread *thread, JavaThreadState to, bool check_asyncs) {
+    // Check NoSafepointVerifier. This also clears unhandled oops if CheckUnhandledOops is used.
     thread->check_possible_safepoint();
 
-    // Change to transition state and ensure it is seen by the VM thread.
-    thread->set_thread_state_fence((JavaThreadState)(from + 1));
-
-    SafepointMechanism::process_if_requested(thread);
+    thread->set_thread_state_fence(_thread_in_vm);
+    SafepointMechanism::process_if_requested_with_exit_check(thread, check_asyncs);
     thread->set_thread_state(to);
   }
 
-  // Same as above, but assumes from = _thread_in_Java. This is simpler, since we
-  // never block on entry to the VM. This will break the code, since e.g. preserve arguments
-  // have not been setup.
+ public:
+  ThreadStateTransition(JavaThread *thread) : _thread(thread) {
+    assert(thread != NULL, "must be active Java thread");
+    assert(thread == Thread::current(), "must be current thread");
+  }
+
   static inline void transition_from_java(JavaThread *thread, JavaThreadState to) {
     assert(thread->thread_state() == _thread_in_Java, "coming from wrong thread state");
+    assert(to == _thread_in_vm || to == _thread_in_native, "invalid transition");
     thread->set_thread_state(to);
   }
 
-  static inline void transition_from_native(JavaThread *thread, JavaThreadState to) {
-    assert((to & 1) == 0, "odd numbers are transitions states");
+  // We never install asynchronous exceptions when coming (back) in to the runtime
+  // from native code because the runtime is not set up to handle exceptions floating
+  // around at arbitrary points.
+  static inline void transition_from_native(JavaThread *thread, JavaThreadState to, bool check_asyncs = true) {
     assert(thread->thread_state() == _thread_in_native, "coming from wrong thread state");
-    assert(!thread->has_last_Java_frame() || thread->frame_anchor()->walkable(), "Unwalkable stack in native->vm transition");
-
-    // Change to transition state and ensure it is seen by the VM thread.
-    thread->set_thread_state_fence(_thread_in_native_trans);
-
-    // We never install asynchronous exceptions when coming (back) in
-    // to the runtime from native code because the runtime is not set
-    // up to handle exceptions floating around at arbitrary points.
-    SafepointMechanism::process_if_requested_with_exit_check(thread, false /* check asyncs */);
-    thread->set_thread_state(to);
+    assert(to == _thread_in_vm || to == _thread_in_Java, "invalid transition");
+    assert(!thread->has_last_Java_frame() || thread->frame_anchor()->walkable(), "Unwalkable stack in native transition");
+    transition_and_process(thread, to, to != _thread_in_Java ? false : check_asyncs);
   }
 
- protected:
-   void trans(JavaThreadState from, JavaThreadState to)  { transition(_thread, from, to); }
-   void trans_from_java(JavaThreadState to)              { transition_from_java(_thread, to); }
-   void trans_from_native(JavaThreadState to)            { transition_from_native(_thread, to); }
+  static inline void transition_from_vm(JavaThread *thread, JavaThreadState to, bool check_asyncs = true) {
+    assert(thread->thread_state() == _thread_in_vm, "coming from wrong thread state");
+    if (to == _thread_in_Java) {
+      transition_and_process(thread, _thread_in_Java, check_asyncs);
+    } else {
+      assert(to == _thread_in_native || to == _thread_blocked, "invalid transition");
+      // Check NoSafepointVerifier. This also clears unhandled oops if CheckUnhandledOops is used.
+      thread->check_possible_safepoint();
+
+      // Once we are in native/blocked vm expects stack to be walkable
+      thread->frame_anchor()->make_walkable(thread);
+      OrderAccess::storestore(); // Keep thread_state change and make_walkable() separate.
+      thread->set_thread_state(to);
+    }
+  }
 };
 
 class ThreadInVMForHandshake : public ThreadStateTransition {
@@ -158,16 +153,15 @@ class ThreadInVMfromJava : public ThreadStateTransition {
   bool _check_asyncs;
  public:
   ThreadInVMfromJava(JavaThread* thread, bool check_asyncs = true) : ThreadStateTransition(thread), _check_asyncs(check_asyncs) {
-    trans_from_java(_thread_in_vm);
+    transition_from_java(thread, _thread_in_vm);
   }
   ~ThreadInVMfromJava()  {
     if (_thread->stack_overflow_state()->stack_yellow_reserved_zone_disabled()) {
       _thread->stack_overflow_state()->enable_stack_yellow_reserved_zone();
     }
-    trans(_thread_in_vm, _thread_in_Java);
     // We prevent asynchronous exceptions from being installed on return to Java in situations
     // where we can't tolerate them. See bugs: 4324348, 4854693, 4998314, 5040492, 5050705.
-    if (_thread->has_special_runtime_exit_condition()) _thread->handle_special_runtime_exit_condition(_check_asyncs);
+    transition_from_vm(_thread, _thread_in_Java, _check_asyncs);
   }
 };
 
@@ -191,7 +185,7 @@ class ThreadInVMfromUnknown {
   }
   ~ThreadInVMfromUnknown()  {
     if (_thread) {
-      ThreadStateTransition::transition(_thread, _thread_in_vm, _thread_in_native);
+      ThreadStateTransition::transition_from_vm(_thread, _thread_in_native);
     }
   }
 };
@@ -201,35 +195,24 @@ class ThreadInVMfromNative : public ThreadStateTransition {
   ResetNoHandleMark __rnhm;
  public:
   ThreadInVMfromNative(JavaThread* thread) : ThreadStateTransition(thread) {
-    trans_from_native(_thread_in_vm);
+    transition_from_native(thread, _thread_in_vm);
   }
   ~ThreadInVMfromNative() {
-    assert(_thread->thread_state() == _thread_in_vm, "coming from wrong thread state");
     // We cannot assert !_thread->owns_locks() since we have valid cases where
     // we call known native code using this wrapper holding locks.
-    _thread->check_possible_safepoint();
-    // Once we are in native vm expects stack to be walkable
-    _thread->frame_anchor()->make_walkable(_thread);
-    OrderAccess::storestore(); // Keep thread_state change and make_walkable() separate.
-    _thread->set_thread_state(_thread_in_native);
+    transition_from_vm(_thread, _thread_in_native);
   }
 };
 
 
 class ThreadToNativeFromVM : public ThreadStateTransition {
  public:
   ThreadToNativeFromVM(JavaThread *thread) : ThreadStateTransition(thread) {
-    // We are leaving the VM at this point and going directly to native code.
-    // Block, if we are in the middle of a safepoint synchronization.
     assert(!thread->owns_locks(), "must release all locks when leaving VM");
-    thread->frame_anchor()->make_walkable(thread);
-    trans(_thread_in_vm, _thread_in_native);
-    // Check for pending. async. exceptions or suspends.
-    if (_thread->has_special_runtime_exit_condition()) _thread->handle_special_runtime_exit_condition(false);
+    transition_from_vm(thread, _thread_in_native);
   }
-
   ~ThreadToNativeFromVM() {
-    trans_from_native(_thread_in_vm);
+    transition_from_native(_thread, _thread_in_vm);
     assert(!_thread->is_pending_jni_exception_check(), "Pending JNI Exception Check");
     // We don't need to clear_walkable because it will happen automagically when we return to java
   }
@@ -247,12 +230,7 @@ class ThreadBlockInVMPreprocess : public ThreadStateTransition {
  public:
   ThreadBlockInVMPreprocess(JavaThread* thread, PRE_PROC& pr = emptyOp, bool allow_suspend = false)
     : ThreadStateTransition(thread), _pr(pr), _allow_suspend(allow_suspend) {
-    assert(thread->thread_state() == _thread_in_vm, "coming from wrong thread state");
-    thread->check_possible_safepoint();
-    // Once we are blocked vm expects stack to be walkable
-    thread->frame_anchor()->make_walkable(thread);
-    OrderAccess::storestore(); // Keep thread_state change and make_walkable() separate.
-    thread->set_thread_state(_thread_blocked);
+    transition_from_vm(thread, _thread_blocked);
   }
   ~ThreadBlockInVMPreprocess() {
     assert(_thread->thread_state() == _thread_blocked, "coming from wrong thread state");

src/hotspot/share/runtime/javaCalls.cpp

@@ -54,7 +54,6 @@
 
 JavaCallWrapper::JavaCallWrapper(const methodHandle& callee_method, Handle receiver, JavaValue* result, TRAPS) {
   JavaThread* thread = THREAD;
-  bool clear_pending_exception = true;
 
   guarantee(thread->is_Java_thread(), "crucial check - the VM thread cannot and must not escape to Java code");
   assert(!thread->owns_locks(), "must release all locks when leaving VM");
@@ -65,19 +64,12 @@ JavaCallWrapper::JavaCallWrapper(const methodHandle& callee_method, Handle recei
   // since it can potentially block.
   JNIHandleBlock* new_handles = JNIHandleBlock::allocate_block(thread);
 
+  // clear any pending exception in thread (native calls start with no exception pending)
+  thread->clear_pending_exception();
+
   // After this, we are official in JavaCode. This needs to be done before we change any of the thread local
   // info, since we cannot find oops before the new information is set up completely.
-  ThreadStateTransition::transition(thread, _thread_in_vm, _thread_in_Java);
-
-  // Make sure that we handle asynchronous stops and suspends _before_ we clear all thread state
-  // in JavaCallWrapper::JavaCallWrapper(). This way, we can decide if we need to do any pd actions
-  // to prepare for stop/suspend (flush register windows on sparcs, cache sp, or other state).
-  if (thread->has_special_runtime_exit_condition()) {
-    thread->handle_special_runtime_exit_condition();
-    if (HAS_PENDING_EXCEPTION) {
-      clear_pending_exception = false;
-    }
-  }
+  ThreadStateTransition::transition_from_vm(thread, _thread_in_Java, true /* check_asyncs */);
 
   // Make sure to set the oop's after the thread transition - since we can block there. No one is GC'ing
   // the JavaCallWrapper before the entry frame is on the stack.
@@ -104,11 +96,6 @@ JavaCallWrapper::JavaCallWrapper(const methodHandle& callee_method, Handle recei
 
   assert (_thread->thread_state() != _thread_in_native, "cannot set native pc to NULL");
 
-  // clear any pending exception in thread (native calls start with no exception pending)
-  if(clear_pending_exception) {
-    _thread->clear_pending_exception();
-  }
-
   MACOS_AARCH64_ONLY(_thread->enable_wx(WXExec));
 }
 

src/hotspot/share/runtime/safepoint.hpp

@@ -129,11 +129,10 @@ class SafepointSynchronize : AllStatic {
   static bool is_a_block_safe_state(JavaThreadState state) {
     // Check that we have a valid thread_state before blocking for safepoints
     switch(state) {
-      case _thread_in_vm_trans:
+      case _thread_in_vm:
       case _thread_in_Java:        // From compiled code
       case _thread_in_native_trans:
       case _thread_blocked_trans:
-      case _thread_new_trans:
         return true;
       default:
         return false;

src/hotspot/share/runtime/thread.cpp

@@ -1227,7 +1227,9 @@ void JavaThread::run() {
 
   // Thread is now sufficiently initialized to be handled by the safepoint code as being
   // in the VM. Change thread state from _thread_new to _thread_in_vm
-  ThreadStateTransition::transition(this, _thread_new, _thread_in_vm);
+  assert(this->thread_state() == _thread_new, "wrong thread state");
+  set_thread_state(_thread_in_vm);
+
   // Before a thread is on the threads list it is always safe, so after leaving the
   // _thread_new we should emit a instruction barrier. The distance to modified code
   // from here is probably far enough, but this is consistent and safe.
@@ -1644,7 +1646,7 @@ void JavaThread::check_and_handle_async_exceptions() {
     case _thread_in_Java: {
       ThreadInVMfromJava tiv(this);
       JavaThread* THREAD = this;
-      Exceptions::throw_unsafe_access_internal_error(THREAD, __FILE__, __LINE__, "a fault occurred in a recent unsafe memory access operation in compiled Java code");
+      Exceptions::throw_unsafe_access_internal_error(THREAD, __FILE__, __LINE__, "a fault occurred in an unsafe memory access operation in compiled Java code");
       return;
     }
     default:

test/hotspot/jtreg/runtime/Unsafe/InternalErrorTest.java

@@ -55,7 +55,7 @@ public class InternalErrorTest {
 
     private static final Unsafe unsafe = Unsafe.getUnsafe();
     private static final int pageSize = WhiteBox.getWhiteBox().getVMPageSize();
-    private static final String expectedErrorMsg = "fault occurred in a recent unsafe memory access";
+    private static final String expectedErrorMsg = "fault occurred in an unsafe memory access";
     private static final String failureMsg1 = "InternalError not thrown";
     private static final String failureMsg2 = "Wrong InternalError: ";