Skip to content

Commit c0540ff

Browse files
author
Daniel D. Daugherty
committed
8231627: ThreadsListHandleInErrorHandlingTest.java fails in printing all threads
Reviewed-by: eosterlund, coleenp, pchilanomate, sspitsyn
1 parent 7e01bc9 commit c0540ff

File tree

3 files changed

+86
-63
lines changed

3 files changed

+86
-63
lines changed

‎src/hotspot/share/runtime/threadSMR.cpp‎

Lines changed: 76 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -1106,70 +1106,87 @@ void ThreadsSMRSupport::print_info_on(const Thread* thread, outputStream* st) {
11061106

11071107
// Print Threads class SMR info.
11081108
void ThreadsSMRSupport::print_info_on(outputStream* st) {
1109-
// Only grab the Threads_lock if we don't already own it and if we
1110-
// are not reporting an error.
1111-
// Note: Not grabbing the Threads_lock during error reporting is
1112-
// dangerous because the data structures we want to print can be
1113-
// freed concurrently. However, grabbing the Threads_lock during
1114-
// error reporting can be equally dangerous since this thread might
1115-
// block during error reporting or a nested error could leave the
1116-
// Threads_lock held. The classic no win scenario.
1117-
//
1118-
MutexLocker ml((Threads_lock->owned_by_self() || VMError::is_error_reported()) ? NULL : Threads_lock);
1119-
1120-
st->print_cr("Threads class SMR info:");
1121-
st->print_cr("_java_thread_list=" INTPTR_FORMAT ", length=%u, "
1122-
"elements={", p2i(_java_thread_list),
1123-
_java_thread_list->length());
1124-
print_info_elements_on(st, _java_thread_list);
1125-
st->print_cr("}");
1126-
if (_to_delete_list != NULL) {
1127-
st->print_cr("_to_delete_list=" INTPTR_FORMAT ", length=%u, "
1128-
"elements={", p2i(_to_delete_list),
1129-
_to_delete_list->length());
1130-
print_info_elements_on(st, _to_delete_list);
1109+
bool needs_unlock = false;
1110+
if (Threads_lock->try_lock()) {
1111+
// We were able to grab the Threads_lock which makes things safe for
1112+
// this call, but if we are error reporting, then a nested error
1113+
// could happen with the Threads_lock held.
1114+
needs_unlock = true;
1115+
}
1116+
1117+
ThreadsList* saved_threads_list = NULL;
1118+
{
1119+
ThreadsListHandle tlh; // make the current ThreadsList safe for reporting
1120+
saved_threads_list = tlh.list(); // save for later comparison
1121+
1122+
st->print_cr("Threads class SMR info:");
1123+
st->print_cr("_java_thread_list=" INTPTR_FORMAT ", length=%u, elements={",
1124+
p2i(saved_threads_list), saved_threads_list->length());
1125+
print_info_elements_on(st, saved_threads_list);
11311126
st->print_cr("}");
1132-
for (ThreadsList *t_list = _to_delete_list->next_list();
1133-
t_list != NULL; t_list = t_list->next_list()) {
1134-
st->print("next-> " INTPTR_FORMAT ", length=%u, "
1135-
"elements={", p2i(t_list), t_list->length());
1136-
print_info_elements_on(st, t_list);
1127+
}
1128+
1129+
if (_to_delete_list != NULL) {
1130+
if (Threads_lock->owned_by_self()) {
1131+
// Only safe if we have the Threads_lock.
1132+
st->print_cr("_to_delete_list=" INTPTR_FORMAT ", length=%u, elements={",
1133+
p2i(_to_delete_list), _to_delete_list->length());
1134+
print_info_elements_on(st, _to_delete_list);
11371135
st->print_cr("}");
1136+
for (ThreadsList *t_list = _to_delete_list->next_list();
1137+
t_list != NULL; t_list = t_list->next_list()) {
1138+
st->print("next-> " INTPTR_FORMAT ", length=%u, elements={",
1139+
p2i(t_list), t_list->length());
1140+
print_info_elements_on(st, t_list);
1141+
st->print_cr("}");
1142+
}
1143+
} else {
1144+
st->print_cr("_to_delete_list=" INTPTR_FORMAT, p2i(_to_delete_list));
1145+
st->print_cr("Skipping _to_delete_list fields and contents for safety.");
11381146
}
11391147
}
1140-
if (!EnableThreadSMRStatistics) {
1141-
return;
1142-
}
1143-
st->print_cr("_java_thread_list_alloc_cnt=" UINT64_FORMAT ", "
1144-
"_java_thread_list_free_cnt=" UINT64_FORMAT ", "
1145-
"_java_thread_list_max=%u, "
1146-
"_nested_thread_list_max=%u",
1147-
_java_thread_list_alloc_cnt,
1148-
_java_thread_list_free_cnt,
1149-
_java_thread_list_max,
1150-
_nested_thread_list_max);
1151-
if (_tlh_cnt > 0) {
1152-
st->print_cr("_tlh_cnt=%u"
1153-
", _tlh_times=%u"
1154-
", avg_tlh_time=%0.2f"
1155-
", _tlh_time_max=%u",
1156-
_tlh_cnt, _tlh_times,
1157-
((double) _tlh_times / _tlh_cnt),
1158-
_tlh_time_max);
1148+
if (EnableThreadSMRStatistics) {
1149+
st->print_cr("_java_thread_list_alloc_cnt=" UINT64_FORMAT ", "
1150+
"_java_thread_list_free_cnt=" UINT64_FORMAT ", "
1151+
"_java_thread_list_max=%u, "
1152+
"_nested_thread_list_max=%u",
1153+
_java_thread_list_alloc_cnt,
1154+
_java_thread_list_free_cnt,
1155+
_java_thread_list_max,
1156+
_nested_thread_list_max);
1157+
if (_tlh_cnt > 0) {
1158+
st->print_cr("_tlh_cnt=%u"
1159+
", _tlh_times=%u"
1160+
", avg_tlh_time=%0.2f"
1161+
", _tlh_time_max=%u",
1162+
_tlh_cnt, _tlh_times,
1163+
((double) _tlh_times / _tlh_cnt),
1164+
_tlh_time_max);
1165+
}
1166+
if (_deleted_thread_cnt > 0) {
1167+
st->print_cr("_deleted_thread_cnt=%u"
1168+
", _deleted_thread_times=%u"
1169+
", avg_deleted_thread_time=%0.2f"
1170+
", _deleted_thread_time_max=%u",
1171+
_deleted_thread_cnt, _deleted_thread_times,
1172+
((double) _deleted_thread_times / _deleted_thread_cnt),
1173+
_deleted_thread_time_max);
1174+
}
1175+
st->print_cr("_delete_lock_wait_cnt=%u, _delete_lock_wait_max=%u",
1176+
_delete_lock_wait_cnt, _delete_lock_wait_max);
1177+
st->print_cr("_to_delete_list_cnt=%u, _to_delete_list_max=%u",
1178+
_to_delete_list_cnt, _to_delete_list_max);
11591179
}
1160-
if (_deleted_thread_cnt > 0) {
1161-
st->print_cr("_deleted_thread_cnt=%u"
1162-
", _deleted_thread_times=%u"
1163-
", avg_deleted_thread_time=%0.2f"
1164-
", _deleted_thread_time_max=%u",
1165-
_deleted_thread_cnt, _deleted_thread_times,
1166-
((double) _deleted_thread_times / _deleted_thread_cnt),
1167-
_deleted_thread_time_max);
1180+
if (needs_unlock) {
1181+
Threads_lock->unlock();
1182+
} else {
1183+
if (_java_thread_list != saved_threads_list) {
1184+
st->print_cr("The _java_thread_list has changed from " INTPTR_FORMAT
1185+
" to " INTPTR_FORMAT
1186+
" so some of the above information may be stale.",
1187+
p2i(saved_threads_list), p2i(_java_thread_list));
1188+
}
11681189
}
1169-
st->print_cr("_delete_lock_wait_cnt=%u, _delete_lock_wait_max=%u",
1170-
_delete_lock_wait_cnt, _delete_lock_wait_max);
1171-
st->print_cr("_to_delete_list_cnt=%u, _to_delete_list_max=%u",
1172-
_to_delete_list_cnt, _to_delete_list_max);
11731190
}
11741191

11751192
// Print ThreadsList elements (4 per line).

‎test/hotspot/jtreg/runtime/ErrorHandling/NestedThreadsListHandleInErrorHandlingTest.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -93,8 +93,10 @@ public static void main(String[] args) throws Exception {
9393
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]*.*"),
9494
// We should have a section of Threads class SMR info:
9595
Pattern.compile("Threads class SMR info:"),
96-
// We should have one nested ThreadsListHandle:
97-
Pattern.compile(".*, _nested_thread_list_max=1"),
96+
// We should have had a double nested ThreadsListHandle since
97+
// ThreadsSMRSupport::print_info_on() now protects itself with
98+
// a ThreadsListHandle in addition to what the test creates:
99+
Pattern.compile(".*, _nested_thread_list_max=2"),
98100
// The current thread (marked with '=>') in the threads list
99101
// should show a hazard ptr and a nested hazard ptr:
100102
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]*.*"),

‎test/hotspot/jtreg/runtime/ErrorHandling/ThreadsListHandleInErrorHandlingTest.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -93,6 +93,10 @@ public static void main(String[] args) throws Exception {
9393
Pattern.compile("Current thread .* _threads_hazard_ptr=0x[0-9A-Fa-f][0-9A-Fa-f]*, _nested_threads_hazard_ptr_cnt=0.*"),
9494
// We should have a section of Threads class SMR info:
9595
Pattern.compile("Threads class SMR info:"),
96+
// We should have had a single nested ThreadsListHandle since
97+
// ThreadsSMRSupport::print_info_on() now protects itself with
98+
// a ThreadsListHandle:
99+
Pattern.compile(".*, _nested_thread_list_max=1"),
96100
// The current thread (marked with '=>') in the threads list
97101
// should show a hazard ptr and no nested hazard ptrs:
98102
Pattern.compile("=>.* JavaThread \"main\" .* _threads_hazard_ptr=0x[0-9A-Fa-f][0-9A-Fa-f]*, _nested_threads_hazard_ptr_cnt=0.*"),

0 commit comments

Comments
 (0)