8274379: Allow process of unsafe access errors in check_special_condition_for_native_trans

Reviewed-by: rehn, dholmes

Author: pchilano

src/hotspot/share/prims/jvm.cpp

@@ -2961,7 +2961,7 @@ JVM_ENTRY(void, JVM_StopThread(JNIEnv* env, jobject jthread, jobject throwable))
       THROW_OOP(java_throwable);
     } else {
       // Use a VM_Operation to throw the exception.
-      JavaThread::send_async_exception(java_thread, java_throwable);
+      JavaThread::send_async_exception(receiver, java_throwable);
     }
   } else {
     // Either:

src/hotspot/share/prims/jvmtiEnv.cpp

@@ -1072,7 +1072,7 @@ JvmtiEnv::StopThread(JavaThread* java_thread, jobject exception) {
   oop e = JNIHandles::resolve_external_guard(exception);
   NULL_CHECK(e, JVMTI_ERROR_NULL_POINTER);
 
-  JavaThread::send_async_exception(java_thread->threadObj(), e);
+  JavaThread::send_async_exception(java_thread, e);
 
   return JVMTI_ERROR_NONE;
 

src/hotspot/share/runtime/safepoint.hpp

@@ -131,7 +131,6 @@ class SafepointSynchronize : AllStatic {
     switch(state) {
       case _thread_in_vm:
       case _thread_in_Java:        // From compiled code
-      case _thread_in_native_trans:
       case _thread_blocked_trans:
         return true;
       default:

src/hotspot/share/runtime/thread.cpp

@@ -1001,8 +1001,10 @@ JavaThread::JavaThread() :
   _monitor_chunks(nullptr),
 
   _suspend_flags(0),
-  _async_exception_condition(_no_async_condition),
   _pending_async_exception(nullptr),
+#ifdef ASSERT
+  _is_unsafe_access_error(false),
+#endif
 
   _thread_state(_thread_new),
   _saved_exception_pc(nullptr),
@@ -1572,9 +1574,6 @@ void JavaThread::remove_monitor_chunk(MonitorChunk* chunk) {
 
 // Asynchronous exceptions support
 //
-// Note: this function shouldn't block if it's called in
-// _thread_in_native_trans state (such as from
-// check_special_condition_for_native_trans()).
 void JavaThread::check_and_handle_async_exceptions() {
   if (has_last_Java_frame() && has_async_exception_condition()) {
     // If we are at a polling page safepoint (not a poll return)
@@ -1600,21 +1599,12 @@ void JavaThread::check_and_handle_async_exceptions() {
     }
   }
 
-  AsyncExceptionCondition condition = clear_async_exception_condition();
-  if (condition == _no_async_condition) {
-    // Conditions have changed since has_special_runtime_exit_condition()
-    // was called:
-    // - if we were here only because of an external suspend request,
-    //   then that was taken care of above (or cancelled) so we are done
-    // - if we were here because of another async request, then it has
-    //   been cleared between the has_special_runtime_exit_condition()
-    //   and now so again we are done
+  if (!clear_async_exception_condition()) {
     return;
   }
 
-  // Check for pending async. exception
   if (_pending_async_exception != NULL) {
-    // Only overwrite an already pending exception, if it is not a threadDeath.
+    // Only overwrite an already pending exception if it is not a threadDeath.
     if (!has_pending_exception() || !pending_exception()->is_a(vmClasses::ThreadDeath_klass())) {
 
       // We cannot call Exceptions::_throw(...) here because we cannot block
@@ -1631,25 +1621,23 @@ void JavaThread::check_and_handle_async_exceptions() {
           }
         ls.print_cr(" of type: %s", _pending_async_exception->klass()->external_name());
       }
-      _pending_async_exception = NULL;
-      // Clear condition from _suspend_flags since we have finished processing it.
-      clear_suspend_flag(_has_async_exception);
     }
-  }
+    // Always null out the _pending_async_exception oop here since the async condition was
+    // already cleared above and thus considered handled.
+    _pending_async_exception = NULL;
+  } else {
+    assert(_is_unsafe_access_error, "must be");
+    DEBUG_ONLY(_is_unsafe_access_error = false);
 
-  if (condition == _async_unsafe_access_error && !has_pending_exception()) {
     // We may be at method entry which requires we save the do-not-unlock flag.
     UnlockFlagSaver fs(this);
     switch (thread_state()) {
     case _thread_in_vm: {
       JavaThread* THREAD = this;
       Exceptions::throw_unsafe_access_internal_error(THREAD, __FILE__, __LINE__, "a fault occurred in an unsafe memory access operation");
-      return;
-    }
-    case _thread_in_native: {
-      ThreadInVMfromNative tiv(this);
-      JavaThread* THREAD = this;
-      Exceptions::throw_unsafe_access_internal_error(THREAD, __FILE__, __LINE__, "a fault occurred in an unsafe memory access operation");
+      // We might have blocked in a ThreadBlockInVM wrapper in the call above so make sure we process pending
+      // suspend requests and object reallocation operations if any since we might be going to Java after this.
+      SafepointMechanism::process_if_requested_with_exit_check(this, true /* check asyncs */);
       return;
     }
     case _thread_in_Java: {
@@ -1662,8 +1650,6 @@ void JavaThread::check_and_handle_async_exceptions() {
       ShouldNotReachHere();
     }
   }
-
-  assert(has_pending_exception(), "must have handled the async condition if no exception");
 }
 
 void JavaThread::handle_special_runtime_exit_condition(bool check_asyncs) {
@@ -1696,9 +1682,8 @@ class InstallAsyncExceptionClosure : public HandshakeClosure {
   }
 };
 
-void JavaThread::send_async_exception(oop java_thread, oop java_throwable) {
+void JavaThread::send_async_exception(JavaThread* target, oop java_throwable) {
   Handle throwable(Thread::current(), java_throwable);
-  JavaThread* target = java_lang_Thread::thread(java_thread);
   InstallAsyncExceptionClosure vm_stop(throwable);
   Handshake::execute(&vm_stop, target);
 }
@@ -1847,21 +1832,17 @@ void JavaThread::check_special_condition_for_native_trans(JavaThread *thread) {
   assert(thread->thread_state() == _thread_in_native_trans, "wrong state");
   assert(!thread->has_last_Java_frame() || thread->frame_anchor()->walkable(), "Unwalkable stack in native->Java transition");
 
+  thread->set_thread_state(_thread_in_vm);
+
   // Enable WXWrite: called directly from interpreter native wrapper.
   MACOS_AARCH64_ONLY(ThreadWXEnable wx(WXWrite, thread));
 
-  SafepointMechanism::process_if_requested_with_exit_check(thread, false /* check asyncs */);
+  SafepointMechanism::process_if_requested_with_exit_check(thread, true /* check asyncs */);
 
   // After returning from native, it could be that the stack frames are not
   // yet safe to use. We catch such situations in the subsequent stack watermark
   // barrier, which will trap unsafe stack frames.
   StackWatermarkSet::before_unwind(thread);
-
-  if (thread->has_async_exception_condition(false /* check unsafe access error */)) {
-    // We are in _thread_in_native_trans state, don't handle unsafe
-    // access error since that may block.
-    thread->check_and_handle_async_exceptions();
-  }
 }
 
 #ifndef PRODUCT

src/hotspot/share/runtime/thread.hpp

@@ -802,37 +802,21 @@ class JavaThread: public Thread {
 
   // Asynchronous exceptions support
  private:
-  enum AsyncExceptionCondition {
-    _no_async_condition = 0,
-    _async_exception,
-    _async_unsafe_access_error
-  };
-  AsyncExceptionCondition _async_exception_condition;
-  oop                     _pending_async_exception;
-
-  void set_async_exception_condition(AsyncExceptionCondition aec) { _async_exception_condition = aec; }
-  AsyncExceptionCondition clear_async_exception_condition() {
-    AsyncExceptionCondition x = _async_exception_condition;
-    _async_exception_condition = _no_async_condition;
-    return x;
-  }
+  oop     _pending_async_exception;
+#ifdef ASSERT
+  bool    _is_unsafe_access_error;
+#endif
 
+  inline bool clear_async_exception_condition();
  public:
-  bool has_async_exception_condition(bool check_unsafe_access_error = true) {
-    return check_unsafe_access_error ? _async_exception_condition != _no_async_condition
-                                     : _async_exception_condition == _async_exception;
+  bool has_async_exception_condition() {
+    return (_suspend_flags & _has_async_exception) != 0;
   }
   inline void set_pending_async_exception(oop e);
-  void set_pending_unsafe_access_error()  {
-    // Don't overwrite an asynchronous exception sent by another thread
-    if (_async_exception_condition == _no_async_condition) {
-      set_async_exception_condition(_async_unsafe_access_error);
-    }
-  }
-  void check_and_handle_async_exceptions();
-  // Installs a pending exception to be inserted later
-  static void send_async_exception(oop thread_oop, oop java_throwable);
+  inline void set_pending_unsafe_access_error();
+  static void send_async_exception(JavaThread* jt, oop java_throwable);
   void send_thread_stop(oop throwable);
+  void check_and_handle_async_exceptions();
 
   // Safepoint support
  public:                                                        // Expose _thread_state for SafeFetchInt()
@@ -1177,8 +1161,7 @@ class JavaThread: public Thread {
   // Return true if JavaThread has an asynchronous condition or
   // if external suspension is requested.
   bool has_special_runtime_exit_condition() {
-    return (_async_exception_condition != _no_async_condition) ||
-           (_suspend_flags & (_obj_deopt JFR_ONLY(| _trace_flag))) != 0;
+    return (_suspend_flags & (_has_async_exception | _obj_deopt JFR_ONLY(| _trace_flag))) != 0;
   }
 
   // Fast-locking support

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

@@ -122,15 +122,22 @@ inline void JavaThread::clear_obj_deopt_flag() {
   clear_suspend_flag(_obj_deopt);
 }
 
+inline bool JavaThread::clear_async_exception_condition() {
+  bool ret = has_async_exception_condition();
+  clear_suspend_flag(_has_async_exception);
+  return ret;
+}
+
 inline void JavaThread::set_pending_async_exception(oop e) {
   _pending_async_exception = e;
-  set_async_exception_condition(_async_exception);
-  // Set _suspend_flags too so we save a comparison in the transition from native to Java
-  // in the native wrappers. It will be cleared in check_and_handle_async_exceptions()
-  // when we actually install the exception.
   set_suspend_flag(_has_async_exception);
 }
 
+inline void JavaThread::set_pending_unsafe_access_error() {
+  set_suspend_flag(_has_async_exception);
+  DEBUG_ONLY(_is_unsafe_access_error = true);
+}
+
 inline JavaThreadState JavaThread::thread_state() const    {
 #if defined(PPC64) || defined (AARCH64)
   // Use membars when accessing volatile _thread_state. See

src/hotspot/share/runtime/vmStructs.cpp

@@ -717,7 +717,6 @@
   nonstatic_field(JavaThread,                  _current_pending_monitor_is_from_java,         bool)                                  \
   volatile_nonstatic_field(JavaThread,         _current_waiting_monitor,                      ObjectMonitor*)                        \
   volatile_nonstatic_field(JavaThread,         _suspend_flags,                                uint32_t)                              \
-  nonstatic_field(JavaThread,                  _async_exception_condition,                    JavaThread::AsyncExceptionCondition)   \
   nonstatic_field(JavaThread,                  _pending_async_exception,                      oop)                                   \
   volatile_nonstatic_field(JavaThread,         _exception_oop,                                oop)                                   \
   volatile_nonstatic_field(JavaThread,         _exception_pc,                                 address)                               \
@@ -1951,7 +1950,6 @@
   declare_toplevel_type(JavaThread*)                                      \
   declare_toplevel_type(JavaThread *const *const)                         \
   declare_toplevel_type(java_lang_Class)                                  \
-  declare_integer_type(JavaThread::AsyncExceptionCondition)               \
   declare_integer_type(JavaThread::TerminatedTypes)                       \
   declare_toplevel_type(jbyte*)                                           \
   declare_toplevel_type(jbyte**)                                          \