> 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:

  Alan's suggestion - don't share ServiceLoader

-------------

Changes:
  - all: https://git.openjdk.org/jdk/pull/16805/files
  - new: https://git.openjdk.org/jdk/pull/16805/files/669e5e5e..6e7bf7fb

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16805&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16805&range=02-03

  Stats: 45 lines in 1 file changed: 16 ins; 20 del; 9 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

Reply via email to