JDK-8258284 : clean up issues with nested ThreadsListHandles
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 16
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2020-12-14
  • Updated: 2022-06-27
  • Resolved: 2020-12-22
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 17
17 b03Fixed
Related Reports
Blocks :  
Relates :  
Relates :  
Relates :  
Description
While working on the fix for the following:

    JDK-8231627 runtime/ErrorHandling/ThreadsListHandleInErrorHandlingTest.java fails because error occurred during printing all threads

I ran into some issues with nested ThreadsListHandles.
More details to follow.
Comments
Changeset: 172af152 Author: Daniel D. Daugherty <dcubed@openjdk.org> Date: 2020-12-22 14:07:43 +0000 URL: https://git.openjdk.java.net/jdk/commit/172af152
22-12-2020

The nested ThreadsListHandle code was redone by: JDK-8191798 redo nested ThreadsListHandle to drop Threads_lock which was integrated in jdk11-b12.
18-12-2020

Although the new test reports a total of 10 failures, there are really only two bugs here: 1) A failure to restore the expected _threads_hazard_ptr when a nested ThreadsListHandle is destroyed: > ./open/test/hotspot/gtest/runtime/test_ThreadsListHandle.cpp:217: Failure > Expected equality of these values: > ThreadsListHandleTest::get_Thread_threads_hazard_ptr(thr) > Which is: NULL > tlh1.list() > Which is: 0x7ff303522800 > thr->_threads_hazard_ptr must match tlh1.list() The test code: // 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()"; This test was expecting the destruction of tlh2 to restore the thread's hazard ptr to tlh1.list(), but that didn't happen. 2) A failure to decrement _nested_threads_hazard_ptr_cnt when a nested ThreadsListHandle is destroyed: > ./open/test/hotspot/gtest/runtime/test_ThreadsListHandle.cpp:221: Failure > Expected equality of these values: > ThreadsListHandleTest::get_Thread_nested_threads_hazard_ptr_cnt(thr) > Which is: 1 > (uint)0 > Which is: 0 > thr->_nested_threads_hazard_ptr_cnt must be 0 The test code: // Test case: after first nested ThreadsListHandle (tlh2) has been destroyed <snip> EXPECT_EQ(ThreadsListHandleTest::get_Thread_nested_threads_hazard_ptr_cnt(thr), (uint)0) << "thr->_nested_threads_hazard_ptr_cnt must be 0"; This test was expecting the destruction of tlh2 to decrement the thread's nested_threads_hazard_ptr_cnt field from 1 to 0, but that didn't happen.
17-12-2020

