8200374: Add ThreadsSMRSupport::verify_hazard_pointer_scanned() to verify threads_do()

Add verification for the threads_do() part of the Thread-SMR protocol.

Reviewed-by: eosterlund, dholmes, rehn

src/hotspot/share/runtime/thread.cpp

@@ -106,6 +106,7 @@
 #include "services/threadService.hpp"
 #include "trace/traceMacros.hpp"
 #include "trace/tracing.hpp"
+#include "trace/tracingExport.hpp"
 #include "utilities/align.hpp"
 #include "utilities/copy.hpp"
 #include "utilities/defaultStream.hpp"
@@ -3435,17 +3436,14 @@ static inline void *prefetch_and_load_ptr(void **addr, intx prefetch_interval) {
 // All JavaThreads
 #define ALL_JAVA_THREADS(X) DO_JAVA_THREADS(ThreadsSMRSupport::get_java_thread_list(), X)
 
-// All JavaThreads + all non-JavaThreads (i.e., every thread in the system)
+// All non-JavaThreads (i.e., every non-JavaThread in the system).
-void Threads::threads_do(ThreadClosure* tc) {
+void Threads::non_java_threads_do(ThreadClosure* tc) {
-  assert_locked_or_safepoint(Threads_lock);
-  // ALL_JAVA_THREADS iterates through all JavaThreads
-  ALL_JAVA_THREADS(p) {
-    tc->do_thread(p);
-  }
   // Someday we could have a table or list of all non-JavaThreads.
   // For now, just manually iterate through them.
   tc->do_thread(VMThread::vm_thread());
-  Universe::heap()->gc_threads_do(tc);
+  if (Universe::heap() != NULL) {
+    Universe::heap()->gc_threads_do(tc);
+  }
   WatcherThread *wt = WatcherThread::watcher_thread();
   // Strictly speaking, the following NULL check isn't sufficient to make sure
   // the data for WatcherThread is still valid upon being examined. However,
@@ -3458,9 +3456,26 @@ void Threads::threads_do(ThreadClosure* tc) {
     tc->do_thread(wt);
   }
 
+#if INCLUDE_TRACE
+  Thread* sampler_thread = TracingExport::sampler_thread_acquire();
+  if (sampler_thread != NULL) {
+    tc->do_thread(sampler_thread);
+  }
+#endif
+
   // If CompilerThreads ever become non-JavaThreads, add them here
 }
 
+// All JavaThreads + all non-JavaThreads (i.e., every thread in the system).
+void Threads::threads_do(ThreadClosure* tc) {
+  assert_locked_or_safepoint(Threads_lock);
+  // ALL_JAVA_THREADS iterates through all JavaThreads.
+  ALL_JAVA_THREADS(p) {
+    tc->do_thread(p);
+  }
+  non_java_threads_do(tc);
+}
+
 void Threads::possibly_parallel_threads_do(bool is_par, ThreadClosure* tc) {
   int cp = Threads::thread_claim_parity();
   ALL_JAVA_THREADS(p) {

src/hotspot/share/runtime/thread.hpp

@@ -2096,6 +2096,7 @@ class Threads: AllStatic {
   // thread to the thread list before allocating its thread object
   static void add(JavaThread* p, bool force_daemon = false);
   static void remove(JavaThread* p);
+  static void non_java_threads_do(ThreadClosure* tc);
   static void threads_do(ThreadClosure* tc);
   static void possibly_parallel_threads_do(bool is_par, ThreadClosure* tc);
 

src/hotspot/share/runtime/threadSMR.cpp

@@ -470,16 +470,6 @@ ThreadsList *ThreadsList::remove_thread(ThreadsList* list, JavaThread* java_thre
 
 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();
   }
@@ -553,6 +543,76 @@ ThreadsListSetter::~ThreadsListSetter() {
   }
 }
 
+// Closure to determine if the specified JavaThread is found by
+// threads_do().
+//
+class VerifyHazardPointerThreadClosure : public ThreadClosure {
+ private:
+  bool _found;
+  Thread *_self;
+
+ public:
+  VerifyHazardPointerThreadClosure(Thread *self) : _found(false), _self(self) {}
+
+  bool found() const { return _found; }
+
+  virtual void do_thread(Thread *thread) {
+    if (thread == _self) {
+      _found = true;
+    }
+  }
+};
+
+// Apply the closure to all threads in the system, with a snapshot of
+// all JavaThreads provided by the list parameter.
+void ThreadsSMRSupport::threads_do(ThreadClosure *tc, ThreadsList *list) {
+  list->threads_do(tc);
+  Threads::non_java_threads_do(tc);
+}
+
+// Apply the closure to all threads in the system.
+void ThreadsSMRSupport::threads_do(ThreadClosure *tc) {
+  threads_do(tc, _java_thread_list);
+}
+
+// Verify that the stable hazard pointer used to safely keep threads
+// alive is scanned by threads_do() which is a key piece of honoring
+// the Thread-SMR protocol.
+void ThreadsSMRSupport::verify_hazard_pointer_scanned(Thread *self, ThreadsList *threads) {
+#ifdef ASSERT
+  assert(threads != NULL, "threads must not be NULL");
+
+  // The closure will attempt to verify that the calling thread can
+  // be found by threads_do() on the specified ThreadsList. If it
+  // is successful, then the specified ThreadsList was acquired as
+  // a stable hazard pointer by the calling thread in a way that
+  // honored the Thread-SMR protocol.
+  //
+  // If the calling thread cannot be found by threads_do() and if
+  // it is not the shutdown thread, then the calling thread is not
+  // honoring the Thread-SMR ptotocol. This means that the specified
+  // ThreadsList is not a stable hazard pointer and can be freed
+  // by another thread from the to-be-deleted list at any time.
+  //
+  // Note: The shutdown thread has removed itself from the Threads
+  // list and is safe to have a waiver from this check 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.
+  //
+  VerifyHazardPointerThreadClosure cl(self);
+  threads_do(&cl, threads);
+
+  // If the calling thread is not honoring the Thread-SMR protocol,
+  // then we will either crash in threads_do() above because 'threads'
+  // was freed by another thread or we will fail the assert() below.
+  // In either case, we won't get past this point with a badly placed
+  // ThreadsListHandle.
+
+  assert(cl.found() || self == VM_Exit::shutdown_thread(), "Acquired a ThreadsList snapshot from a thread not recognized by the Thread-SMR protocol.");
+#endif
+}
+
 void ThreadsListSetter::set() {
   assert(_target->get_threads_hazard_ptr() == NULL, "hazard ptr should not already be set");
   (void) ThreadsSMRSupport::acquire_stable_list(_target, /* is_ThreadsListSetter */ true);
@@ -626,6 +686,8 @@ ThreadsList *ThreadsSMRSupport::acquire_stable_list_fast_path(Thread *self) {
   // are protected and hence they should not be deleted until everyone
   // agrees it is safe to do so.
 
+  verify_hazard_pointer_scanned(self, threads);
+
   return threads;
 }
 
@@ -662,6 +724,8 @@ ThreadsList *ThreadsSMRSupport::acquire_stable_list_nested_path(Thread *self) {
   }
   log_debug(thread, smr)("tid=" UINTX_FORMAT ": ThreadsSMRSupport::acquire_stable_list: add NestedThreadsList node containing ThreadsList=" INTPTR_FORMAT, os::current_thread_id(), p2i(node->t_list()));
 
+  verify_hazard_pointer_scanned(self, node->t_list());
+
   return node->t_list();
 }
 
@@ -722,7 +786,7 @@ void ThreadsSMRSupport::free_list(ThreadsList* threads) {
   // Gather a hash table of the current hazard ptrs:
   ThreadScanHashtable *scan_table = new ThreadScanHashtable(hash_table_size);
   ScanHazardPtrGatherThreadsListClosure scan_cl(scan_table);
-  Threads::threads_do(&scan_cl);
+  threads_do(&scan_cl);
 
   // Walk through the linked list of pending freeable ThreadsLists
   // and free the ones that are not referenced from hazard ptrs.
@@ -784,7 +848,7 @@ bool ThreadsSMRSupport::is_a_protected_JavaThread(JavaThread *thread) {
   // hazard ptrs.
   ThreadScanHashtable *scan_table = new ThreadScanHashtable(hash_table_size);
   ScanHazardPtrGatherProtectedThreadsClosure scan_cl(scan_table);
-  Threads::threads_do(&scan_cl);
+  threads_do(&scan_cl);
 
   bool thread_is_protected = false;
   if (scan_table->has_entry((void*)thread)) {
@@ -949,7 +1013,7 @@ void ThreadsSMRSupport::smr_delete(JavaThread *thread) {
         log_debug(thread, smr)("tid=" UINTX_FORMAT ": ThreadsSMRSupport::smr_delete: thread=" INTPTR_FORMAT " is not deleted.", os::current_thread_id(), p2i(thread));
         if (log_is_enabled(Debug, os, thread)) {
           ScanHazardPtrPrintMatchingThreadsClosure scan_cl(thread);
-          Threads::threads_do(&scan_cl);
+          threads_do(&scan_cl);
         }
       }
     } // We have to drop the Threads_lock to wait or delete the thread

src/hotspot/share/runtime/threadSMR.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2018, 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
@@ -28,6 +28,8 @@
 #include "memory/allocation.hpp"
 #include "runtime/timer.hpp"
 
+class ThreadClosure;
+
 // Thread Safe Memory Reclamation (Thread-SMR) support.
 //
 // ThreadsListHandles are used to safely perform operations on one or more
@@ -124,9 +126,12 @@ class ThreadsSMRSupport : AllStatic {
   static void release_stable_list_nested_path(Thread *self);
   static void release_stable_list_wake_up(char *log_str);
   static void set_delete_notify();
+  static void threads_do(ThreadClosure *tc);
+  static void threads_do(ThreadClosure *tc, ThreadsList *list);
   static void update_deleted_thread_time_max(uint new_value);
   static void update_java_thread_list_max(uint new_value);
   static void update_tlh_time_max(uint new_value);
+  static void verify_hazard_pointer_scanned(Thread *self, ThreadsList *threads);
   static ThreadsList* xchg_java_thread_list(ThreadsList* new_list);
 
  public:

src/hotspot/share/trace/tracingExport.cpp

@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) 2018, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+#include "precompiled.hpp"
+#include "runtime/orderAccess.inline.hpp"
+#include "trace/tracingExport.hpp"
+
+// Allow lock-free reads of _sampler_thread.
+Thread* volatile TracingExport::_sampler_thread = NULL;
+
+Thread* TracingExport::sampler_thread_acquire() {
+  return (Thread*)OrderAccess::load_acquire(&_sampler_thread);
+}
+
+void TracingExport::set_sampler_thread_with_lock(Thread* thread) {
+  // Grab Threads_lock to avoid conflicts with Thread-SMR scans.
+  MutexLocker ml(Threads_lock);
+  assert(thread == NULL || _sampler_thread == NULL, "_sampler_thread should not already be set.");
+  // To match with the lock-free load_acquire():
+  OrderAccess::release_store(&_sampler_thread, thread);
+}

src/hotspot/share/trace/tracingExport.hpp

@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) 2018, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+#ifndef SHARE_VM_TRACE_TRACINGEXPORT_HPP
+#define SHARE_VM_TRACE_TRACINGEXPORT_HPP
+
+#include "runtime/thread.hpp"
+
+class TracingExport : AllStatic {
+ public:
+  static Thread* sampler_thread_acquire();
+  static void set_sampler_thread_with_lock(Thread* thread);
+
+ private:
+  static Thread* volatile _sampler_thread;
+};
+
+#endif // SHARE_VM_TRACE_TRACINGEXPORT_HPP