[jdk17u-dev] RFR: 8320687: sun.jvmstat.monitor.MonitoredHost.getMonitoredHost() throws unexpected exceptions when invoked concurrently [v3]

[Volker Simonis]

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