On Fri, 24 Nov 2023 06:06:16 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
> The fix proposes to guard the usage of the shared ServiceLoader instance > through the monitoredHosts object monitor. An alternative was to just create a new instance of `ServiceLoader` within the method implementation whenever a `MonitoredHost` was not found in the `monitoredHosts`. That works fine too. However, that will have a (relative) disadvantage that we keep loading the MonitoredHostService instances afresh and thus loses the benefits that using a single `ServiceLoader` instance provides (`ServiceLoader` instance internally maintains a cache of already loaded `Service` instances as noted in its javadoc). So I decided to stick to using a single instance of `ServiceLoader` and have its access guarded by synchronization. Loading and instantiating `MonitoredHostService` instances within this synchronized block, I think, shouldn't cause issues because the `ServiceLoader`'s load method is passed the classloader of `sun.jvmstat.monitor.MonitoredHostService` class, which happens to be the `jdk.internal.jvmstat` JDK module. Thus it wouldn't involve unexpected user/application class instances being loaded while holding this lock. ------------- PR Comment: https://git.openjdk.org/jdk/pull/16805#issuecomment-1825194163
