Skip to content

Commit

Permalink
8258284: clean up issues with nested ThreadsListHandles
Browse files Browse the repository at this point in the history
Reviewed-by: eosterlund, rehn
  • Loading branch information
Daniel D. Daugherty committed Dec 22, 2020
1 parent 3df6ec2 commit 172af15
Show file tree
Hide file tree
Showing 4 changed files with 715 additions and 15 deletions.
1 change: 1 addition & 0 deletions src/hotspot/share/runtime/thread.hpp
Expand Up @@ -185,6 +185,7 @@ class Thread: public ThreadShadow {
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 ThreadsSMRSupport; // for _nested_threads_hazard_ptr_cnt, _threads_hazard_ptr, _threads_list_ptr access
friend class ThreadsListHandleTest; // for _nested_threads_hazard_ptr_cnt, _threads_hazard_ptr, _threads_list_ptr access

ThreadsList* volatile _threads_hazard_ptr;
SafeThreadsListPtr* _threads_list_ptr;
Expand Down
44 changes: 29 additions & 15 deletions src/hotspot/share/runtime/threadSMR.cpp
Expand Up @@ -466,9 +466,14 @@ void SafeThreadsListPtr::acquire_stable_list_nested_path() {
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 (!_previous->_has_ref_count) {
// Promote the thread's current SafeThreadsListPtr to be reference counted.
current_list->inc_nested_handle_cnt();
_previous->_has_ref_count = true;
}
// Clear the hazard ptr so we can go through the fast path below and
// acquire a nested stable ThreadsList.
Atomic::store(&_thread->_threads_hazard_ptr, (ThreadsList*)NULL);

if (EnableThreadSMRStatistics && _thread->nested_threads_hazard_ptr_cnt() > ThreadsSMRSupport::_nested_thread_list_max) {
ThreadsSMRSupport::_nested_thread_list_max = _thread->nested_threads_hazard_ptr_cnt();
Expand All @@ -485,26 +490,35 @@ void SafeThreadsListPtr::acquire_stable_list_nested_path() {
//
void SafeThreadsListPtr::release_stable_list() {
assert(_thread != NULL, "sanity check");
assert(_thread->get_threads_hazard_ptr() != NULL, "sanity check");
assert(_thread->get_threads_hazard_ptr() == _list, "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();
}
// This thread created a nested ThreadsListHandle after the current
// ThreadsListHandle so we had to protect this ThreadsList with a
// ref count. We no longer need that protection.
_list->dec_nested_handle_cnt();

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");
}
if (_previous == NULL) {
// The ThreadsListHandle being released is a leaf ThreadsListHandle.
// This is the "normal" case and this is where we set this thread's
// hazard ptr back to NULL.
_thread->set_threads_hazard_ptr(NULL);
} else {
// The ThreadsListHandle being released is a nested ThreadsListHandle.
if (EnableThreadSMRStatistics) {
_thread->dec_nested_threads_hazard_ptr_cnt();
}
// The previous ThreadsList becomes this thread's hazard ptr again.
// It is a stable ThreadsList since the non-zero _nested_handle_cnt
// keeps it from being freed so we can just set the thread's hazard
// ptr without going through the stabilization/tagging protocol.
assert(_previous->_list->_nested_handle_cnt > 0, "must be > than zero");
_thread->set_threads_hazard_ptr(_previous->_list);
}

// After releasing the hazard ptr, other threads may go ahead and
Expand Down
4 changes: 4 additions & 0 deletions src/hotspot/share/runtime/threadSMR.hpp
Expand Up @@ -165,6 +165,7 @@ class ThreadsList : public CHeapObj<mtThread> {
friend class VMStructs;
friend class SafeThreadsListPtr; // for {dec,inc}_nested_handle_cnt() access
friend class ThreadsSMRSupport; // for _nested_handle_cnt, {add,remove}_thread(), {,set_}next_list() access
friend class ThreadsListHandleTest; // for _nested_handle_cnt access

const uint _length;
ThreadsList* _next_list;
Expand Down Expand Up @@ -206,6 +207,7 @@ class ThreadsList : public CHeapObj<mtThread> {
// 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 ThreadsListHandleTest; // for access to the fields
friend class ThreadsListSetter;

SafeThreadsListPtr* _previous;
Expand Down Expand Up @@ -277,6 +279,8 @@ class ThreadsListSetter : public StackObj {
// ThreadsList from being deleted until it is safe.
//
class ThreadsListHandle : public StackObj {
friend class ThreadsListHandleTest; // for _list_ptr access

SafeThreadsListPtr _list_ptr;
elapsedTimer _timer; // Enabled via -XX:+EnableThreadSMRStatistics.

Expand Down

1 comment on commit 172af15

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.