On Thu, 8 May 2025 19:17:19 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Rename awaitBlocked to awaitTrueAndBlocked
>
> test/jdk/java/lang/management/ThreadMXBean/VirtualThreadDeadlocks.java line
> 99:
>
>> 97: System.out.println("Waiting for thread1 and thread2 to deadlock
>> ...");
>> 98: awaitBlocked(thread1, reached1);
>> 99: awaitBlocked(thread2, reached2);
>
> This looks okay but does mean that awaitBlocked is doing two things, maybe it
> should be rename to awaitTrueAndBlocked.
>
> An alternative that would remove AtomicReference from the picture is to just
> have two volatile booleans and have the main thread poll both until true.
> That would leave the use of awaitBlock unchanged but what you have is okay
> too.
Yes, I actually had that in an initial version. Then I thought it was cleaner
to just have all waiting logic in awaitBlocked so I changed it. I renamed
`awaitBlocked` to `awaitTrueAndBlocked` for now. But let me know if you prefer
the two volatile booleans instead.
Also, we could remove the `CyclicBarrier` and have each thread poll the other’s
thread flag now, but given this bug was found due to the barrier I left it as
is.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25119#discussion_r2080371142
