On Tue, 10 Mar 2026 10:25:48 GMT, Volkan Yazici <[email protected]> wrote:
>> Can I please get a review of this test-only change which proposes to use >> "othervm" for some of these tests that configure java.util.logging logger >> handlers? >> >> As noted in https://bugs.openjdk.org/browse/JDK-8379477 some of these test >> definitions don't explicitly require a othervm and thus can interfere with >> one another when configuring logging handlers (for the same logger). The >> tests themselves don't seem to do anything useful with those loggers or >> logging handlers, so the logging configuration could infact be completely >> removed. But after discussing about it, we decided to just switch them over >> to othervm for now and also use ConsoleHandler in all these tests. >> >> tier testing completed successfully with this change. > > test/jdk/com/sun/net/httpserver/TaskRejectedTest.java line 58: > >> 56: private static final int TIMEOUT = 10000; // 10 sec >> 57: >> 58: private static final Logger logger = >> Logger.getLogger(HttpServer.class.getPackage().getName()); > > It is very easy to refactor these back into a local variable, just like they > were before, and take the wrong path. Would you mind documenting the > rationale for this class field, please? > > Suggestion: > > /** Anchor to avoid the {@code Logger} instance get GC'ed */ > private static final Logger logger = > Logger.getLogger(HttpServer.class.getPackage().getName()); > > > Note that this remark applies to multiple places in this PR. Hello Volkan, the `Logger` class already has this detail which says https://docs.oracle.com/en/java/javase/25/docs/api/java.logging/java/util/logging/Logger.html: > It is important to note that the Logger returned by one of the getLogger > factory methods may be garbage collected at any time if a strong reference to > the Logger is not kept. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30158#discussion_r2912172206
