infernap12/jdk
1
0
Fork 0
mirror of https://github.com/openjdk/jdk.git synced 2026-01-28 12:09:14 +00:00
Code Issues Packages Projects Releases Wiki Activity

8264393: JDK-8258284 introduced dangling TLH race

Browse Source 
Reviewed-by: dholmes, rehn, eosterlund
This commit is contained in:
Daniel D. Daugherty 2021-04-03 19:06:34 +00:00
parent 9b2232bc1f
commit f259eeaf65
4 changed files with 44 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
src/hotspot/share/runtime/threadSMR.cpp
View File

@ -417,7 +417,7 @@ void SafeThreadsListPtr::acquire_stable_list() {
_previous = _thread->_threads_list_ptr;
_thread->_threads_list_ptr = this;
if (_thread->get_threads_hazard_ptr() == NULL) {
if (_thread->get_threads_hazard_ptr() == NULL && _previous == NULL) {
// The typical case is first.
acquire_stable_list_fast_path();
return;
@ -484,8 +484,6 @@ void SafeThreadsListPtr::acquire_stable_list_fast_path() {
//
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
@ -506,7 +504,7 @@ void SafeThreadsListPtr::acquire_stable_list_nested_path() {
}
// 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);
_thread->set_threads_hazard_ptr(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();
@ -523,11 +521,26 @@ 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;
// We're releasing either a leaf or nested ThreadsListHandle. In either
// case, we set this thread's hazard ptr back to NULL and we do it before
// _nested_handle_cnt is decremented below.
_thread->set_threads_hazard_ptr(NULL);
if (_previous != NULL) {
// The ThreadsListHandle being released is a nested ThreadsListHandle.
if (EnableThreadSMRStatistics) {
_thread->dec_nested_threads_hazard_ptr_cnt();
}
// The previous ThreadsList is stable because the _nested_handle_cnt is
// > 0, but we cannot safely make it this thread's hazard ptr again.
// The protocol for installing and verifying a ThreadsList as a
// thread's hazard ptr is handled by acquire_stable_list_fast_path().
// And that protocol cannot be properly done with a ThreadsList that
// might not be the current system ThreadsList.
assert(_previous->_list->_nested_handle_cnt > 0, "must be > zero");
}
if (_has_ref_count) {
// This thread created a nested ThreadsListHandle after the current
// ThreadsListHandle so we had to protect this ThreadsList with a
@ -536,23 +549,6 @@ void SafeThreadsListPtr::release_stable_list() {
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));
}
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
// free up some memory temporarily used by a ThreadsList snapshot.

37
test/hotspot/gtest/runtime/test_ThreadsListHandle.cpp
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2020, 2021, 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
@ -213,9 +213,10 @@ TEST_VM(ThreadsListHandle, sanity) {
// Test case: after first nested ThreadsListHandle (tlh2) has been destroyed
// Verify the current thread refers to tlh1:
EXPECT_EQ(ThreadsListHandleTest::get_Thread_threads_hazard_ptr(thr), tlh1.list())
<< "thr->_threads_hazard_ptr must match tlh1.list()";
// Verify the current thread's hazard ptr is NULL:
EXPECT_EQ(ThreadsListHandleTest::get_Thread_threads_hazard_ptr(thr), (ThreadsList*)NULL)
<< "thr->_threads_hazard_ptr must be NULL";
// Verify the current thread's threads list ptr refers to tlh1:
EXPECT_EQ(ThreadsListHandleTest::get_Thread_threads_list_ptr(thr), list_ptr1)
<< "thr->_threads_list_ptr must match list_ptr1";
EXPECT_EQ(ThreadsListHandleTest::get_Thread_nested_threads_hazard_ptr_cnt(thr), (uint)0)
@ -396,9 +397,10 @@ TEST_VM(ThreadsListHandle, sanity) {
// Test case: after double nested ThreadsListHandle (tlh3) has been destroyed
// Verify the current thread refers to tlh2:
EXPECT_EQ(ThreadsListHandleTest::get_Thread_threads_hazard_ptr(thr), tlh2.list())
<< "thr->_threads_hazard_ptr must match tlh2.list()";
// Verify the current thread's hazard ptr is NULL:
EXPECT_EQ(ThreadsListHandleTest::get_Thread_threads_hazard_ptr(thr), (ThreadsList*)NULL)
<< "thr->_threads_hazard_ptr must be NULL";
// Verify the current thread's threads list ptr refers to tlh2:
EXPECT_EQ(tlh1.list(), tlh2.list())
<< "tlh1.list() must match tlh2.list()";
EXPECT_EQ(ThreadsListHandleTest::get_Thread_threads_list_ptr(thr), list_ptr2)
@ -443,9 +445,10 @@ TEST_VM(ThreadsListHandle, sanity) {
// Test case: after first nested ThreadsListHandle (tlh2) has been destroyed
// Verify the current thread refers to tlh1:
EXPECT_EQ(ThreadsListHandleTest::get_Thread_threads_hazard_ptr(thr), tlh1.list())
<< "thr->_threads_hazard_ptr must match tlh1.list()";
// Verify the current thread's hazard ptr is NULL:
EXPECT_EQ(ThreadsListHandleTest::get_Thread_threads_hazard_ptr(thr), (ThreadsList*)NULL)
<< "thr->_threads_hazard_ptr must be NULL";
// Verify the current thread's threads list ptr refers to tlh1:
EXPECT_EQ(ThreadsListHandleTest::get_Thread_threads_list_ptr(thr), list_ptr1)
<< "thr->_threads_list_ptr must match list_ptr1";
EXPECT_EQ(ThreadsListHandleTest::get_Thread_nested_threads_hazard_ptr_cnt(thr), (uint)0)
@ -562,9 +565,10 @@ TEST_VM(ThreadsListHandle, sanity) {
// Test case: after first back-to-back nested ThreadsListHandle (tlh2a) has been destroyed
// Verify the current thread refers to tlh1:
EXPECT_EQ(ThreadsListHandleTest::get_Thread_threads_hazard_ptr(thr), tlh1.list())
<< "thr->_threads_hazard_ptr must match tlh1.list()";
// Verify the current thread's hazard ptr is NULL:
EXPECT_EQ(ThreadsListHandleTest::get_Thread_threads_hazard_ptr(thr), (ThreadsList*)NULL)
<< "thr->_threads_hazard_ptr must be NULL";
// Verify the current thread's threads list ptr refers to tlh1:
EXPECT_EQ(ThreadsListHandleTest::get_Thread_threads_list_ptr(thr), list_ptr1)
<< "thr->_threads_list_ptr must match list_ptr1";
EXPECT_EQ(ThreadsListHandleTest::get_Thread_nested_threads_hazard_ptr_cnt(thr), (uint)0)
@ -639,9 +643,10 @@ TEST_VM(ThreadsListHandle, sanity) {
// Test case: after second back-to-back nested ThreadsListHandle (tlh2b) has been destroyed
// Verify the current thread refers to tlh1:
EXPECT_EQ(ThreadsListHandleTest::get_Thread_threads_hazard_ptr(thr), tlh1.list())
<< "thr->_threads_hazard_ptr must match tlh1.list()";
// Verify the current thread's hazard ptr is NULL:
EXPECT_EQ(ThreadsListHandleTest::get_Thread_threads_hazard_ptr(thr), (ThreadsList*)NULL)
<< "thr->_threads_hazard_ptr must be NULL";
// Verify the current thread's threads list ptr refers to tlh1:
EXPECT_EQ(ThreadsListHandleTest::get_Thread_threads_list_ptr(thr), list_ptr1)
<< "thr->_threads_list_ptr must match list_ptr1";
EXPECT_EQ(ThreadsListHandleTest::get_Thread_nested_threads_hazard_ptr_cnt(thr), (uint)0)

4
test/hotspot/jtreg/runtime/ErrorHandling/NestedThreadsListHandleInErrorHandlingTest.java
View File

@ -98,8 +98,8 @@ public class NestedThreadsListHandleInErrorHandlingTest {
// a ThreadsListHandle in addition to what the test creates:
Pattern.compile(".*, _nested_thread_list_max=2"),
// The current thread (marked with '=>') in the threads list
// 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]*.*"),
// should show a nested hazard ptr:
Pattern.compile("=>.* JavaThread \"main\" .*, _nested_threads_hazard_ptr_cnt=1, _nested_threads_hazard_ptr=0x[0-9A-Fa-f][0-9A-Fa-f]*.*"),
};
int currentPattern = 0;

4
test/hotspot/jtreg/runtime/ErrorHandling/ThreadsListHandleInErrorHandlingTest.java
View File

@ -98,8 +98,8 @@ public class ThreadsListHandleInErrorHandlingTest {
// a ThreadsListHandle:
Pattern.compile(".*, _nested_thread_list_max=1"),
// The current thread (marked with '=>') in the threads list
// 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.*"),
// should show no nested hazard ptrs:
Pattern.compile("=>.* JavaThread \"main\" .*, _nested_threads_hazard_ptr_cnt=0.*"),
};
int currentPattern = 0;