8242484: Rework thread deletion during VM termination

Always delete JavaThread that calls Thread::destroy_vm()

Reviewed-by: dholmes, rehn, coleenp

src/hotspot/share/runtime/thread.cpp

@@ -4427,6 +4427,16 @@ bool Threads::destroy_vm() {
 
   thread->exit(true);
 
+  // We are no longer on the main thread list but could still be in a
+  // secondary list where another thread may try to interact with us.
+  // So wait until all such interactions are complete before we bring
+  // the VM to the termination safepoint. Normally this would be done
+  // using thread->smr_delete() below where we delete the thread, but
+  // we can't call that after the termination safepoint is active as
+  // we will deadlock on the Threads_lock. Once all interactions are
+  // complete it is safe to directly delete the thread at any time.
+  ThreadsSMRSupport::wait_until_not_protected(thread);
+
   // Stop VM thread.
   {
     // 4945125 The vm thread comes to a safepoint during exit.
@@ -4466,21 +4476,9 @@ bool Threads::destroy_vm() {
   // exit_globals() will delete tty
   exit_globals();
 
-  // We are here after VM_Exit::set_vm_exited() so we can't call
-  // thread->smr_delete() or we will block on the Threads_lock. We
-  // must check that there are no active references to this thread
-  // before attempting to delete it. A thread could be waiting on
-  // _handshake_turn_sem trying to execute a direct handshake with
-  // this thread.
-  if (!ThreadsSMRSupport::is_a_protected_JavaThread(thread)) {
-    delete thread;
-  } else {
-    // Clear value for _thread_key in TLS to prevent, depending
-    // on pthreads implementation, possible execution of
-    // thread-specific destructor in infinite loop at thread
-    // exit.
-    Thread::clear_thread_current();
-  }
+  // Deleting the shutdown thread here is safe. See comment on
+  // wait_until_not_protected() above.
+  delete thread;
 
 #if INCLUDE_JVMCI
   if (JVMCICounterSize > 0) {

src/hotspot/share/runtime/threadSMR.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2020, 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
@@ -950,14 +950,30 @@ void ThreadsSMRSupport::set_delete_notify() {
 // ThreadsListHandle.
 //
 void ThreadsSMRSupport::smr_delete(JavaThread *thread) {
-  assert(!Threads_lock->owned_by_self(), "sanity");
-
-  bool has_logged_once = false;
   elapsedTimer timer;
   if (EnableThreadSMRStatistics) {
     timer.start();
   }
 
+  wait_until_not_protected(thread);
+
+  delete thread;
+  if (EnableThreadSMRStatistics) {
+    timer.stop();
+    uint millis = (uint)timer.milliseconds();
+    ThreadsSMRSupport::inc_deleted_thread_cnt();
+    ThreadsSMRSupport::add_deleted_thread_times(millis);
+    ThreadsSMRSupport::update_deleted_thread_time_max(millis);
+  }
+
+  log_debug(thread, smr)("tid=" UINTX_FORMAT ": ThreadsSMRSupport::smr_delete: thread=" INTPTR_FORMAT " is deleted.", os::current_thread_id(), p2i(thread));
+}
+
+void ThreadsSMRSupport::wait_until_not_protected(JavaThread *thread) {
+  assert(!Threads_lock->owned_by_self(), "sanity");
+
+  bool has_logged_once = false;
+
   while (true) {
     {
       // Will not make a safepoint check because this JavaThread
@@ -979,14 +995,14 @@ void ThreadsSMRSupport::smr_delete(JavaThread *thread) {
       }
       if (!has_logged_once) {
         has_logged_once = true;
-        log_debug(thread, smr)("tid=" UINTX_FORMAT ": ThreadsSMRSupport::smr_delete: thread=" INTPTR_FORMAT " is not deleted.", os::current_thread_id(), p2i(thread));
+        log_debug(thread, smr)("tid=" UINTX_FORMAT ": ThreadsSMRSupport::wait_until_not_protected: thread=" INTPTR_FORMAT " is not deleted.", os::current_thread_id(), p2i(thread));
         if (log_is_enabled(Debug, os, thread)) {
           ScanHazardPtrPrintMatchingThreadsClosure scan_cl(thread);
           threads_do(&scan_cl);
           ThreadsList* current = _to_delete_list;
           while (current != NULL) {
             if (current->_nested_handle_cnt != 0 && current->includes(thread)) {
-              log_debug(thread, smr)("tid=" UINTX_FORMAT ": ThreadsSMRSupport::smr_delete: found nested hazard pointer to thread=" INTPTR_FORMAT, os::current_thread_id(), p2i(thread));
+              log_debug(thread, smr)("tid=" UINTX_FORMAT ": ThreadsSMRSupport::wait_until_not_protected: found nested hazard pointer to thread=" INTPTR_FORMAT, os::current_thread_id(), p2i(thread));
             }
             current = current->next_list();
           }
@@ -1012,17 +1028,6 @@ void ThreadsSMRSupport::smr_delete(JavaThread *thread) {
     ThreadsSMRSupport::delete_lock()->unlock();
     // Retry the whole scenario.
   }
-
-  delete thread;
-  if (EnableThreadSMRStatistics) {
-    timer.stop();
-    uint millis = (uint)timer.milliseconds();
-    ThreadsSMRSupport::inc_deleted_thread_cnt();
-    ThreadsSMRSupport::add_deleted_thread_times(millis);
-    ThreadsSMRSupport::update_deleted_thread_time_max(millis);
-  }
-
-  log_debug(thread, smr)("tid=" UINTX_FORMAT ": ThreadsSMRSupport::smr_delete: thread=" INTPTR_FORMAT " is deleted.", os::current_thread_id(), p2i(thread));
 }
 
 // Apply the closure to all threads in the system, with a snapshot of

src/hotspot/share/runtime/threadSMR.hpp

@@ -144,6 +144,7 @@ class ThreadsSMRSupport : AllStatic {
   static ThreadsList* get_java_thread_list();
   static bool is_a_protected_JavaThread(JavaThread *thread);
   static bool is_a_protected_JavaThread_with_lock(JavaThread *thread);
+  static void wait_until_not_protected(JavaThread *thread);
   static bool is_bootstrap_list(ThreadsList* list);
   static void remove_thread(JavaThread *thread);
   static void smr_delete(JavaThread *thread);