8282952: Thread::exit should be immune to Thread.stop

Reviewed-by: dcubed, pchilanomate, alanb

src/hotspot/share/runtime/thread.cpp

@@ -1373,23 +1373,24 @@ void JavaThread::exit(bool destroy_vm, ExitType exit_type) {
       }
     }
 
-    // Call Thread.exit(). We try 3 times in case we got another Thread.stop during
-    // the execution of the method. If that is not enough, then we don't really care. Thread.stop
-    // is deprecated anyhow.
     if (!is_Compiler_thread()) {
-      int count = 3;
-      while (java_lang_Thread::threadGroup(threadObj()) != NULL && (count-- > 0)) {
-        EXCEPTION_MARK;
-        JavaValue result(T_VOID);
-        Klass* thread_klass = vmClasses::Thread_klass();
-        JavaCalls::call_virtual(&result,
-                                threadObj, thread_klass,
-                                vmSymbols::exit_method_name(),
-                                vmSymbols::void_method_signature(),
-                                THREAD);
-        CLEAR_PENDING_EXCEPTION;
-      }
+      // We have finished executing user-defined Java code and now have to do the
+      // implementation specific clean-up by calling Thread.exit(). We prevent any
+      // asynchronous exceptions from being delivered while in Thread.exit()
+      // to ensure the clean-up is not corrupted.
+      NoAsyncExceptionDeliveryMark _no_async(this);
+
+      EXCEPTION_MARK;
+      JavaValue result(T_VOID);
+      Klass* thread_klass = vmClasses::Thread_klass();
+      JavaCalls::call_virtual(&result,
+                              threadObj, thread_klass,
+                              vmSymbols::exit_method_name(),
+                              vmSymbols::void_method_signature(),
+                              THREAD);
+      CLEAR_PENDING_EXCEPTION;
     }
+
     // notify JVMTI
     if (JvmtiExport::should_post_thread_life()) {
       JvmtiExport::post_thread_end(this);
@@ -1592,7 +1593,7 @@ void JavaThread::check_and_handle_async_exceptions() {
     // If we are at a polling page safepoint (not a poll return)
     // then we must defer async exception because live registers
     // will be clobbered by the exception path. Poll return is
-    // ok because the call we a returning from already collides
+    // ok because the call we are returning from already collides
     // with exception handling registers and so there is no issue.
     // (The exception handling path kills call result registers but
     //  this is ok since the exception kills the result anyway).
@@ -1613,6 +1614,9 @@ void JavaThread::check_and_handle_async_exceptions() {
   }
 
   if (!clear_async_exception_condition()) {
+    if ((_suspend_flags & _async_delivery_disabled) != 0) {
+      log_info(exceptions)("Async exception delivery is disabled");
+    }
     return;
   }
 

src/hotspot/share/runtime/thread.hpp

@@ -785,9 +785,10 @@ class JavaThread: public Thread {
   enum SuspendFlags {
     // NOTE: avoid using the sign-bit as cc generates different test code
     //       when the sign-bit is used, and sometimes incorrectly - see CR 6398077
-    _has_async_exception    = 0x00000001U, // there is a pending async exception
-    _trace_flag             = 0x00000004U, // call tracing backend
-    _obj_deopt              = 0x00000008U  // suspend for object reallocation and relocking for JVMTI agent
+    _has_async_exception     = 0x00000001U, // there is a pending async exception
+    _async_delivery_disabled = 0x00000002U, // async exception delivery is disabled
+    _trace_flag              = 0x00000004U, // call tracing backend
+    _obj_deopt               = 0x00000008U  // suspend for object reallocation and relocking for JVMTI agent
   };
 
   // various suspension related flags - atomically updated
@@ -815,7 +816,8 @@ class JavaThread: public Thread {
   inline bool clear_async_exception_condition();
  public:
   bool has_async_exception_condition() {
-    return (_suspend_flags & _has_async_exception) != 0;
+    return (_suspend_flags & _has_async_exception) != 0 &&
+           (_suspend_flags & _async_delivery_disabled) == 0;
   }
   inline void set_pending_async_exception(oop e);
   inline void set_pending_unsafe_access_error();
@@ -823,6 +825,13 @@ class JavaThread: public Thread {
   void send_thread_stop(oop throwable);
   void check_and_handle_async_exceptions();
 
+  class NoAsyncExceptionDeliveryMark : public StackObj {
+    friend JavaThread;
+    JavaThread *_target;
+    inline NoAsyncExceptionDeliveryMark(JavaThread *t);
+    inline ~NoAsyncExceptionDeliveryMark();
+  };
+
   // Safepoint support
  public:                                                        // Expose _thread_state for SafeFetchInt()
   volatile JavaThreadState _thread_state;

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

@@ -124,7 +124,9 @@ inline void JavaThread::clear_obj_deopt_flag() {
 
 inline bool JavaThread::clear_async_exception_condition() {
   bool ret = has_async_exception_condition();
-  clear_suspend_flag(_has_async_exception);
+  if (ret) {
+    clear_suspend_flag(_has_async_exception);
+  }
   return ret;
 }
 
@@ -138,6 +140,15 @@ inline void JavaThread::set_pending_unsafe_access_error() {
   DEBUG_ONLY(_is_unsafe_access_error = true);
 }
 
+
+inline JavaThread::NoAsyncExceptionDeliveryMark::NoAsyncExceptionDeliveryMark(JavaThread *t) : _target(t) {
+  assert((_target->_suspend_flags & _async_delivery_disabled) == 0, "Nesting is not supported");
+  _target->set_suspend_flag(_async_delivery_disabled);
+}
+inline JavaThread::NoAsyncExceptionDeliveryMark::~NoAsyncExceptionDeliveryMark() {
+  _target->clear_suspend_flag(_async_delivery_disabled);
+}
+
 inline JavaThreadState JavaThread::thread_state() const    {
 #if defined(PPC64) || defined (AARCH64)
   // Use membars when accessing volatile _thread_state. See