RFR: 8371502: serviceability/jvmti/vthread/ThreadListStackTracesTest/ThreadListStackTracesTest.java failing [v3]

[Serguei Spitsyn]

On Thu, 11 Dec 2025 03:48:58 GMT, Leonid Mesnik <lmesnik at openjdk.org> wrote:

>> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   review: use ReentrantLock.hasQueuedThread()
>
> 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.

Thank you. I was thinking about the same but decided to go and introduce new check method.
First, the check for `rlock.hasQueuedThreads()` seems to be good enough even without `ensureReady()` call. But it is hard to be sure it is the case. Second, the check to `ensureReady()` can be returned early without waiting on the `rlock`. In such a case, we have to rely on the `rlock.hasQueuedThreads()` which does not check the target thread state. Also, the code duplication is small but it is more clean and reliable way to check the needed condition.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/28734#discussion_r2609112411