--- old/src/hotspot/share/runtime/thread.cpp 2018-05-01 12:16:59.000000000 -0400 +++ new/src/hotspot/share/runtime/thread.cpp 2018-05-01 12:16:59.000000000 -0400 @@ -243,7 +243,7 @@ // This initial value ==> never claimed. _oops_do_parity = 0; _threads_hazard_ptr = NULL; - _nested_threads_hazard_ptr = NULL; + _threads_list_ptr = NULL; _nested_threads_hazard_ptr_cnt = 0; _rcu_counter = 0; @@ -881,33 +881,11 @@ st->print("tid=" INTPTR_FORMAT " ", p2i(this)); osthread()->print_on(st); } - if (_threads_hazard_ptr != NULL) { - st->print("_threads_hazard_ptr=" INTPTR_FORMAT, p2i(_threads_hazard_ptr)); - } - if (_nested_threads_hazard_ptr != NULL) { - print_nested_threads_hazard_ptrs_on(st); - } + ThreadsSMRSupport::print_info_on(this, st); st->print(" "); debug_only(if (WizardMode) print_owned_locks_on(st);) } -void Thread::print_nested_threads_hazard_ptrs_on(outputStream* st) const { - assert(_nested_threads_hazard_ptr != NULL, "must be set to print"); - - if (EnableThreadSMRStatistics) { - st->print(", _nested_threads_hazard_ptr_cnt=%u", _nested_threads_hazard_ptr_cnt); - } - st->print(", _nested_threads_hazard_ptrs="); - for (NestedThreadsList* node = _nested_threads_hazard_ptr; node != NULL; - node = node->next()) { - if (node != _nested_threads_hazard_ptr) { - // First node does not need a comma-space separator. - st->print(", "); - } - st->print(INTPTR_FORMAT, p2i(node->t_list())); - } -} - // Thread::print_on_error() is called by fatal error handler. Don't use // any lock or allocate memory. void Thread::print_on_error(outputStream* st, char* buf, int buflen) const { @@ -930,12 +908,7 @@ st->print(" [id=%d]", osthread()->thread_id()); } - if (_threads_hazard_ptr != NULL) { - st->print(" _threads_hazard_ptr=" INTPTR_FORMAT, p2i(_threads_hazard_ptr)); - } - if (_nested_threads_hazard_ptr != NULL) { - print_nested_threads_hazard_ptrs_on(st); - } + ThreadsSMRSupport::print_info_on(this, st); } void Thread::print_value_on(outputStream* st) const { @@ -2995,12 +2968,7 @@ p2i(stack_end()), p2i(stack_base())); st->print("]"); - if (_threads_hazard_ptr != NULL) { - st->print(" _threads_hazard_ptr=" INTPTR_FORMAT, p2i(_threads_hazard_ptr)); - } - if (_nested_threads_hazard_ptr != NULL) { - print_nested_threads_hazard_ptrs_on(st); - } + ThreadsSMRSupport::print_info_on(this, st); return; } --- old/src/hotspot/share/runtime/thread.hpp 2018-05-01 12:17:06.000000000 -0400 +++ new/src/hotspot/share/runtime/thread.hpp 2018-05-01 12:17:05.000000000 -0400 @@ -52,10 +52,10 @@ # include "stack_zero.hpp" #endif +class SafeThreadsListPtr; class ThreadSafepointState; class ThreadsList; class ThreadsSMRSupport; -class NestedThreadsList; class JvmtiThreadState; class JvmtiGetLoadedClassesClosure; @@ -135,13 +135,14 @@ void* _real_malloc_address; // JavaThread lifecycle support: + friend class SafeThreadsListPtr; // for _threads_list_ptr, cmpxchg_threads_hazard_ptr(), {dec_,inc_,}nested_threads_hazard_ptr_cnt(), {g,s}et_threads_hazard_ptr(), inc_nested_handle_cnt(), tag_hazard_ptr() access friend class ScanHazardPtrGatherProtectedThreadsClosure; // for cmpxchg_threads_hazard_ptr(), get_threads_hazard_ptr(), is_hazard_ptr_tagged() access - friend class ScanHazardPtrGatherThreadsListClosure; // for get_nested_threads_hazard_ptr(), get_threads_hazard_ptr(), untag_hazard_ptr() access + friend class ScanHazardPtrGatherThreadsListClosure; // for get_threads_hazard_ptr(), untag_hazard_ptr() access friend class ScanHazardPtrPrintMatchingThreadsClosure; // for get_threads_hazard_ptr(), is_hazard_ptr_tagged() access - friend class ThreadsListSetter; // for get_threads_hazard_ptr() access - friend class ThreadsSMRSupport; // for get_threads_hazard_ptr() access + friend class ThreadsSMRSupport; // for _nested_threads_hazard_ptr_cnt, _threads_hazard_ptr, _threads_list_ptr access ThreadsList* volatile _threads_hazard_ptr; + SafeThreadsListPtr* _threads_list_ptr; ThreadsList* cmpxchg_threads_hazard_ptr(ThreadsList* exchange_value, ThreadsList* compare_value); ThreadsList* get_threads_hazard_ptr(); void set_threads_hazard_ptr(ThreadsList* new_list); @@ -154,15 +155,6 @@ static ThreadsList* untag_hazard_ptr(ThreadsList* list) { return (ThreadsList*)(intptr_t(list) & ~intptr_t(1)); } - NestedThreadsList* _nested_threads_hazard_ptr; - NestedThreadsList* get_nested_threads_hazard_ptr() { - return _nested_threads_hazard_ptr; - } - void set_nested_threads_hazard_ptr(NestedThreadsList* value) { - assert(Threads_lock->owned_by_self(), - "must own Threads_lock for _nested_threads_hazard_ptr to be valid."); - _nested_threads_hazard_ptr = value; - } // This field is enabled via -XX:+EnableThreadSMRStatistics: uint _nested_threads_hazard_ptr_cnt; void dec_nested_threads_hazard_ptr_cnt() { @@ -640,7 +632,6 @@ // Printing virtual void print_on(outputStream* st) const; - virtual void print_nested_threads_hazard_ptrs_on(outputStream* st) const; void print() const { print_on(tty); } virtual void print_on_error(outputStream* st, char* buf, int buflen) const; void print_value_on(outputStream* st) const; --- old/src/hotspot/share/runtime/threadSMR.cpp 2018-05-01 12:17:11.000000000 -0400 +++ new/src/hotspot/share/runtime/threadSMR.cpp 2018-05-01 12:17:11.000000000 -0400 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018, 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 @@ -32,6 +32,7 @@ #include "services/threadService.hpp" #include "utilities/copy.hpp" #include "utilities/globalDefinitions.hpp" +#include "utilities/ostream.hpp" #include "utilities/resourceHash.hpp" #include "utilities/vmError.hpp" @@ -252,8 +253,6 @@ current_list = thread->get_threads_hazard_ptr(); // No hazard ptr so nothing more to do. if (current_list == NULL) { - assert(thread->get_nested_threads_hazard_ptr() == NULL, - "cannot have a nested hazard ptr with a NULL regular hazard ptr"); return; } @@ -278,13 +277,6 @@ // ThreadsList. AddThreadHazardPointerThreadClosure add_cl(_table); current_list->threads_do(&add_cl); - - // Any NestedThreadsLists are also protecting JavaThreads so - // gather those also; the ThreadsLists may be different. - for (NestedThreadsList* node = thread->get_nested_threads_hazard_ptr(); - node != NULL; node = node->next()) { - node->t_list()->threads_do(&add_cl); - } } }; @@ -302,8 +294,6 @@ if (thread == NULL) return; ThreadsList *threads = thread->get_threads_hazard_ptr(); if (threads == NULL) { - assert(thread->get_nested_threads_hazard_ptr() == NULL, - "cannot have a nested hazard ptr with a NULL regular hazard ptr"); return; } // In this closure we always ignore the tag that might mark this @@ -315,16 +305,6 @@ if (!_table->has_entry((void*)threads)) { _table->add_entry((void*)threads); } - - // Any NestedThreadsLists are also protecting JavaThreads so - // gather those also; the ThreadsLists may be different. - for (NestedThreadsList* node = thread->get_nested_threads_hazard_ptr(); - node != NULL; node = node->next()) { - threads = node->t_list(); - if (!_table->has_entry((void*)threads)) { - _table->add_entry((void*)threads); - } - } } }; @@ -343,8 +323,6 @@ if (thread == NULL) return; ThreadsList *current_list = thread->get_threads_hazard_ptr(); if (current_list == NULL) { - assert(thread->get_nested_threads_hazard_ptr() == NULL, - "cannot have a nested hazard ptr with a NULL regular hazard ptr"); return; } // If the hazard ptr is unverified, then ignore it. @@ -362,32 +340,19 @@ break; } } - - // Any NestedThreadsLists are also protecting JavaThreads so - // check those also; the ThreadsLists may be different. - for (NestedThreadsList* node = thread->get_nested_threads_hazard_ptr(); - node != NULL; node = node->next()) { - JavaThreadIterator jti(node->t_list()); - for (JavaThread *p = jti.first(); p != NULL; p = jti.next()) { - if (p == _thread) { - log_debug(thread, smr)("tid=" UINTX_FORMAT ": ThreadsSMRSupport::smr_delete: thread1=" INTPTR_FORMAT " has a nested hazard pointer for thread2=" INTPTR_FORMAT, os::current_thread_id(), p2i(thread), p2i(_thread)); - return; - } - } - } } }; // Closure to determine if the specified JavaThread is found by // threads_do(). // -class VerifyHazardPointerThreadClosure : public ThreadClosure { +class VerifyHazardPtrThreadClosure : public ThreadClosure { private: bool _found; Thread *_self; public: - VerifyHazardPointerThreadClosure(Thread *self) : _found(false), _self(self) {} + VerifyHazardPtrThreadClosure(Thread *self) : _found(false), _self(self) {} bool found() const { return _found; } @@ -401,39 +366,37 @@ // Acquire a stable ThreadsList. // -ThreadsList *ThreadsSMRSupport::acquire_stable_list(Thread *self, bool is_ThreadsListSetter) { - assert(self != NULL, "sanity check"); - // acquire_stable_list_nested_path() will grab the Threads_lock - // so let's make sure the ThreadsListHandle is in a safe place. - // ThreadsListSetter cannot make this check on this code path. - debug_only(if (!is_ThreadsListSetter && StrictSafepointChecks) self->check_for_valid_safepoint_state(/* potential_vm_operation */ false);) +void SafeThreadsListPtr::acquire_stable_list() { + assert(_thread != NULL, "sanity check"); + _needs_release = true; + _previous = _thread->_threads_list_ptr; + _thread->_threads_list_ptr = this; - if (self->get_threads_hazard_ptr() == NULL) { + if (_thread->get_threads_hazard_ptr() == NULL) { // The typical case is first. - return acquire_stable_list_fast_path(self); + acquire_stable_list_fast_path(); + return; } // The nested case is rare. - return acquire_stable_list_nested_path(self); + acquire_stable_list_nested_path(); } -// Fast path (and lock free) way to acquire a stable ThreadsList. +// Fast path way to acquire a stable ThreadsList. // -ThreadsList *ThreadsSMRSupport::acquire_stable_list_fast_path(Thread *self) { - assert(self != NULL, "sanity check"); - assert(self->get_threads_hazard_ptr() == NULL, "sanity check"); - assert(self->get_nested_threads_hazard_ptr() == NULL, - "cannot have a nested hazard ptr with a NULL regular hazard ptr"); +void SafeThreadsListPtr::acquire_stable_list_fast_path() { + assert(_thread != NULL, "sanity check"); + assert(_thread->get_threads_hazard_ptr() == NULL, "sanity check"); ThreadsList* threads; // Stable recording of a hazard ptr for SMR. This code does not use - // locks so its use of the _java_thread_list & _threads_hazard_ptr + // locks so its use of the _smr_java_thread_list & _threads_hazard_ptr // fields is racy relative to code that uses those fields with locks. // OrderAccess and Atomic functions are used to deal with those races. // while (true) { - threads = get_java_thread_list(); + threads = ThreadsSMRSupport::get_java_thread_list(); // Publish a tagged hazard ptr to denote that the hazard ptr is not // yet verified as being stable. Due to the fence after the hazard @@ -442,11 +405,11 @@ // non-multiple copy atomic machines where stores can be observed // in different order from different observer threads. ThreadsList* unverified_threads = Thread::tag_hazard_ptr(threads); - self->set_threads_hazard_ptr(unverified_threads); + _thread->set_threads_hazard_ptr(unverified_threads); - // If _java_thread_list has changed, we have lost a race with + // If _smr_java_thread_list has changed, we have lost a race with // Threads::add() or Threads::remove() and have to try again. - if (get_java_thread_list() != threads) { + if (ThreadsSMRSupport::get_java_thread_list() != threads) { continue; } @@ -456,7 +419,7 @@ // being followed to access JavaThread ptrs. If we lose the race, // we simply retry. If we win the race, then the stable hazard // ptr is officially published. - if (self->cmpxchg_threads_hazard_ptr(threads, unverified_threads) == unverified_threads) { + if (_thread->cmpxchg_threads_hazard_ptr(threads, unverified_threads) == unverified_threads) { break; } } @@ -466,143 +429,102 @@ // are protected and hence they should not be deleted until everyone // agrees it is safe to do so. - verify_hazard_pointer_scanned(self, threads); + _list = threads; - return threads; + verify_hazard_ptr_scanned(); } // Acquire a nested stable ThreadsList; this is rare so it uses -// Threads_lock. +// reference counting. // -ThreadsList *ThreadsSMRSupport::acquire_stable_list_nested_path(Thread *self) { - assert(self != NULL, "sanity check"); - assert(self->get_threads_hazard_ptr() != NULL, +void SafeThreadsListPtr::acquire_stable_list_nested_path() { + assert(_thread != NULL, "sanity check"); + assert(_thread->get_threads_hazard_ptr() != NULL, "cannot have a NULL regular hazard ptr when acquiring a nested hazard ptr"); // The thread already has a hazard ptr (ThreadsList ref) so we need // to create a nested ThreadsListHandle with the current ThreadsList - // since it might be different than our current hazard ptr. The need - // for a nested ThreadsListHandle is rare so we do this while holding - // the Threads_lock so we don't race with the scanning code; the code - // is so much simpler this way. - - NestedThreadsList* node; - { - // Only grab the Threads_lock if we don't already own it. - MutexLockerEx ml(Threads_lock->owned_by_self() ? NULL : Threads_lock); - node = new NestedThreadsList(get_java_thread_list()); - // We insert at the front of the list to match up with the delete - // in release_stable_list(). - node->set_next(self->get_nested_threads_hazard_ptr()); - self->set_nested_threads_hazard_ptr(node); - if (EnableThreadSMRStatistics) { - self->inc_nested_threads_hazard_ptr_cnt(); - if (self->nested_threads_hazard_ptr_cnt() > _nested_thread_list_max) { - _nested_thread_list_max = self->nested_threads_hazard_ptr_cnt(); - } - } + // since it might be different than our current hazard ptr. To remedy + // the situation, the ThreadsList pointed to by the pre-existing + // stable hazard ptr is reference counted before the hazard ptr may + // be released and moved to a new ThreadsList. The old ThreadsList + // is remembered in the ThreadsListHandle. + + ThreadsList* current_list = _previous->_list; + if (EnableThreadSMRStatistics) { + _thread->inc_nested_threads_hazard_ptr_cnt(); + } + current_list->inc_nested_handle_cnt(); + _previous->_has_ref_count = true; // promote SafeThreadsListPtr to be reference counted + _thread->_threads_hazard_ptr = NULL; // clear the hazard ptr so we can go through the fast path below + + if (EnableThreadSMRStatistics && _thread->nested_threads_hazard_ptr_cnt() > ThreadsSMRSupport::_nested_thread_list_max) { + ThreadsSMRSupport::_nested_thread_list_max = _thread->nested_threads_hazard_ptr_cnt(); } - 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()); + acquire_stable_list_fast_path(); - return node->t_list(); + verify_hazard_ptr_scanned(); + + log_debug(thread, smr)("tid=" UINTX_FORMAT ": SafeThreadsListPtr::acquire_stable_list: add nested list pointer to ThreadsList=" INTPTR_FORMAT, os::current_thread_id(), p2i(_list)); } // Release a stable ThreadsList. // -void ThreadsSMRSupport::release_stable_list(Thread *self) { - assert(self != NULL, "sanity check"); - // release_stable_list_nested_path() will grab the Threads_lock - // so let's make sure the ThreadsListHandle is in a safe place. - debug_only(if (StrictSafepointChecks) self->check_for_valid_safepoint_state(/* potential_vm_operation */ false);) +void SafeThreadsListPtr::release_stable_list() { + assert(_thread != NULL, "sanity check"); + assert(_thread->_threads_list_ptr == this, "sanity check"); + _thread->_threads_list_ptr = _previous; + + if (_has_ref_count) { + // If a SafeThreadsListPtr has been promoted to use reference counting + // due to nesting of ThreadsListHandles, then the reference count must be + // decremented, at which point it may be freed. The forgotten value of + // the list no longer matters at this point and should already be NULL. + assert(_thread->get_threads_hazard_ptr() == NULL, "sanity check"); + if (EnableThreadSMRStatistics) { + _thread->dec_nested_threads_hazard_ptr_cnt(); + } + _list->dec_nested_handle_cnt(); - if (self->get_nested_threads_hazard_ptr() == NULL) { - // The typical case is first. - release_stable_list_fast_path(self); - return; + log_debug(thread, smr)("tid=" UINTX_FORMAT ": SafeThreadsListPtr::release_stable_list: delete nested list pointer to ThreadsList=" INTPTR_FORMAT, os::current_thread_id(), p2i(_list)); + } else { + // The normal case: a leaf ThreadsListHandle. This merely requires setting + // the thread hazard ptr back to NULL. + assert(_thread->get_threads_hazard_ptr() != NULL, "sanity check"); + _thread->set_threads_hazard_ptr(NULL); } - // The nested case is rare. - release_stable_list_nested_path(self); -} - -// Fast path way to release a stable ThreadsList. The release portion -// is lock-free, but the wake up portion is not. -// -void ThreadsSMRSupport::release_stable_list_fast_path(Thread *self) { - assert(self != NULL, "sanity check"); - assert(self->get_threads_hazard_ptr() != NULL, "sanity check"); - assert(self->get_nested_threads_hazard_ptr() == NULL, - "cannot have a nested hazard ptr when releasing a regular hazard ptr"); - // After releasing the hazard ptr, other threads may go ahead and // free up some memory temporarily used by a ThreadsList snapshot. - self->set_threads_hazard_ptr(NULL); // We use double-check locking to reduce traffic on the system // wide Thread-SMR delete_lock. if (ThreadsSMRSupport::delete_notify()) { // An exiting thread might be waiting in smr_delete(); we need to // check with delete_lock to be sure. - release_stable_list_wake_up((char *) "regular hazard ptr"); + ThreadsSMRSupport::release_stable_list_wake_up(_has_ref_count); } } -// Release a nested stable ThreadsList; this is rare so it uses -// Threads_lock. -// -void ThreadsSMRSupport::release_stable_list_nested_path(Thread *self) { - assert(self != NULL, "sanity check"); - assert(self->get_nested_threads_hazard_ptr() != NULL, "sanity check"); - assert(self->get_threads_hazard_ptr() != NULL, - "must have a regular hazard ptr to have nested hazard ptrs"); - - // We have a nested ThreadsListHandle so we have to release it first. - // The need for a nested ThreadsListHandle is rare so we do this while - // holding the Threads_lock so we don't race with the scanning code; - // the code is so much simpler this way. - - NestedThreadsList *node; - { - // Only grab the Threads_lock if we don't already own it. - MutexLockerEx ml(Threads_lock->owned_by_self() ? NULL : Threads_lock); - // We remove from the front of the list to match up with the insert - // in acquire_stable_list(). - node = self->get_nested_threads_hazard_ptr(); - self->set_nested_threads_hazard_ptr(node->next()); - if (EnableThreadSMRStatistics) { - self->dec_nested_threads_hazard_ptr_cnt(); - } - } - - // An exiting thread might be waiting in smr_delete(); we need to - // check with delete_lock to be sure. - release_stable_list_wake_up((char *) "nested hazard ptr"); - - log_debug(thread, smr)("tid=" UINTX_FORMAT ": ThreadsSMRSupport::release_stable_list: delete NestedThreadsList node containing ThreadsList=" INTPTR_FORMAT, os::current_thread_id(), p2i(node->t_list())); - - delete node; -} - -// Verify that the stable hazard pointer used to safely keep threads +// Verify that the stable hazard ptr 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) { +void SafeThreadsListPtr::verify_hazard_ptr_scanned() { #ifdef ASSERT - assert(threads != NULL, "threads must not be NULL"); + assert(_list != NULL, "_list 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. + // a stable hazard ptr 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. + // ThreadsList is not a stable hazard ptr 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 @@ -610,8 +532,8 @@ // 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); + VerifyHazardPtrThreadClosure cl(_thread); + ThreadsSMRSupport::threads_do(&cl, _list); // If the calling thread is not honoring the Thread-SMR protocol, // then we will either crash in threads_do() above because 'threads' @@ -619,12 +541,17 @@ // 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."); + assert(cl.found() || _thread == VM_Exit::shutdown_thread(), "Acquired a ThreadsList snapshot from a thread not recognized by the Thread-SMR protocol."); #endif } // 'entries + 1' so we always have at least one entry. -ThreadsList::ThreadsList(int entries) : _length(entries), _threads(NEW_C_HEAP_ARRAY(JavaThread*, entries + 1, mtThread)), _next_list(NULL) { +ThreadsList::ThreadsList(int entries) : + _length(entries), + _next_list(NULL), + _threads(NEW_C_HEAP_ARRAY(JavaThread*, entries + 1, mtThread)), + _nested_handle_cnt(0) +{ *(JavaThread**)(_threads + entries) = NULL; // Make sure the extra entry is NULL. } @@ -649,6 +576,15 @@ return new_list; } +void ThreadsList::dec_nested_handle_cnt() { + // The decrement needs to be MO_ACQ_REL. At the moment, the Atomic::dec + // backend on PPC does not yet conform to these requirements. Therefore + // the decrement is simulated with an Atomic::sub(1, &addr). + // Without this MO_ACQ_REL Atomic::dec simulation, the nested SMR mechanism + // is not generally safe to use. + Atomic::sub(1, &_nested_handle_cnt); +} + int ThreadsList::find_index_of_JavaThread(JavaThread *target) { if (target == NULL) { return -1; @@ -676,6 +612,22 @@ return NULL; } +void ThreadsList::inc_nested_handle_cnt() { + // The increment needs to be MO_SEQ_CST. At the moment, the Atomic::inc + // backend on PPC does not yet conform to these requirements. Therefore + // the increment is simulated with a load phi; cas phi + 1; loop. + // Without this MO_SEQ_CST Atomic::inc simulation, the nested SMR mechanism + // is not generally safe to use. + intx sample = OrderAccess::load_acquire(&_nested_handle_cnt); + for (;;) { + if (Atomic::cmpxchg(sample + 1, &_nested_handle_cnt, sample) == sample) { + return; + } else { + sample = OrderAccess::load_acquire(&_nested_handle_cnt); + } + } +} + bool ThreadsList::includes(const JavaThread * const p) const { if (p == NULL) { return false; @@ -712,7 +664,7 @@ return new_list; } -ThreadsListHandle::ThreadsListHandle(Thread *self) : _list(ThreadsSMRSupport::acquire_stable_list(self, /* is_ThreadsListSetter */ false)), _self(self) { +ThreadsListHandle::ThreadsListHandle(Thread *self) : _list_ptr(self, /* acquire */ true) { assert(self == Thread::current(), "sanity check"); if (EnableThreadSMRStatistics) { _timer.start(); @@ -720,7 +672,6 @@ } ThreadsListHandle::~ThreadsListHandle() { - ThreadsSMRSupport::release_stable_list(_self); if (EnableThreadSMRStatistics) { _timer.stop(); uint millis = (uint)_timer.milliseconds(); @@ -780,30 +731,17 @@ return true; } -ThreadsListSetter::~ThreadsListSetter() { - if (_target_needs_release) { - // The hazard ptr in the target needs to be released. - ThreadsSMRSupport::release_stable_list(_target); - } -} - -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); - _target_needs_release = true; -} - void ThreadsSMRSupport::add_thread(JavaThread *thread){ - ThreadsList *new_list = ThreadsList::add_thread(ThreadsSMRSupport::get_java_thread_list(), thread); + ThreadsList *new_list = ThreadsList::add_thread(get_java_thread_list(), thread); if (EnableThreadSMRStatistics) { - ThreadsSMRSupport::inc_java_thread_list_alloc_cnt(); - ThreadsSMRSupport::update_java_thread_list_max(new_list->length()); + inc_java_thread_list_alloc_cnt(); + update_java_thread_list_max(new_list->length()); } // Initial _java_thread_list will not generate a "Threads::add" mesg. log_debug(thread, smr)("tid=" UINTX_FORMAT ": Threads::add: new ThreadsList=" INTPTR_FORMAT, os::current_thread_id(), p2i(new_list)); - ThreadsList *old_list = ThreadsSMRSupport::xchg_java_thread_list(new_list); - ThreadsSMRSupport::free_list(old_list); + ThreadsList *old_list = xchg_java_thread_list(new_list); + free_list(old_list); } // set_delete_notify() and clear_delete_notify() are called @@ -851,6 +789,8 @@ ThreadScanHashtable *scan_table = new ThreadScanHashtable(hash_table_size); ScanHazardPtrGatherThreadsListClosure scan_cl(scan_table); threads_do(&scan_cl); + OrderAccess::acquire(); // Must order reads of hazard ptr before reads of + // nested reference counters // Walk through the linked list of pending freeable ThreadsLists // and free the ones that are not referenced from hazard ptrs. @@ -860,7 +800,7 @@ bool threads_is_freed = false; while (current != NULL) { next = current->next_list(); - if (!scan_table->has_entry((void*)current)) { + if (!scan_table->has_entry((void*)current) && current->_nested_handle_cnt == 0) { // This ThreadsList is not referenced by a hazard ptr. if (prev != NULL) { prev->set_next_list(next); @@ -913,6 +853,22 @@ ThreadScanHashtable *scan_table = new ThreadScanHashtable(hash_table_size); ScanHazardPtrGatherProtectedThreadsClosure scan_cl(scan_table); threads_do(&scan_cl); + OrderAccess::acquire(); // Must order reads of hazard ptr before reads of + // nested reference counters + + // Walk through the linked list of pending freeable ThreadsLists + // and include the ones that are currently in use by a nested + // ThreadsListHandle in the search set. + ThreadsList* current = _to_delete_list; + while (current != NULL) { + if (current->_nested_handle_cnt != 0) { + // 'current' is in use by a nested ThreadsListHandle so the hazard + // ptr is protecting all the JavaThreads on that ThreadsList. + AddThreadHazardPointerThreadClosure add_cl(scan_table); + current->threads_do(&add_cl); + } + current = current->next_list(); + } bool thread_is_protected = false; if (scan_table->has_entry((void*)thread)) { @@ -925,8 +881,8 @@ // Wake up portion of the release stable ThreadsList protocol; // uses the delete_lock(). // -void ThreadsSMRSupport::release_stable_list_wake_up(char *log_str) { - assert(log_str != NULL, "sanity check"); +void ThreadsSMRSupport::release_stable_list_wake_up(bool is_nested) { + const char* log_str = is_nested ? "nested hazard ptr" : "regular hazard ptr"; // Note: delete_lock is held in smr_delete() for the entire // hazard ptr search so that we do not lose this notify() if @@ -1002,6 +958,13 @@ 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)); + } + current = current->next_list(); + } } } } // We have to drop the Threads_lock to wait or delete the thread @@ -1058,6 +1021,17 @@ // Debug, logging, and printing stuff at the end: +// Print SMR info for a SafeThreadsListPtr to a given output stream. +void SafeThreadsListPtr::print_on(outputStream* st) { + if (this == _thread->_threads_list_ptr) { + // The top level hazard ptr. + st->print(" _threads_hazard_ptr=" INTPTR_FORMAT, p2i(_list)); + } else { + // Nested hazard ptrs. + st->print(", _nested_threads_hazard_ptr=" INTPTR_FORMAT, p2i(_list)); + } +} + // Log Threads class SMR info. void ThreadsSMRSupport::log_statistics() { LogTarget(Info, thread, smr) log; @@ -1067,6 +1041,30 @@ } } +// Print SMR info for a thread to a given output stream. +void ThreadsSMRSupport::print_info_on(const Thread* thread, outputStream* st) { + if (thread->_threads_hazard_ptr != NULL) { + st->print(" _threads_hazard_ptr=" INTPTR_FORMAT, p2i(thread->_threads_hazard_ptr)); + } + if (EnableThreadSMRStatistics && thread->_threads_list_ptr != NULL) { + // The count is only interesting if we have a _threads_list_ptr. + st->print(", _nested_threads_hazard_ptr_cnt=%u", thread->_nested_threads_hazard_ptr_cnt); + } + if (SafepointSynchronize::is_at_safepoint() || Thread::current() == thread) { + // It is only safe to walk the list if we're at a safepoint or the + // calling thread is walking its own list. + SafeThreadsListPtr* current = thread->_threads_list_ptr; + if (current != NULL) { + // Skip the top nesting level as it is always printed above. + current = current->previous(); + } + while (current != NULL) { + current->print_on(st); + current = current->previous(); + } + } +} + // Print Threads class SMR info. void ThreadsSMRSupport::print_info_on(outputStream* st) { // Only grab the Threads_lock if we don't already own it @@ -1096,8 +1094,8 @@ if (!EnableThreadSMRStatistics) { return; } - st->print_cr("_java_thread_list_alloc_cnt=" UINT64_FORMAT "," - "_java_thread_list_free_cnt=" UINT64_FORMAT "," + st->print_cr("_java_thread_list_alloc_cnt=" UINT64_FORMAT ", " + "_java_thread_list_free_cnt=" UINT64_FORMAT ", " "_java_thread_list_max=%u, " "_nested_thread_list_max=%u", _java_thread_list_alloc_cnt, --- old/src/hotspot/share/runtime/threadSMR.hpp 2018-05-01 12:17:17.000000000 -0400 +++ new/src/hotspot/share/runtime/threadSMR.hpp 2018-05-01 12:17:17.000000000 -0400 @@ -28,6 +28,10 @@ #include "memory/allocation.hpp" #include "runtime/timer.hpp" +class JavaThread; +class Monitor; +class outputStream; +class Thread; class ThreadClosure; // Thread Safe Memory Reclamation (Thread-SMR) support. @@ -82,6 +86,8 @@ // SMR Support for the Threads class. // class ThreadsSMRSupport : AllStatic { + friend class SafeThreadsListPtr; // for _nested_thread_list_max, delete_notify(), release_stable_list_wake_up() access + // The coordination between ThreadsSMRSupport::release_stable_list() and // ThreadsSMRSupport::smr_delete() uses the delete_lock in order to // reduce the traffic on the Threads_lock. @@ -122,24 +128,20 @@ static void inc_java_thread_list_alloc_cnt(); static void inc_tlh_cnt(); static bool is_a_protected_JavaThread(JavaThread *thread); - static void release_stable_list_fast_path(Thread *self); - static void release_stable_list_nested_path(Thread *self); - static void release_stable_list_wake_up(char *log_str); + static void release_stable_list_wake_up(bool is_nested); 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 void verify_hazard_ptr_scanned(Thread *self, ThreadsList *threads); static ThreadsList* xchg_java_thread_list(ThreadsList* new_list); public: - static ThreadsList *acquire_stable_list(Thread *self, bool is_ThreadsListSetter); static void add_thread(JavaThread *thread); static ThreadsList* get_java_thread_list(); static bool is_a_protected_JavaThread_with_lock(JavaThread *thread); - static void release_stable_list(Thread *self); static void remove_thread(JavaThread *thread); static void smr_delete(JavaThread *thread); static void update_tlh_stats(uint millis); @@ -148,16 +150,19 @@ static void log_statistics(); static void print_info_elements_on(outputStream* st, ThreadsList* t_list); static void print_info_on(outputStream* st); + static void print_info_on(const Thread* thread, outputStream* st); }; // A fast list of JavaThreads. // class ThreadsList : public CHeapObj { - friend class ThreadsSMRSupport; // for next_list(), set_next_list() access + friend class SafeThreadsListPtr; // for {dec,inc}_nested_handle_cnt() access + friend class ThreadsSMRSupport; // for _nested_handle_cnt, {add,remove}_thread(), {,set_}next_list() access const uint _length; ThreadsList* _next_list; JavaThread *const *const _threads; + volatile intx _nested_handle_cnt; template void threads_do_dispatch(T *cl, JavaThread *const thread) const; @@ -165,6 +170,9 @@ ThreadsList *next_list() const { return _next_list; } void set_next_list(ThreadsList *list) { _next_list = list; } + void inc_nested_handle_cnt(); + void dec_nested_handle_cnt(); + static ThreadsList* add_thread(ThreadsList* list, JavaThread* java_thread); static ThreadsList* remove_thread(ThreadsList* list, JavaThread* java_thread); @@ -187,20 +195,60 @@ bool includes(const JavaThread * const p) const; }; -// Linked list of ThreadsLists to support nested ThreadsListHandles. -class NestedThreadsList : public CHeapObj { - ThreadsList*const _t_list; - NestedThreadsList* _next; +// An abstract safe ptr to a ThreadsList comprising either a stable hazard ptr +// for leaves, or a retained reference count for nested uses. The user of this +// API does not need to know which mechanism is providing the safety. +class SafeThreadsListPtr { + friend class ThreadsListSetter; + + SafeThreadsListPtr* _previous; + Thread* _thread; + ThreadsList* _list; + bool _has_ref_count; + bool _needs_release; + + void acquire_stable_list(); + void acquire_stable_list_fast_path(); + void acquire_stable_list_nested_path(); + + void release_stable_list(); + + void verify_hazard_ptr_scanned(); public: - NestedThreadsList(ThreadsList* t_list) : _t_list(t_list) { - assert(Threads_lock->owned_by_self(), - "must own Threads_lock for saved t_list to be valid."); + // Constructor that attaches the list onto a thread. + SafeThreadsListPtr(Thread *thread, bool acquire) : + _previous(NULL), + _thread(thread), + _list(NULL), + _has_ref_count(false), + _needs_release(false) + { + if (acquire) { + acquire_stable_list(); + } + } + + // Constructor that transfers ownership of the pointer. + SafeThreadsListPtr(SafeThreadsListPtr& other) : + _previous(other._previous), + _thread(other._thread), + _list(other._list), + _has_ref_count(other._has_ref_count), + _needs_release(other._needs_release) + { + other._needs_release = false; } - ThreadsList* t_list() { return _t_list; } - NestedThreadsList* next() { return _next; } - void set_next(NestedThreadsList* value) { _next = value; } + ~SafeThreadsListPtr() { + if (_needs_release) { + release_stable_list(); + } + } + + ThreadsList* list() const { return _list; } + SafeThreadsListPtr* previous() const { return _previous; } + void print_on(outputStream* st); }; // A helper to optionally set the hazard ptr in ourself. This helper can @@ -209,24 +257,20 @@ // class ThreadsListSetter : public StackObj { private: - bool _target_needs_release; // needs release only when set() - Thread * _target; + SafeThreadsListPtr _list_ptr; public: - ThreadsListSetter() : _target_needs_release(false), _target(Thread::current()) { - } - ~ThreadsListSetter(); - ThreadsList* list(); - void set(); - bool target_needs_release() { return _target_needs_release; } + ThreadsListSetter() : _list_ptr(Thread::current(), /* acquire */ false) {} + ThreadsList* list() { return _list_ptr.list(); } + void set() { _list_ptr.acquire_stable_list(); } + bool is_set() { return _list_ptr._needs_release; } }; // This stack allocated ThreadsListHandle keeps all JavaThreads in the // ThreadsList from being deleted until it is safe. // class ThreadsListHandle : public StackObj { - ThreadsList * _list; - Thread *const _self; + SafeThreadsListPtr _list_ptr; elapsedTimer _timer; // Enabled via -XX:+EnableThreadSMRStatistics. public: @@ -234,22 +278,22 @@ ~ThreadsListHandle(); ThreadsList *list() const { - return _list; + return _list_ptr.list(); } template void threads_do(T *cl) const { - return _list->threads_do(cl); + return list()->threads_do(cl); } bool cv_internal_thread_to_JavaThread(jobject jthread, JavaThread ** jt_pp, oop * thread_oop_p); bool includes(JavaThread* p) { - return _list->includes(p); + return list()->includes(p); } uint length() const { - return _list->length(); + return list()->length(); } }; --- old/src/hotspot/share/runtime/threadSMR.inline.hpp 2018-05-01 12:17:23.000000000 -0400 +++ new/src/hotspot/share/runtime/threadSMR.inline.hpp 2018-05-01 12:17:23.000000000 -0400 @@ -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 @@ -77,14 +77,6 @@ } } - -inline ThreadsList* ThreadsListSetter::list() { - ThreadsList *ret = _target->get_threads_hazard_ptr(); - assert(ret != NULL, "hazard ptr should be set"); - assert(!Thread::is_hazard_ptr_tagged(ret), "hazard ptr should be validated"); - return ret; -} - inline ThreadsList* ThreadsSMRSupport::get_java_thread_list() { return (ThreadsList*)OrderAccess::load_acquire(&_java_thread_list); } --- old/src/hotspot/share/services/threadService.hpp 2018-05-01 12:17:29.000000000 -0400 +++ new/src/hotspot/share/services/threadService.hpp 2018-05-01 12:17:29.000000000 -0400 @@ -377,7 +377,7 @@ ThreadSnapshot* snapshots() { return _snapshots; } void set_t_list() { _setter.set(); } ThreadsList* t_list(); - bool t_list_has_been_set() { return _setter.target_needs_release(); } + bool t_list_has_been_set() { return _setter.is_set(); } void oops_do(OopClosure* f); void metadata_do(void f(Metadata*)); }; --- old/test/hotspot/jtreg/runtime/ErrorHandling/NestedThreadsListHandleInErrorHandlingTest.java 2018-05-01 12:17:34.000000000 -0400 +++ new/test/hotspot/jtreg/runtime/ErrorHandling/NestedThreadsListHandleInErrorHandlingTest.java 2018-05-01 12:17:33.000000000 -0400 @@ -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 @@ -85,14 +85,14 @@ Pattern [] pattern = new Pattern[] { // The "Current thread" line should show a hazard ptr and // a nested hazard ptr: - Pattern.compile("Current thread .* _threads_hazard_ptr=0x[0-9A-Fa-f][0-9A-Fa-f]*, _nested_threads_hazard_ptr_cnt=1, _nested_threads_hazard_ptrs=0x.*"), + Pattern.compile("Current thread .* _threads_hazard_ptr=0x[0-9A-Fa-f][0-9A-Fa-f]*, _nested_threads_hazard_ptr_cnt=1, _nested_threads_hazard_ptr=0x[0-9A-Fa-f][0-9A-Fa-f]*.*"), // We should have a section of Threads class SMR info: Pattern.compile("Threads class SMR info:"), // We should have one nested ThreadsListHandle: Pattern.compile(".*, _nested_thread_list_max=1"), // The current thread (marked with '=>') in the threads list - // should show a hazard ptr: - Pattern.compile("=>.* JavaThread \"main\" .*_threads_hazard_ptr=0x[0-9A-Fa-f][0-9A-Fa-f]*, _nested_threads_hazard_ptr_cnt=1, _nested_threads_hazard_ptrs=0x.*"), + // should show a hazard ptr and a nested hazard ptr: + Pattern.compile("=>.* JavaThread \"main\" .* _threads_hazard_ptr=0x[0-9A-Fa-f][0-9A-Fa-f]*, _nested_threads_hazard_ptr_cnt=1, _nested_threads_hazard_ptr=0x[0-9A-Fa-f][0-9A-Fa-f]*.*"), }; int currentPattern = 0; --- old/test/hotspot/jtreg/runtime/ErrorHandling/ThreadsListHandleInErrorHandlingTest.java 2018-05-01 12:17:39.000000000 -0400 +++ new/test/hotspot/jtreg/runtime/ErrorHandling/ThreadsListHandleInErrorHandlingTest.java 2018-05-01 12:17:38.000000000 -0400 @@ -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 @@ -83,13 +83,14 @@ String line = null; Pattern [] pattern = new Pattern[] { - // The "Current thread" line should show a hazard ptr: - Pattern.compile("Current thread .* _threads_hazard_ptr=0x.*"), + // The "Current thread" line should show a hazard ptr + // and no nested hazard ptrs: + Pattern.compile("Current thread .* _threads_hazard_ptr=0x[0-9A-Fa-f][0-9A-Fa-f]*, _nested_threads_hazard_ptr_cnt=0.*"), // We should have a section of Threads class SMR info: Pattern.compile("Threads class SMR info:"), // The current thread (marked with '=>') in the threads list - // should show a hazard ptr: - Pattern.compile("=>.* JavaThread \"main\" .*_threads_hazard_ptr=0x.*"), + // should show a hazard ptr and no nested hazard ptrs: + Pattern.compile("=>.* JavaThread \"main\" .* _threads_hazard_ptr=0x[0-9A-Fa-f][0-9A-Fa-f]*, _nested_threads_hazard_ptr_cnt=0.*"), }; int currentPattern = 0;