8021335: Missing synchronization when reading counters for live threads and peak thread count

Reviewed-by: dholmes, mchung
This commit is contained in:
Dean Long 2018-10-25 18:41:26 -07:00
parent 7533f9f78a
commit c24f6506e7
4 changed files with 96 additions and 47 deletions

View File

@ -1819,6 +1819,9 @@ static void ensure_join(JavaThread* thread) {
thread->clear_pending_exception();
}
static bool is_daemon(oop threadObj) {
return (threadObj != NULL && java_lang_Thread::is_daemon(threadObj));
}
// For any new cleanup additions, please check to see if they need to be applied to
// cleanup_failed_attach_current_thread as well.
@ -1910,7 +1913,7 @@ void JavaThread::exit(bool destroy_vm, ExitType exit_type) {
MutexLockerEx ml(SR_lock(), Mutex::_no_safepoint_check_flag);
if (!is_external_suspend()) {
set_terminated(_thread_exiting);
ThreadService::current_thread_exiting(this);
ThreadService::current_thread_exiting(this, is_daemon(threadObj()));
break;
}
// Implied else:
@ -1930,6 +1933,7 @@ void JavaThread::exit(bool destroy_vm, ExitType exit_type) {
}
// no more external suspends are allowed at this point
} else {
assert(!is_terminated() && !is_exiting(), "must not be exiting");
// before_exit() has already posted JVMTI THREAD_END events
}
@ -4332,7 +4336,7 @@ jboolean Threads::is_supported_jni_version(jint version) {
void Threads::add(JavaThread* p, bool force_daemon) {
// The threads lock must be owned at this point
assert_locked_or_safepoint(Threads_lock);
assert(Threads_lock->owned_by_self(), "must have threads lock");
BarrierSet::barrier_set()->on_thread_attach(p);
@ -4348,7 +4352,7 @@ void Threads::add(JavaThread* p, bool force_daemon) {
bool daemon = true;
// Bootstrapping problem: threadObj can be null for initial
// JavaThread (or for threads attached via JNI)
if ((!force_daemon) && (threadObj == NULL || !java_lang_Thread::is_daemon(threadObj))) {
if ((!force_daemon) && !is_daemon((threadObj))) {
_number_of_non_daemon_threads++;
daemon = false;
}
@ -4393,7 +4397,7 @@ void Threads::remove(JavaThread* p) {
_number_of_threads--;
oop threadObj = p->threadObj();
bool daemon = true;
if (threadObj == NULL || !java_lang_Thread::is_daemon(threadObj)) {
if (!is_daemon(threadObj)) {
_number_of_non_daemon_threads--;
daemon = false;

View File

@ -57,8 +57,8 @@ PerfCounter* ThreadService::_total_threads_count = NULL;
PerfVariable* ThreadService::_live_threads_count = NULL;
PerfVariable* ThreadService::_peak_threads_count = NULL;
PerfVariable* ThreadService::_daemon_threads_count = NULL;
volatile int ThreadService::_exiting_threads_count = 0;
volatile int ThreadService::_exiting_daemon_threads_count = 0;
volatile int ThreadService::_atomic_threads_count = 0;
volatile int ThreadService::_atomic_daemon_threads_count = 0;
ThreadDumpResult* ThreadService::_threaddump_list = NULL;
@ -101,50 +101,104 @@ void ThreadService::reset_peak_thread_count() {
_peak_threads_count->set_value(get_live_thread_count());
}
static bool is_hidden_thread(JavaThread *thread) {
// hide VM internal or JVMTI agent threads
return thread->is_hidden_from_external_view() || thread->is_jvmti_agent_thread();
}
void ThreadService::add_thread(JavaThread* thread, bool daemon) {
// Do not count VM internal or JVMTI agent threads
if (thread->is_hidden_from_external_view() ||
thread->is_jvmti_agent_thread()) {
assert(Threads_lock->owned_by_self(), "must have threads lock");
// Do not count hidden threads
if (is_hidden_thread(thread)) {
return;
}
_total_threads_count->inc();
_live_threads_count->inc();
Atomic::inc(&_atomic_threads_count);
int count = _atomic_threads_count;
if (_live_threads_count->get_value() > _peak_threads_count->get_value()) {
_peak_threads_count->set_value(_live_threads_count->get_value());
if (count > _peak_threads_count->get_value()) {
_peak_threads_count->set_value(count);
}
if (daemon) {
_daemon_threads_count->inc();
Atomic::inc(&_atomic_daemon_threads_count);
}
}
void ThreadService::decrement_thread_counts(JavaThread* jt, bool daemon) {
Atomic::dec(&_atomic_threads_count);
if (daemon) {
Atomic::dec(&_atomic_daemon_threads_count);
}
}
void ThreadService::remove_thread(JavaThread* thread, bool daemon) {
Atomic::dec(&_exiting_threads_count);
if (daemon) {
Atomic::dec(&_exiting_daemon_threads_count);
}
assert(Threads_lock->owned_by_self(), "must have threads lock");
if (thread->is_hidden_from_external_view() ||
thread->is_jvmti_agent_thread()) {
// Do not count hidden threads
if (is_hidden_thread(thread)) {
return;
}
_live_threads_count->set_value(_live_threads_count->get_value() - 1);
if (daemon) {
_daemon_threads_count->set_value(_daemon_threads_count->get_value() - 1);
assert(!thread->is_terminated(), "must not be terminated");
if (!thread->is_exiting()) {
// JavaThread::exit() skipped calling current_thread_exiting()
decrement_thread_counts(thread, daemon);
}
int daemon_count = _atomic_daemon_threads_count;
int count = _atomic_threads_count;
// Counts are incremented at the same time, but atomic counts are
// decremented earlier than perf counts.
assert(_live_threads_count->get_value() > count,
"thread count mismatch %d : %d",
(int)_live_threads_count->get_value(), count);
_live_threads_count->dec(1);
if (daemon) {
assert(_daemon_threads_count->get_value() > daemon_count,
"thread count mismatch %d : %d",
(int)_daemon_threads_count->get_value(), daemon_count);
_daemon_threads_count->dec(1);
}
// Counts are incremented at the same time, but atomic counts are
// decremented earlier than perf counts.
assert(_daemon_threads_count->get_value() >= daemon_count,
"thread count mismatch %d : %d",
(int)_daemon_threads_count->get_value(), daemon_count);
assert(_live_threads_count->get_value() >= count,
"thread count mismatch %d : %d",
(int)_live_threads_count->get_value(), count);
assert(_live_threads_count->get_value() > 0 ||
(_live_threads_count->get_value() == 0 && count == 0 &&
_daemon_threads_count->get_value() == 0 && daemon_count == 0),
"thread counts should reach 0 at the same time, live %d,%d daemon %d,%d",
(int)_live_threads_count->get_value(), count,
(int)_daemon_threads_count->get_value(), daemon_count);
assert(_daemon_threads_count->get_value() > 0 ||
(_daemon_threads_count->get_value() == 0 && daemon_count == 0),
"thread counts should reach 0 at the same time, daemon %d,%d",
(int)_daemon_threads_count->get_value(), daemon_count);
}
void ThreadService::current_thread_exiting(JavaThread* jt) {
assert(jt == JavaThread::current(), "Called by current thread");
Atomic::inc(&_exiting_threads_count);
oop threadObj = jt->threadObj();
if (threadObj != NULL && java_lang_Thread::is_daemon(threadObj)) {
Atomic::inc(&_exiting_daemon_threads_count);
void ThreadService::current_thread_exiting(JavaThread* jt, bool daemon) {
// Do not count hidden threads
if (is_hidden_thread(jt)) {
return;
}
assert(jt == JavaThread::current(), "Called by current thread");
assert(!jt->is_terminated() && jt->is_exiting(), "must be exiting");
decrement_thread_counts(jt, daemon);
}
// FIXME: JVMTI should call this function

View File

@ -58,10 +58,12 @@ private:
static PerfVariable* _peak_threads_count;
static PerfVariable* _daemon_threads_count;
// These 2 counters are atomically incremented once the thread is exiting.
// They will be atomically decremented when ThreadService::remove_thread is called.
static volatile int _exiting_threads_count;
static volatile int _exiting_daemon_threads_count;
// These 2 counters are like the above thread counts, but are
// atomically decremented in ThreadService::current_thread_exiting instead of
// ThreadService::remove_thread, so that the thread count is updated before
// Thread.join() returns.
static volatile int _atomic_threads_count;
static volatile int _atomic_daemon_threads_count;
static bool _thread_monitoring_contention_enabled;
static bool _thread_cpu_time_enabled;
@ -72,11 +74,13 @@ private:
// requested by multiple threads concurrently.
static ThreadDumpResult* _threaddump_list;
static void decrement_thread_counts(JavaThread* jt, bool daemon);
public:
static void init();
static void add_thread(JavaThread* thread, bool daemon);
static void remove_thread(JavaThread* thread, bool daemon);
static void current_thread_exiting(JavaThread* jt);
static void current_thread_exiting(JavaThread* jt, bool daemon);
static bool set_thread_monitoring_contention(bool flag);
static bool is_thread_monitoring_contention() { return _thread_monitoring_contention_enabled; }
@ -89,11 +93,8 @@ public:
static jlong get_total_thread_count() { return _total_threads_count->get_value(); }
static jlong get_peak_thread_count() { return _peak_threads_count->get_value(); }
static jlong get_live_thread_count() { return _live_threads_count->get_value() - _exiting_threads_count; }
static jlong get_daemon_thread_count() { return _daemon_threads_count->get_value() - _exiting_daemon_threads_count; }
static int exiting_threads_count() { return _exiting_threads_count; }
static int exiting_daemon_threads_count() { return _exiting_daemon_threads_count; }
static jlong get_live_thread_count() { return _atomic_threads_count; }
static jlong get_daemon_thread_count() { return _atomic_daemon_threads_count; }
// Support for thread dump
static void add_thread_dump(ThreadDumpResult* dump);

View File

@ -158,10 +158,6 @@ public class ResetPeakThreadCount {
" Expected to be == previous peak = " + peak1 + " + " +
delta);
}
// wait until the current thread count gets incremented
while (mbean.getThreadCount() < (current + count)) {
Thread.sleep(100);
}
current = mbean.getThreadCount();
System.out.println(" Live thread count before returns " + current);
return current;
@ -195,12 +191,6 @@ public class ResetPeakThreadCount {
allThreads[i].join();
}
// there is a race in the counter update logic somewhere causing
// the thread counters go ff
// we need to give the terminated threads some extra time to really die
// JDK-8021335
Thread.sleep(500);
long current = mbean.getThreadCount();
System.out.println(" Live thread count before returns " + current);
return current;