8263191: Consolidate ThreadInVMfromJavaNoAsyncException and ThreadBlockInVMWithDeadlockCheck with existing wrappers

Reviewed-by: coleenp, dholmes, dcubed

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

@@ -150,8 +150,9 @@ class ThreadInVMForHandshake : public ThreadStateTransition {
 };
 
 class ThreadInVMfromJava : public ThreadStateTransition {
+  bool _check_asyncs;
  public:
-  ThreadInVMfromJava(JavaThread* thread) : ThreadStateTransition(thread) {
+  ThreadInVMfromJava(JavaThread* thread, bool check_asyncs = true) : ThreadStateTransition(thread), _check_asyncs(check_asyncs) {
     trans_from_java(_thread_in_vm);
   }
   ~ThreadInVMfromJava()  {
@@ -159,8 +160,9 @@ class ThreadInVMfromJava : public ThreadStateTransition {
       _thread->stack_overflow_state()->enable_stack_yellow_reserved_zone();
     }
     trans(_thread_in_vm, _thread_in_Java);
-    // Check for pending. async. exceptions or suspends.
+    // We prevent asynchronous exceptions from being installed on return to Java in situations
-    if (_thread->has_special_runtime_exit_condition()) _thread->handle_special_runtime_exit_condition();
+    // 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);
   }
 };
 
@@ -222,93 +224,45 @@ class ThreadToNativeFromVM : public ThreadStateTransition {
 };
 
 
+// Parameter in_flight_mutex_addr is only used by class Mutex to avoid certain deadlock
+// scenarios while making transitions that might block for a safepoint or handshake.
+// It's the address of a pointer to the mutex we are trying to acquire. This will be used to
+// access and release said mutex when transitioning back from blocked to vm (destructor) in
+// case we need to stop for a safepoint or handshake.
 class ThreadBlockInVM : public ThreadStateTransition {
- public:
-  ThreadBlockInVM(JavaThread *thread)
-  : ThreadStateTransition(thread) {
-    // Once we are blocked vm expects stack to be walkable
-    thread->frame_anchor()->make_walkable(thread);
-    trans(_thread_in_vm, _thread_blocked);
-  }
-  ~ThreadBlockInVM() {
-    trans(_thread_blocked, _thread_in_vm);
-    // We don't need to clear_walkable because it will happen automagically when we return to java
-  }
-};
-
-// Unlike ThreadBlockInVM, this class is designed to avoid certain deadlock scenarios while making
-// transitions inside class Mutex in cases where we need to block for a safepoint or handshake. It
-// receives an extra argument compared to ThreadBlockInVM, the address of a pointer to the mutex we
-// are trying to acquire. This will be used to access and release the mutex if needed to avoid
-// said deadlocks.
-// It works like ThreadBlockInVM but differs from it in two ways:
-// - When transitioning in (constructor), it checks for safepoints without blocking, i.e., calls
-//   back if needed to allow a pending safepoint to continue but does not block in it.
-// - When transitioning back (destructor), if there is a pending safepoint or handshake it releases
-//   the mutex that is only partially acquired.
-class ThreadBlockInVMWithDeadlockCheck : public ThreadStateTransition {
  private:
   Mutex** _in_flight_mutex_addr;
 
-  void release_mutex() {
-    assert(_in_flight_mutex_addr != NULL, "_in_flight_mutex_addr should have been set on constructor");
-    Mutex* in_flight_mutex = *_in_flight_mutex_addr;
-    if (in_flight_mutex != NULL) {
-      in_flight_mutex->release_for_safepoint();
-      *_in_flight_mutex_addr = NULL;
-    }
-  }
  public:
-  ThreadBlockInVMWithDeadlockCheck(JavaThread* thread, Mutex** in_flight_mutex_addr)
+  ThreadBlockInVM(JavaThread* thread, Mutex** in_flight_mutex_addr = NULL)
   : ThreadStateTransition(thread), _in_flight_mutex_addr(in_flight_mutex_addr) {
+    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);
-
-    // All unsafe states are treated the same by the VMThread
-    // so we can skip the _thread_in_vm_trans state here. Since
-    // we don't read poll, it's enough to order the stores.
-    OrderAccess::storestore();
-
     thread->set_thread_state(_thread_blocked);
   }
-  ~ThreadBlockInVMWithDeadlockCheck() {
+  ~ThreadBlockInVM() {
+    assert(_thread->thread_state() == _thread_blocked, "coming from wrong thread state");
     // Change to transition state and ensure it is seen by the VM thread.
-    _thread->set_thread_state_fence((JavaThreadState)(_thread_blocked_trans));
+    _thread->set_thread_state_fence(_thread_blocked_trans);
 
     if (SafepointMechanism::should_process(_thread)) {
-      release_mutex();
+      if (_in_flight_mutex_addr != NULL) {
+        release_mutex();
+      }
       SafepointMechanism::process_if_requested(_thread);
     }
 
     _thread->set_thread_state(_thread_in_vm);
   }
-};
 
-
+  void release_mutex() {
-// This special transition class is only used to prevent asynchronous exceptions
+    Mutex* in_flight_mutex = *_in_flight_mutex_addr;
-// from being installed on vm exit in situations where we can't tolerate them.
+    if (in_flight_mutex != NULL) {
-// See bugs: 4324348, 4854693, 4998314, 5040492, 5050705.
+      in_flight_mutex->release_for_safepoint();
-class ThreadInVMfromJavaNoAsyncException : public ThreadStateTransition {
+      *_in_flight_mutex_addr = NULL;
- public:
-  ThreadInVMfromJavaNoAsyncException(JavaThread* thread) : ThreadStateTransition(thread) {
-    trans_from_java(_thread_in_vm);
-  }
-  ~ThreadInVMfromJavaNoAsyncException()  {
-    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);
-    // NOTE: We do not check for pending. async. exceptions.
-    // If we did and moved the pending async exception over into the
-    // pending exception field, we would need to deopt (currently C2
-    // only). However, to do so would require that we transition back
-    // to the _thread_in_vm state. Instead we postpone the handling of
-    // the async exception.
-
-
-    // Check for pending. suspends only.
-    if (_thread->has_special_runtime_exit_condition())
-      _thread->handle_special_runtime_exit_condition(false);
   }
 };
 
@@ -383,7 +337,7 @@ class VMNativeEntryWrapper {
 
 #define JRT_ENTRY_NO_ASYNC(result_type, header)                      \
   result_type header {                                               \
-    ThreadInVMfromJavaNoAsyncException __tiv(thread);                \
+    ThreadInVMfromJava __tiv(thread, false /* check asyncs */);      \
     VM_ENTRY_BASE(result_type, header, thread)                       \
     debug_only(VMEntryWrapper __vew;)
 
@@ -401,7 +355,7 @@ class VMNativeEntryWrapper {
 
 #define JRT_BLOCK_NO_ASYNC                                           \
     {                                                                \
-    ThreadInVMfromJavaNoAsyncException __tiv(thread);                \
+    ThreadInVMfromJava __tiv(thread, false /* check asyncs */);      \
     Thread* THREAD = thread;                                         \
     debug_only(VMEntryWrapper __vew;)
 

src/hotspot/share/runtime/mutex.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2021, 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
@@ -84,12 +84,12 @@ void Mutex::lock_contended(Thread* self) {
     // Is it a JavaThread participating in the safepoint protocol.
     if (is_active_Java_thread) {
       assert(rank() > Mutex::special, "Potential deadlock with special or lesser rank mutex");
-      { ThreadBlockInVMWithDeadlockCheck tbivmdc(self->as_Java_thread(), &in_flight_mutex);
+      { ThreadBlockInVM tbivmdc(self->as_Java_thread(), &in_flight_mutex);
-        in_flight_mutex = this;  // save for ~ThreadBlockInVMWithDeadlockCheck
+        in_flight_mutex = this;  // save for ~ThreadBlockInVM
         _lock.lock();
       }
       if (in_flight_mutex != NULL) {
-        // Not unlocked by ~ThreadBlockInVMWithDeadlockCheck
+        // Not unlocked by ~ThreadBlockInVM
         break;
       }
     } else {
@@ -236,7 +236,7 @@ bool Monitor::wait(int64_t timeout, bool as_suspend_equivalent) {
   Mutex* in_flight_mutex = NULL;
 
   {
-    ThreadBlockInVMWithDeadlockCheck tbivmdc(self, &in_flight_mutex);
+    ThreadBlockInVM tbivmdc(self, &in_flight_mutex);
     OSThreadWaitState osts(self->osthread(), false /* not Object.wait() */);
     if (as_suspend_equivalent) {
       self->set_suspend_equivalent();
@@ -245,7 +245,7 @@ bool Monitor::wait(int64_t timeout, bool as_suspend_equivalent) {
     }
 
     wait_status = _lock.wait(timeout);
-    in_flight_mutex = this;  // save for ~ThreadBlockInVMWithDeadlockCheck
+    in_flight_mutex = this;  // save for ~ThreadBlockInVM
 
     // were we externally suspended while we were waiting?
     if (as_suspend_equivalent && self->handle_special_suspend_equivalent_condition()) {
@@ -260,7 +260,7 @@ bool Monitor::wait(int64_t timeout, bool as_suspend_equivalent) {
   }
 
   if (in_flight_mutex != NULL) {
-    // Not unlocked by ~ThreadBlockInVMWithDeadlockCheck
+    // Not unlocked by ~ThreadBlockInVM
     assert_owner(NULL);
     // Conceptually reestablish ownership of the lock.
     set_owner(self);

src/hotspot/share/runtime/safepoint.cpp

@@ -968,7 +968,7 @@ void ThreadSafepointState::handle_polling_page_exception() {
     // If we have a pending async exception deoptimize the frame
     // as otherwise we may never deliver it.
     if (self->has_async_condition()) {
-      ThreadInVMfromJavaNoAsyncException __tiv(self);
+      ThreadInVMfromJava __tiv(self, false /* check asyncs */);
       Deoptimization::deoptimize_frame(self, caller_fr.id());
     }