On Wed, 12 Feb 2025 18:28:19 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
> The code changes look reasonable. WRT to detecting deadlocks a possibility is > also to use `ManagementFactory.getThreadMXBean().findDeadlockedThreads();` > > See for instance > > https://github.com/openjdk/jdk/blob/336d0d8592aed734e7b8139e1ecd71d33825c75a/test/jdk/java/util/logging/TestLogConfigurationDeadLock.java#L167 I don't think I need this because if there was a deadlock, I know it can only be between the 2 threads I started. > test/jdk/java/util/logging/LoggingDeadlock5.java line 31: > >> 29: * java.logging >> 30: * @compile -XDignore.symbol.file LoggingDeadlock5.java >> 31: * @run main/othervm/timeout=10 LoggingDeadlock5 > > I would not specify any /timeout value here, and rather let jtreg deal with > that. I was just copying the other timeout tests in this directory. This test is written to avoid blocking (apart from the self test case) and complete in about 1 second, so removing this should be fine. Will do so in the next push. > test/jdk/java/util/logging/LoggingDeadlock5.java line 115: > >> 113: >> 114: private static class DeadLocker { >> 115: private final static Duration JOIN_WAIT = >> Duration.ofMillis(500); > > You might want to use `jdk.test.lib.Utils.adjustTimeout()` here to account > for the case when the test is run on slower configurations. Typically in > higher tiers, this test might be run with -Xcomp or -Xint and on debug builds > which might cause slow downs and cause the join() method to exit too early - > reporting false positives. None of the other logging deadlock tests use this mechanism, but thanks for pointing it out. I will investigate. ------------- PR Comment: https://git.openjdk.org/jdk/pull/23491#issuecomment-2656028524 PR Review Comment: https://git.openjdk.org/jdk/pull/23491#discussion_r1954151677 PR Review Comment: https://git.openjdk.org/jdk/pull/23491#discussion_r1954153098
