8222231: Clean up interfaceSupport.inline.hpp duplicated code

Reviewed-by: dholmes, pchilanomate

src/hotspot/share/prims/jni.cpp

@@ -3971,7 +3971,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_and_fence(thread, _thread_in_vm, _thread_in_native);
+    ThreadStateTransition::transition(thread, _thread_in_vm, _thread_in_native);
   } else {
     // If create_vm exits because of a pending exception, exit with that
     // exception.  In the future when we figure out how to reclaim memory,
@@ -4073,7 +4073,7 @@ static jint JNICALL jni_DestroyJavaVM_inner(JavaVM *vm) {
     res = JNI_OK;
     return res;
   } else {
-    ThreadStateTransition::transition_and_fence(thread, _thread_in_vm, _thread_in_native);
+    ThreadStateTransition::transition(thread, _thread_in_vm, _thread_in_native);
     res = JNI_ERR;
     return res;
   }
@@ -4195,7 +4195,7 @@ static jint attach_current_thread(JavaVM *vm, void **penv, void *_args, bool dae
   // using ThreadStateTransition::transition, we do a callback to the safepoint code if
   // needed.
 
-  ThreadStateTransition::transition_and_fence(thread, _thread_in_vm, _thread_in_native);
+  ThreadStateTransition::transition(thread, _thread_in_vm, _thread_in_native);
 
   // Perform any platform dependent FPU setup
   os::setup_fpu();

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

@@ -66,22 +66,6 @@ class InterfaceSupport: AllStatic {
   static void verify_stack();
   static void verify_last_frame();
 # endif
-
- public:
-  static void serialize_thread_state_with_handler(JavaThread* thread) {
-    serialize_thread_state_internal(thread, true);
-  }
-
-  // Should only call this if we know that we have a proper SEH set up.
-  static void serialize_thread_state(JavaThread* thread) {
-    serialize_thread_state_internal(thread, false);
-  }
-
- private:
-  static void serialize_thread_state_internal(JavaThread* thread, bool needs_exception_handler) {
-    // Make sure new state is seen by VM thread
-    OrderAccess::fence();
-  }
 };
 
 
@@ -103,28 +87,8 @@ class ThreadStateTransition : public StackObj {
     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");
-    // Change to transition state
+    // Change to transition state and ensure it is seen by the VM thread.
-    thread->set_thread_state((JavaThreadState)(from + 1));
+    thread->set_thread_state_fence((JavaThreadState)(from + 1));
-
-    InterfaceSupport::serialize_thread_state(thread);
-
-    SafepointMechanism::block_if_requested(thread);
-    thread->set_thread_state(to);
-
-    CHECK_UNHANDLED_OOPS_ONLY(thread->clear_unhandled_oops();)
-  }
-
-  // transition_and_fence must be used on any thread state transition
-  // where there might not be a Java call stub on the stack, in
-  // particular on Windows where the Structured Exception Handler is
-  // set up in the call stub.
-  static inline void transition_and_fence(JavaThread *thread, JavaThreadState from, JavaThreadState to) {
-    assert(thread->thread_state() == from, "coming from wrong thread state");
-    assert((from & 1) == 0 && (to & 1) == 0, "odd numbers are transitions states");
-    // Change to transition state
-    thread->set_thread_state((JavaThreadState)(from + 1));
-
-    InterfaceSupport::serialize_thread_state_with_handler(thread);
 
     SafepointMechanism::block_if_requested(thread);
     thread->set_thread_state(to);
@@ -143,19 +107,14 @@ class ThreadStateTransition : public StackObj {
   static inline void transition_from_native(JavaThread *thread, JavaThreadState to) {
     assert((to & 1) == 0, "odd numbers are transitions states");
     assert(thread->thread_state() == _thread_in_native, "coming from wrong thread state");
-    // Change to transition state
+    // Change to transition state and ensure it is seen by the VM thread.
-    thread->set_thread_state(_thread_in_native_trans);
+    thread->set_thread_state_fence(_thread_in_native_trans);
-
-    InterfaceSupport::serialize_thread_state_with_handler(thread);
 
     // 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.
     if (SafepointMechanism::should_block(thread) || thread->is_suspend_after_native()) {
       JavaThread::check_safepoint_and_suspend_for_native_trans(thread);
-
-      // Clear unhandled oops anywhere where we could block, even if we don't.
-      CHECK_UNHANDLED_OOPS_ONLY(thread->clear_unhandled_oops();)
     }
 
     thread->set_thread_state(to);
@@ -164,7 +123,6 @@ class ThreadStateTransition : public StackObj {
    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); }
-   void trans_and_fence(JavaThreadState from, JavaThreadState to) { transition_and_fence(_thread, from, to); }
 };
 
 class ThreadInVMForHandshake : public ThreadStateTransition {
@@ -173,9 +131,8 @@ class ThreadInVMForHandshake : public ThreadStateTransition {
   void transition_back() {
     // This can be invoked from transition states and must return to the original state properly
     assert(_thread->thread_state() == _thread_in_vm, "should only call when leaving VM after handshake");
-    _thread->set_thread_state(_thread_in_vm_trans);
+    // Change to transition state and ensure it is seen by the VM thread.
-
+    _thread->set_thread_state_fence(_thread_in_vm_trans);
-    InterfaceSupport::serialize_thread_state(_thread);
 
     SafepointMechanism::block_if_requested(_thread);
 
@@ -217,7 +174,6 @@ class ThreadInVMfromJava : public ThreadStateTransition {
 
 
 class ThreadInVMfromUnknown {
- private:
   JavaThread* _thread;
  public:
   ThreadInVMfromUnknown() : _thread(NULL) {
@@ -236,7 +192,7 @@ class ThreadInVMfromUnknown {
   }
   ~ThreadInVMfromUnknown()  {
     if (_thread) {
-      ThreadStateTransition::transition_and_fence(_thread, _thread_in_vm, _thread_in_native);
+      ThreadStateTransition::transition(_thread, _thread_in_vm, _thread_in_native);
     }
   }
 };
@@ -248,7 +204,7 @@ class ThreadInVMfromNative : public ThreadStateTransition {
     trans_from_native(_thread_in_vm);
   }
   ~ThreadInVMfromNative() {
-    trans_and_fence(_thread_in_vm, _thread_in_native);
+    trans(_thread_in_vm, _thread_in_native);
   }
 };
 
@@ -260,7 +216,7 @@ class ThreadToNativeFromVM : public ThreadStateTransition {
     // 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_and_fence(_thread_in_vm, _thread_in_native);
+    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);
   }
@@ -279,10 +235,10 @@ class ThreadBlockInVM : public ThreadStateTransition {
   : ThreadStateTransition(thread) {
     // Once we are blocked vm expects stack to be walkable
     thread->frame_anchor()->make_walkable(thread);
-    trans_and_fence(_thread_in_vm, _thread_blocked);
+    trans(_thread_in_vm, _thread_blocked);
   }
   ~ThreadBlockInVM() {
-    trans_and_fence(_thread_blocked, _thread_in_vm);
+    trans(_thread_blocked, _thread_in_vm);
     OrderAccess::cross_modify_fence();
     // We don't need to clear_walkable because it will happen automagically when we return to java
   }
@@ -322,14 +278,10 @@ class ThreadBlockInVMWithDeadlockCheck : public ThreadStateTransition {
     OrderAccess::storestore();
 
     thread->set_thread_state(_thread_blocked);
-
-    CHECK_UNHANDLED_OOPS_ONLY(_thread->clear_unhandled_oops();)
   }
   ~ThreadBlockInVMWithDeadlockCheck() {
-    // Change to transition state
+    // Change to transition state and ensure it is seen by the VM thread.
-    _thread->set_thread_state((JavaThreadState)(_thread_blocked_trans));
+    _thread->set_thread_state_fence((JavaThreadState)(_thread_blocked_trans));
-
-    InterfaceSupport::serialize_thread_state_with_handler(_thread);
 
     if (SafepointMechanism::should_block(_thread)) {
       release_monitor();
@@ -337,8 +289,6 @@ class ThreadBlockInVMWithDeadlockCheck : public ThreadStateTransition {
     }
 
     _thread->set_thread_state(_thread_in_vm);
-    CHECK_UNHANDLED_OOPS_ONLY(_thread->clear_unhandled_oops();)
-
     OrderAccess::cross_modify_fence();
   }
 };

src/hotspot/share/runtime/mutex.cpp

@@ -159,7 +159,7 @@ bool Monitor::wait(bool no_safepoint_check, long timeout,
   // !no_safepoint_check logically implies java_thread
   guarantee(no_safepoint_check || self->is_Java_thread(), "invariant");
 
-  #ifdef ASSERT
+#ifdef ASSERT
   Monitor * least = get_least_ranked_lock_besides_this(self->owned_locks());
   assert(least != this, "Specification of get_least_... call above");
   if (least != NULL && least->rank() <= special) {
@@ -168,7 +168,14 @@ bool Monitor::wait(bool no_safepoint_check, long timeout,
                name(), rank(), least->name(), least->rank());
     assert(false, "Shouldn't block(wait) while holding a lock of rank special");
   }
-  #endif // ASSERT
+#endif // ASSERT
+
+#ifdef CHECK_UNHANDLED_OOPS
+  // Clear unhandled oops in JavaThreads so we get a crash right away.
+  if (self->is_Java_thread() && !no_safepoint_check) {
+    self->clear_unhandled_oops();
+  }
+#endif // CHECK_UNHANDLED_OOPS
 
   int wait_status;
   // conceptually set the owner to NULL in anticipation of

src/hotspot/share/runtime/safepoint.cpp

@@ -805,9 +805,9 @@ void SafepointSynchronize::block(JavaThread *thread) {
 
       // This part we can skip if we notice we miss or are in a future safepoint.
       OrderAccess::storestore();
-      thread->set_thread_state(_thread_blocked);
+      // Load in wait barrier should not float up
+      thread->set_thread_state_fence(_thread_blocked);
 
-      OrderAccess::fence(); // Load in wait barrier should not float up
       _wait_barrier->wait(static_cast<int>(safepoint_id));
       assert(_state != _synchronized, "Can't be");
 

src/hotspot/share/runtime/thread.cpp

@@ -1837,7 +1837,7 @@ 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_and_fence(this, _thread_new, _thread_in_vm);
+  ThreadStateTransition::transition(this, _thread_new, _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.
@@ -2475,11 +2475,10 @@ void JavaThread::java_suspend_self_with_safepoint_check() {
   JavaThreadState state = thread_state();
   set_thread_state(_thread_blocked);
   java_suspend_self();
-  set_thread_state(state);
+  set_thread_state_fence(state);
   // Since we are not using a regular thread-state transition helper here,
   // we must manually emit the instruction barrier after leaving a safe state.
   OrderAccess::cross_modify_fence();
-  InterfaceSupport::serialize_thread_state_with_handler(this);
   if (state != _thread_in_native) {
     SafepointMechanism::block_if_requested(this);
   }

src/hotspot/share/runtime/thread.hpp

@@ -1288,6 +1288,7 @@ class JavaThread: public Thread {
   // Safepoint support
   inline JavaThreadState thread_state() const;
   inline void set_thread_state(JavaThreadState s);
+  inline void set_thread_state_fence(JavaThreadState s);  // fence after setting thread state
   inline ThreadSafepointState* safepoint_state() const;
   inline void set_safepoint_state(ThreadSafepointState* state);
   inline bool is_at_poll_safepoint();

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

@@ -141,6 +141,11 @@ inline void JavaThread::set_thread_state(JavaThreadState s) {
 #endif
 }
 
+inline void JavaThread::set_thread_state_fence(JavaThreadState s) {
+  set_thread_state(s);
+  OrderAccess::fence();
+}
+
 ThreadSafepointState* JavaThread::safepoint_state() const  {
   return _safepoint_state;
 }