On Thu, 11 Dec 2025 02:36:12 GMT, Serguei Spitsyn <[email protected]> wrote:
>> This fixes the test which is unreliable and fails intermittently from time
>> to time.
>> The test was failing in the method `checkReentrantLock()` when comparing the
>> expected state with result from `vt.getState()`. The issue is that the call
>> to `ThreadListStackTracesTest.reentrantLock.lock()` in a
>> `ReentrantLockTestTask` can reach waiting state on some class loading but
>> not on the ReentrantLock. Please see the first comment for test sources
>> details.
>> The fix is to add a big enough sleep before call to the checkStates().
>>
>> Testing:
>> - submitted about thousands of mach5 runs of the
>> `ThreadListStackTracesTest` test runs before and after the fix
>> - TBD: submit mach5 tiers 1-3 to be completely safe
>
> Serguei Spitsyn has updated the pull request incrementally with one
> additional commit since the last revision:
>
> review: use ReentrantLock.hasQueuedThread()
Changes requested by lmesnik (Reviewer).
test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest/ThreadListStackTracesTest.java
line 49:
> 47: public void ensureReadyAndWaiting(Thread vt, Thread.State expState,
> ReentrantLock rlock) {
> 48: // wait while the thread is not ready or thread state is
> unexpected
> 49: while (!threadReady || (vt.getState() != expState) ||
> !rlock.hasQueuedThreads()) {
Small nit.
The comment is incomplete. And code duplication is unneded.
You can make
task.ensureReady(vt, expState);
// Ensure that thread waiting on the rlock
while (!rlock.hasQueuedThreads()) {
sleep(1);
}
to reduce code duplication.
Really,
task.ensureReady(vt, expState);
Shouldn't be needed. We can't have lock.hasQueuedThreads() == true until
we update `threadReady` and hit `vt.getState()`.
But I think it is fine to check all conditions.
-------------
PR Review: https://git.openjdk.org/jdk/pull/28734#pullrequestreview-3565479172
PR Review Comment: https://git.openjdk.org/jdk/pull/28734#discussion_r2609008674
