> Can I please get a review of this change which proposes to fix the issue > noted in https://bugs.openjdk.org/browse/JDK-8320687? > > As noted in the issue, the > `sun.jvmstat.monitor.MonitoredHost.getMonitoredHost()` uses a shared instance > of `java.util.ServiceLoader` to load `MonitoredHostService` services. The > `ServiceLoader` class javadoc explicitly notes that it isn't thread safe. The > issue at hand is caused to due using an instance of `ServiceLoader` > concurrently by multiple threads. > > The fix proposes to guard the usage of the shared `ServiceLoader` instance > through the `monitoredHosts` object monitor. We already use that monitor when > dealing with the internal cache which is populated after loading the relevant > `MonitoredHostService`(s). > > A new jtreg test has been introduced which always reproduces the issue > without the source changes and passes with this fix. > > tier1, tier2, tier3 and svc_tools tests have been run with this change and > all passed.
Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision: Kevin's inputs - simplify the synchronized block ------------- Changes: - all: https://git.openjdk.org/jdk/pull/16805/files - new: https://git.openjdk.org/jdk/pull/16805/files/78e5cd7b..669e5e5e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16805&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16805&range=01-02 Stats: 39 lines in 1 file changed: 7 ins; 28 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/16805.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16805/head:pull/16805 PR: https://git.openjdk.org/jdk/pull/16805