On Thu, 5 Sep 2024 11:29:41 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Address review comments > > test/jdk/java/foreign/TestMappedHandshake.java line 90: > >> 88: >> 89: accessExecutor.shutdown(); >> 90: >> assertTrue(accessExecutor.awaitTermination(MAX_EXECUTOR_WAIT_SECONDS, >> TimeUnit.SECONDS)); > > ExecutorService is an AutoCloseable to you should be able to use > try-with-resources and get rid of the the shutdown+awaitTermination. The test (like also its sibling `TestHandshake`) is deliberately using `shutdown`/`awaitTermination` to make sure the test finished in a reasonable amount of time (and generate correct assertion failures if it doesn't). This was mostly added to have better debugging at a time when `TestHandshake` was failing spuriously, so I'd prefer to leave it as is. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20854#discussion_r1745307197
