--- old/src/hotspot/share/runtime/threadSMR.cpp 2018-03-28 20:13:02.000000000 -0400 +++ new/src/hotspot/share/runtime/threadSMR.cpp 2018-03-28 20:13:01.000000000 -0400 @@ -470,16 +470,6 @@ 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,74 @@ } } +// 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 + // 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 +684,8 @@ // 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 +722,8 @@ } 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 +784,7 @@ // 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 +846,7 @@ // 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 +1011,7 @@ 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