On Thu, 8 May 2025 13:46:02 GMT, Patricio Chilano Mateo
<pchilanom...@openjdk.org> wrote:
> Please review this small test fix. We need to make sure the two threads are
> blocked on the expected locks before invoking findMonitorDeadlockedThreads.
> In the failing cases, one of the threads is seen as blocked while waiting for
> a class to be initialized by the other thread (I've added the stack traces
> showing where this occurs in the bug comments).
>
> I should point out that there is an inconsistency in the VM here though. We
> are changing the state to Thread.State.BLOCKED while using ObjectLocker
> internally when there is contention to enter the monitor, but we don’t change
> the state to Thread.State.WAITING when using ObjectLocker and calling
> wait_uninterruptibly(). I still think the test should be improved to avoid
> having to think if there is some other synchronize statement executed along
> the way (CyclicBarrier implementation for instance, or code run by a vthread
> during startup).
>
> I was able to reproduce the failure and verified it doesn’t reproduce anymore
> with the fix.
>
> Thanks,
> Patricio
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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25119#discussion_r2080327776
