[jdk17u-dev] RFR: 8320687: sun.jvmstat.monitor.MonitoredHost.getMonitoredHost() throws unexpected exceptions when invoked concurrently [v3]
Volker Simonis
simonis at openjdk.org
Fri May 23 14:25:57 UTC 2025
On Tue, 20 May 2025 17:57:44 GMT, Paul Hohensee <phh at openjdk.org> wrote:
>> Backport for parity with Oracle 17.0.16. Clean except for copyright date in MonitoredHost.java, and replacement of try-with-resources in new test with try-catch because ExecutorService does not implement AutoClosable in 17u. New test passes, as well as the existing tests under test/jdk/sun/jvmstat/monitor.
>
> Paul Hohensee has updated the pull request incrementally with one additional commit since the last revision:
>
> 8320687: Revert trigger
Looks good.
test/jdk/sun/jvmstat/monitor/MonitoredVm/ConcurrentGetMonitoredHost.java line 60:
> 58: System.out.println("Submitting " + numTasks + " concurrent tasks to" +
> 59: " get MonitoredHost for " + vmid);
> 60: try {
As you've already mentioned, the original implementation uses try-with-resources to close the `ExecutorService` but `ExecutorService` only implements `AutoClosable` since JDK 19. Alternatively, we could put the new `ExecutorService`'s `close()` handling into a `finallly` block:
boolean terminated = isTerminated();
if (!terminated) {
shutdown();
boolean interrupted = false;
while (!terminated) {
try {
terminated = awaitTermination(1L, TimeUnit.DAYS);
} catch (InterruptedException e) {
if (!interrupted) {
shutdownNow();
interrupted = true;
}
}
}
if (interrupted) {
Thread.currentThread().interrupt();
}
}
But that seems to be overkill, taking into account that in the body of the `try/catch` we already wait for the completion of all `ExecutorService` tasks. So I leave it up to you if you want the additional safe guard.
-------------
Marked as reviewed by simonis (Reviewer).
PR Review: https://git.openjdk.org/jdk17u-dev/pull/3593#pullrequestreview-2864613303
PR Review Comment: https://git.openjdk.org/jdk17u-dev/pull/3593#discussion_r2104707968
More information about the jdk-updates-dev
mailing list