Here's an analysis of the test failures: $ cat ~/Projects/8258284/8258284-baseline-failure-analysis Here are the failures from the 'release' bits run with analysis added: > [----------] 1 test from ThreadsListHandle > [ RUN ] ThreadsListHandle.sanity_vm > ./open/test/hotspot/gtest/runtime/test_ThreadsListHandle.cpp:217: Failure > Expected equality of these values: > ThreadsListHandleTest::get_Thread_threads_hazard_ptr(thr) > Which is: NULL > tlh1.list() > Which is: 0x7ff303522800 > thr->_threads_hazard_ptr must match tlh1.list() The test code: // 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()"; This test was expecting the destruction of tlh2 to restore the thread's hazard ptr to tlh1.list(), but that didn't happen. > ./open/test/hotspot/gtest/runtime/test_ThreadsListHandle.cpp:221: Failure > Expected equality of these values: > ThreadsListHandleTest::get_Thread_nested_threads_hazard_ptr_cnt(thr) > Which is: 1 > (uint)0 > Which is: 0 > thr->_nested_threads_hazard_ptr_cnt must be 0 The test code: // Test case: after first nested ThreadsListHandle (tlh2) has been destroyed <snip> EXPECT_EQ(ThreadsListHandleTest::get_Thread_nested_threads_hazard_ptr_cnt(thr), (uint)0) << "thr->_nested_threads_hazard_ptr_cnt must be 0"; This test was expecting the destruction of tlh2 to decrement the thread's nested_threads_hazard_ptr_cnt field from 1 to 0, but that didn't happen. > ./open/test/hotspot/gtest/runtime/test_ThreadsListHandle.cpp:400: Failure > Expected equality of these values: > ThreadsListHandleTest::get_Thread_threads_hazard_ptr(thr) > Which is: NULL > tlh2.list() > Which is: 0x7ff303522800 > thr->_threads_hazard_ptr must match tlh2.list() The test code: // 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()"; This test was expecting the destruction of tlh3 to restore the thread's hazard ptr to tlh2.list(), but that didn't happen. > ./open/test/hotspot/gtest/runtime/test_ThreadsListHandle.cpp:408: Failure > Expected equality of these values: > ThreadsListHandleTest::get_Thread_nested_threads_hazard_ptr_cnt(thr) > Which is: 2 > (uint)1 > Which is: 1 > thr->_nested_threads_hazard_ptr_cnt must be 1 The test code: // Test case: after double nested ThreadsListHandle (tlh3) has been destroyed <snip> EXPECT_EQ(ThreadsListHandleTest::get_Thread_nested_threads_hazard_ptr_cnt(thr), (uint)1) << "thr->_nested_threads_hazard_ptr_cnt must be 1"; This test was expecting the destruction of tlh3 to decrement the thread's nested_threads_hazard_ptr_cnt field from 2 to 1, but that didn't happen. > ./open/test/hotspot/gtest/runtime/test_ThreadsListHandle.cpp:447: Failure > Expected equality of these values: > ThreadsListHandleTest::get_Thread_threads_hazard_ptr(thr) > Which is: NULL > tlh1.list() > Which is: 0x7ff303522800 > thr->_threads_hazard_ptr must match tlh1.list() The test code: // 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()"; This test was expecting the destruction of tlh2 to restore the thread's hazard ptr to tlh1.list(), but that didn't happen. > ./open/test/hotspot/gtest/runtime/test_ThreadsListHandle.cpp:451: Failure > Expected equality of these values: > ThreadsListHandleTest::get_Thread_nested_threads_hazard_ptr_cnt(thr) > Which is: 1 > (uint)0 > Which is: 0 > thr->_nested_threads_hazard_ptr_cnt must be 0 The test code: // Test case: after first nested ThreadsListHandle (tlh2) has been destroyed<snip> EXPECT_EQ(ThreadsListHandleTest::get_Thread_nested_threads_hazard_ptr_cnt(thr), (uint)0) << "thr->_nested_threads_hazard_ptr_cnt must be 0"; This test was expecting the destruction of tlh2 to decrement the thread's nested_threads_hazard_ptr_cnt field from 1 to 0, but that didn't happen. > ./open/test/hotspot/gtest/runtime/test_ThreadsListHandle.cpp:566: Failure > Expected equality of these values: > ThreadsListHandleTest::get_Thread_threads_hazard_ptr(thr) > Which is: NULL > tlh1.list() > Which is: 0x7ff303522800 > thr->_threads_hazard_ptr must match tlh1.list() The test code: // 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()"; This test was expecting the destruction of tlh2a to restore the thread's hazard ptr to tlh1.list(), but that didn't happen. > ./open/test/hotspot/gtest/runtime/test_ThreadsListHandle.cpp:570: Failure > Expected equality of these values: > ThreadsListHandleTest::get_Thread_nested_threads_hazard_ptr_cnt(thr) > Which is: 1 > (uint)0 > Which is: 0 > thr->_nested_threads_hazard_ptr_cnt must be 0 The test code: // Test case: after first back-to-back nested ThreadsListHandle (tlh2a) has been destroyed <snip> EXPECT_EQ(ThreadsListHandleTest::get_Thread_nested_threads_hazard_ptr_cnt(thr), (uint)0) << "thr->_nested_threads_hazard_ptr_cnt must be 0"; This test was expecting the destruction of tlh2a to decrement the thread's nested_threads_hazard_ptr_cnt field from 1 to 0, but that didn't happen. > ./open/test/hotspot/gtest/runtime/test_ThreadsListHandle.cpp:643: Failure > Expected equality of these values: > ThreadsListHandleTest::get_Thread_threads_hazard_ptr(thr) > Which is: NULL > tlh1.list() > Which is: 0x7ff303522800 > thr->_threads_hazard_ptr must match tlh1.list() The test code: // 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()"; This test was expecting the destruction of tlh2b to restore the thread's hazard ptr to tlh1.list(), but that didn't happen. > ./open/test/hotspot/gtest/runtime/test_ThreadsListHandle.cpp:647: Failure > Expected equality of these values: > ThreadsListHandleTest::get_Thread_nested_threads_hazard_ptr_cnt(thr) > Which is: 1 > (uint)0 > Which is: 0 > thr->_nested_threads_hazard_ptr_cnt must be 0 The test code: // Test case: after second back-to-back nested ThreadsListHandle (tlh2b) has been destroyed <snip> EXPECT_EQ(ThreadsListHandleTest::get_Thread_nested_threads_hazard_ptr_cnt(thr), (uint)0) << "thr->_nested_threads_hazard_ptr_cnt must be 0"; This test was expecting the destruction of tlh2b to decrement the thread's nested_threads_hazard_ptr_cnt field from 1 to 0, but that didn't happen. > [ FAILED ] ThreadsListHandle.sanity_vm (0 ms) > [----------] 1 test from ThreadsListHandle (67 ms total)
16-12-2020

I've added the patch that adds the test: 8258284-new-test-only.patch I've also added the log files for running the new test on the baseline: 8258284.baseline-failures.zip
16-12-2020

I've created a new test case to verify my understanding of how ThreadsListHandles are supposed to work and what they change on the calling JavaThread. Here's the test cases: $ grep 'Test case:' test_ThreadsListHandle.cpp // Test case: no ThreadsListHandle // Test case: single ThreadsListHandle, no recursion // Test case: after first ThreadsListHandle (tlh1) has been destroyed // Test case: first ThreadsListHandle to prepare for nesting // Test case: first nested ThreadsListHandle // Test case: after first nested ThreadsListHandle (tlh2) has been destroyed // Test case: after first ThreadsListHandle to prepare for nesting has been destroyed // Test case: first ThreadsListHandle to prepare for double nesting // Test case: first nested ThreadsListHandle // Test case: double nested ThreadsListHandle // Test case: after double nested ThreadsListHandle (tlh3) has been destroyed // Test case: after first nested ThreadsListHandle (tlh2) has been destroyed // Test case: after first ThreadsListHandle to prepare for double nesting has been destroyed // Test case: first ThreadsListHandle to prepare for back-to-back nesting // Test case: first back-to-back nested ThreadsListHandle // Test case: after first back-to-back nested ThreadsListHandle (tlh2a) has been destroyed // Test case: second back-to-back nested ThreadsListHandle // Test case: after second back-to-back nested ThreadsListHandle (tlh2b) has been destroyed // Test case: after first ThreadsListHandle to prepare for back-to-back nesting has been destroyed
16-12-2020