# HG changeset patch
# User dcubed
# Date 1522253073 14400
# Node ID 6d5bd76650dfe923d0ddcc6d20f766398b88109e
# Parent  e79bbf1635da44ae3aa108c23f5660168a028952
8199813: SIGSEGV in ThreadsList::includes()
Summary: ThreadsListHandles cannot be used by JavaThreads that are not on the Threads list.
Reviewed-by: eosterlund, gthornbr, dholmes, rehn

diff -r e79bbf1635da -r 6d5bd76650df src/hotspot/os/linux/os_linux.cpp
--- a/src/hotspot/os/linux/os_linux.cpp	Wed Mar 28 16:39:32 2018 +0200
+++ b/src/hotspot/os/linux/os_linux.cpp	Wed Mar 28 12:04:33 2018 -0400
@@ -1644,10 +1644,7 @@
         //
         // Dynamic loader will make all stacks executable after
         // this function returns, and will not do that again.
-#ifdef ASSERT
-        ThreadsListHandle tlh;
-        assert(tlh.length() == 0, "no Java threads should exist yet.");
-#endif
+        assert(Threads::number_of_threads() == 0, "no Java threads should exist yet.");
       } else {
         warning("You have loaded library %s which might have disabled stack guard. "
                 "The VM will try to fix the stack guard now.\n"
diff -r e79bbf1635da -r 6d5bd76650df src/hotspot/share/runtime/thread.cpp
--- a/src/hotspot/share/runtime/thread.cpp	Wed Mar 28 16:39:32 2018 +0200
+++ b/src/hotspot/share/runtime/thread.cpp	Wed Mar 28 12:04:33 2018 -0400
@@ -2392,11 +2392,13 @@
 }
 
 #ifdef ASSERT
-// verify the JavaThread has not yet been published in the Threads::list, and
-// hence doesn't need protection from concurrent access at this stage
+// Verify the JavaThread has not yet been published in the Threads::list, and
+// hence doesn't need protection from concurrent access at this stage.
 void JavaThread::verify_not_published() {
-  ThreadsListHandle tlh;
-  assert(!tlh.includes(this), "JavaThread shouldn't have been published yet!");
+  // Cannot create a ThreadsListHandle here and check !tlh.includes(this)
+  // since an unpublished JavaThread doesn't participate in the
+  // Thread-SMR protocol for keeping a ThreadsList alive.
+  assert(!on_thread_list(), "JavaThread shouldn't have been published yet!");
 }
 #endif
 
@@ -4262,11 +4264,6 @@
     VMThread::destroy();
   }
 
-  // clean up ideal graph printers
-#if defined(COMPILER2) && !defined(PRODUCT)
-  IdealGraphPrinter::clean_up();
-#endif
-
   // Now, all Java threads are gone except daemon threads. Daemon threads
   // running Java code or in VM are stopped by the Safepoint. However,
   // daemon threads executing native code are still running.  But they
@@ -4275,6 +4272,16 @@
 
   VM_Exit::set_vm_exited();
 
+  // Clean up ideal graph printers after the VMThread has started
+  // the final safepoint which will block all the Compiler threads.
+  // Note that this Thread has already logically exited so the
+  // clean_up() function's use of a JavaThreadIteratorWithHandle
+  // would be a problem except set_vm_exited() has remembered the
+  // shutdown thread which is granted a policy exception.
+#if defined(COMPILER2) && !defined(PRODUCT)
+  IdealGraphPrinter::clean_up();
+#endif
+
   notify_vm_shutdown();
 
   // We are after VM_Exit::set_vm_exited() so we can't call
diff -r e79bbf1635da -r 6d5bd76650df src/hotspot/share/runtime/threadSMR.cpp
--- a/src/hotspot/share/runtime/threadSMR.cpp	Wed Mar 28 16:39:32 2018 +0200
+++ b/src/hotspot/share/runtime/threadSMR.cpp	Wed Mar 28 12:04:33 2018 -0400
@@ -28,6 +28,7 @@
 #include "runtime/jniHandles.inline.hpp"
 #include "runtime/thread.inline.hpp"
 #include "runtime/threadSMR.inline.hpp"
+#include "runtime/vm_operations.hpp"
 #include "services/threadService.hpp"
 #include "utilities/copy.hpp"
 #include "utilities/globalDefinitions.hpp"
@@ -469,6 +470,16 @@
 
 ThreadsListHandle::ThreadsListHandle(Thread *self) : _list(ThreadsSMRSupport::acquire_stable_list(self, /* is_ThreadsListSetter */ false)), _self(self) {
   assert(self == Thread::current(), "sanity check");
+  // Threads::threads_do() is used by the Thread-SMR protocol to visit all
+  // Threads in the system which ensures the safety of the ThreadsList
+  // managed by this ThreadsListHandle, but JavaThreads that are not on
+  // the Threads list cannot be included in that visit. The JavaThread that
+  // calls Threads::destroy_vm() is exempt from this check because it has
+  // to logically exit as part of the shutdown procedure. This is safe
+  // because VM_Exit::_shutdown_thread is not set until after the VMThread
+  // has started the final safepoint which holds the Threads_lock for the
+  // remainder of the VM's life.
+  assert(!self->is_Java_thread() || self == VM_Exit::shutdown_thread() || (((JavaThread*)self)->on_thread_list() && !((JavaThread*)self)->is_terminated()), "JavaThread must be on the Threads list to use a ThreadsListHandle");
   if (EnableThreadSMRStatistics) {
     _timer.start();
   }
diff -r e79bbf1635da -r 6d5bd76650df src/hotspot/share/runtime/vm_operations.cpp
--- a/src/hotspot/share/runtime/vm_operations.cpp	Wed Mar 28 16:39:32 2018 +0200
+++ b/src/hotspot/share/runtime/vm_operations.cpp	Wed Mar 28 12:04:33 2018 -0400
@@ -417,7 +417,7 @@
 }
 
 volatile bool VM_Exit::_vm_exited = false;
-Thread * VM_Exit::_shutdown_thread = NULL;
+Thread * volatile VM_Exit::_shutdown_thread = NULL;
 
 int VM_Exit::set_vm_exited() {
 
diff -r e79bbf1635da -r 6d5bd76650df src/hotspot/share/runtime/vm_operations.hpp
--- a/src/hotspot/share/runtime/vm_operations.hpp	Wed Mar 28 16:39:32 2018 +0200
+++ b/src/hotspot/share/runtime/vm_operations.hpp	Wed Mar 28 12:04:33 2018 -0400
@@ -459,7 +459,7 @@
  private:
   int  _exit_code;
   static volatile bool _vm_exited;
-  static Thread * _shutdown_thread;
+  static Thread * volatile _shutdown_thread;
   static void wait_if_vm_exited();
  public:
   VM_Exit(int exit_code) {
@@ -468,6 +468,7 @@
   static int wait_for_threads_in_native_to_block();
   static int set_vm_exited();
   static bool vm_exited()                      { return _vm_exited; }
+  static Thread * shutdown_thread()            { return _shutdown_thread; }
   static void block_if_vm_exited() {
     if (_vm_exited) {
       wait_if_vm_exited();