Skip to content
Permalink
Browse files
8264123: add ThreadsList.is_valid() support
Reviewed-by: dholmes, eosterlund, rehn
  • Loading branch information
Daniel D. Daugherty committed Apr 3, 2021
1 parent e8eda65 commit 9b2232bc1fd7dd9af7d475136870bc47b52ef4e7
@@ -173,11 +173,12 @@ class Thread: public ThreadShadow {
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
friend class ValidateHazardPtrsClosure; // for get_threads_hazard_ptr(), untag_hazard_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();
ThreadsList* get_threads_hazard_ptr() const;
void set_threads_hazard_ptr(ThreadsList* new_list);
static bool is_hazard_ptr_tagged(ThreadsList* list) {
return (intptr_t(list) & intptr_t(1)) == intptr_t(1);
@@ -87,7 +87,7 @@ inline ThreadsList* Thread::cmpxchg_threads_hazard_ptr(ThreadsList* exchange_val
return (ThreadsList*)Atomic::cmpxchg(&_threads_hazard_ptr, compare_value, exchange_value);
}

inline ThreadsList* Thread::get_threads_hazard_ptr() {
inline ThreadsList* Thread::get_threads_hazard_ptr() const {
return (ThreadsList*)Atomic::load_acquire(&_threads_hazard_ptr);
}

@@ -282,6 +282,9 @@ class ScanHazardPtrGatherProtectedThreadsClosure : public ThreadClosure {
if (thread->cmpxchg_threads_hazard_ptr(NULL, current_list) == current_list) return;
}

guarantee(ThreadsList::is_valid(current_list), "current_list="
INTPTR_FORMAT " is not valid!", p2i(current_list));

// The current JavaThread has a hazard ptr (ThreadsList reference)
// which might be _java_thread_list or it might be an older
// ThreadsList that has been removed but not freed. In either case,
@@ -294,6 +297,10 @@ class ScanHazardPtrGatherProtectedThreadsClosure : public ThreadClosure {

// Closure to gather hazard ptrs (ThreadsList references) into a hash table.
//
// Since this closure gathers hazard ptrs that may be tagged, this hash
// table of hazard ptrs should only be used for value comparison and not
// traversal of the ThreadsList.
//
class ScanHazardPtrGatherThreadsListClosure : public ThreadClosure {
private:
ThreadScanHashtable *_table;
@@ -304,18 +311,23 @@ class ScanHazardPtrGatherThreadsListClosure : public ThreadClosure {
assert_locked_or_safepoint(Threads_lock);

if (thread == NULL) return;
ThreadsList *threads = thread->get_threads_hazard_ptr();
if (threads == NULL) {
return;
ThreadsList *hazard_ptr = thread->get_threads_hazard_ptr();
if (hazard_ptr == NULL) return;
if (!Thread::is_hazard_ptr_tagged(hazard_ptr)) {
// We only validate hazard_ptrs that are not tagged since a tagged
// hazard ptr can be deleted at any time.
guarantee(ThreadsList::is_valid(hazard_ptr), "hazard_ptr=" INTPTR_FORMAT
" for thread=" INTPTR_FORMAT " is not valid!", p2i(hazard_ptr),
p2i(thread));
}
// In this closure we always ignore the tag that might mark this
// hazard ptr as not yet verified. If we happen to catch an
// unverified hazard ptr that is subsequently discarded (not
// published), then the only side effect is that we might keep a
// to-be-deleted ThreadsList alive a little longer.
threads = Thread::untag_hazard_ptr(threads);
if (!_table->has_entry((void*)threads)) {
_table->add_entry((void*)threads);
hazard_ptr = Thread::untag_hazard_ptr(hazard_ptr);
if (!_table->has_entry((void*)hazard_ptr)) {
_table->add_entry((void*)hazard_ptr);
}
}
};
@@ -355,6 +367,27 @@ class ScanHazardPtrPrintMatchingThreadsClosure : public ThreadClosure {
}
};

// Closure to validate hazard ptrs.
//
class ValidateHazardPtrsClosure : public ThreadClosure {
public:
ValidateHazardPtrsClosure() {};

virtual void do_thread(Thread* thread) {
assert_locked_or_safepoint(Threads_lock);

if (thread == NULL) return;
ThreadsList *hazard_ptr = thread->get_threads_hazard_ptr();
if (hazard_ptr == NULL) return;
// If the hazard ptr is unverified, then ignore it since it could
// be deleted at any time now.
if (Thread::is_hazard_ptr_tagged(hazard_ptr)) return;
guarantee(ThreadsList::is_valid(hazard_ptr), "hazard_ptr=" INTPTR_FORMAT
" for thread=" INTPTR_FORMAT " is not valid!", p2i(hazard_ptr),
p2i(thread));
}
};

// Closure to determine if the specified JavaThread is found by
// threads_do().
//
@@ -403,7 +436,7 @@ void SafeThreadsListPtr::acquire_stable_list_fast_path() {
ThreadsList* threads;

// Stable recording of a hazard ptr for SMR. This code does not use
// locks so its use of the _smr_java_thread_list & _threads_hazard_ptr
// locks so its use of the _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.
//
@@ -419,7 +452,7 @@ void SafeThreadsListPtr::acquire_stable_list_fast_path() {
ThreadsList* unverified_threads = Thread::tag_hazard_ptr(threads);
_thread->set_threads_hazard_ptr(unverified_threads);

// If _smr_java_thread_list has changed, we have lost a race with
// If _java_thread_list has changed, we have lost a race with
// Threads::add() or Threads::remove() and have to try again.
if (ThreadsSMRSupport::get_java_thread_list() != threads) {
continue;
@@ -530,6 +563,10 @@ void SafeThreadsListPtr::release_stable_list() {
// An exiting thread might be waiting in smr_delete(); we need to
// check with delete_lock to be sure.
ThreadsSMRSupport::release_stable_list_wake_up(_has_ref_count);
guarantee(_previous == NULL || ThreadsList::is_valid(_previous->_list),
"_previous->_list=" INTPTR_FORMAT
" is not valid after calling release_stable_list_wake_up!",
p2i(_previous->_list));
}
}

@@ -601,6 +638,7 @@ static JavaThread* const* make_threads_list_data(int entries) {
}

ThreadsList::ThreadsList(int entries) :
_magic(THREADS_LIST_MAGIC),
_length(entries),
_next_list(NULL),
_threads(make_threads_list_data(entries)),
@@ -611,6 +649,7 @@ ThreadsList::~ThreadsList() {
if (_threads != empty_threads_list_data) {
FREE_C_HEAP_ARRAY(JavaThread*, _threads);
}
_magic = 0xDEADBEEF;
}

// Add a JavaThread to a ThreadsList. The returned ThreadsList is a
@@ -882,6 +921,9 @@ void ThreadsSMRSupport::free_list(ThreadsList* threads) {
log_debug(thread, smr)("tid=" UINTX_FORMAT ": ThreadsSMRSupport::free_list: threads=" INTPTR_FORMAT " is not freed.", os::current_thread_id(), p2i(threads));
}

ValidateHazardPtrsClosure validate_cl;
threads_do(&validate_cl);

delete scan_table;
}

@@ -1096,8 +1138,9 @@ void ThreadsSMRSupport::log_statistics() {

// 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));
ThreadsList* hazard_ptr = thread->get_threads_hazard_ptr();
if (hazard_ptr != NULL) {
st->print(" _threads_hazard_ptr=" INTPTR_FORMAT, p2i(hazard_ptr));
}
if (EnableThreadSMRStatistics && thread->_threads_list_ptr != NULL) {
// The count is only interesting if we have a _threads_list_ptr.
@@ -162,11 +162,13 @@ class ThreadsSMRSupport : AllStatic {
// A fast list of JavaThreads.
//
class ThreadsList : public CHeapObj<mtThread> {
enum { THREADS_LIST_MAGIC = (int)(('T' << 24) | ('L' << 16) | ('S' << 8) | 'T') };
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

uint _magic;
const uint _length;
ThreadsList* _next_list;
JavaThread *const *const _threads;
@@ -203,6 +205,8 @@ class ThreadsList : public CHeapObj<mtThread> {
int find_index_of_JavaThread(JavaThread* target);
JavaThread* find_JavaThread_from_java_tid(jlong java_tid) const;
bool includes(const JavaThread * const p) const;

static bool is_valid(ThreadsList* list) { return list->_magic == THREADS_LIST_MAGIC; }
};

// An abstract safe ptr to a ThreadsList comprising either a stable hazard ptr

1 comment on commit 9b2232b

@openjdk-notifier

This comment has been minimized.

Copy link

@openjdk-notifier openjdk-notifier bot commented on 9b2232b Apr 3, 2021

Please sign in to comment.