On Tue, 9 Apr 2024 11:08:31 GMT, Kevin Walls <kev...@openjdk.org> wrote:
> This test incorrectly fails, although rarely, thinking its "thread 2" has
> deadlocked.
> A change of sleep will likely fix this, but there are other issues, so
> cleaning up the test a little.
>
> Remove the probe for the ManagementFactory class, to check we are on jdk5 or
> later. 8-)
>
> When sleeping, sleep 100, not 1ms, we don't need to spin fast and actually
> race with the other thread.
>
> We have a 1000 iteration loop, but don't seem to use it. We only check once
> then either return (pass), fail, or break (which is also fail). Use the loop
> to check for the status change, which is likely what was intended.
>
> Show the stackframes on all failures.
Marked as reviewed by lmesnik (Reviewer).
test/jdk/javax/management/notification/BroadcasterSupportDeadlockTest.java line
122:
> 120: java.util.Map<Thread,StackTraceElement[]> traces =
> Thread.getAllStackTraces();
> 121: showStackTrace("Thread 1", traces.get(t1));
> 122: showStackTrace("Thread 2", traces.get(t2));
Could you please replace System.exit() with throwing Exception. Other looks
good.
-------------
PR Review: https://git.openjdk.org/jdk/pull/18687#pullrequestreview-2002384686
PR Review Comment: https://git.openjdk.org/jdk/pull/18687#discussion_r1566573453
