[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8320687: sun.jvmstat.monitor.MonitoredHost.getMonitoredHost() throws unexpected exceptions 
From:       Jaikiran Pai <jpai () openjdk ! org>
Date:       2023-11-25 6:26:08
Message-ID: BLvfDg2lqL3TN67jJQ0h-TbpPVgujlhTOEbZZbwFhAg=.b452149b-951d-4af0-a7f2-677966e9f2f7 () github ! com
[Download RAW message or body]

On Fri, 24 Nov 2023 13:00:33 GMT, Jaikiran Pai <jpai@openjdk.org> wrote:

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

Thank you Kevin and Alan for the reviews. tier1, tier2, tier3 and svc_tools all \
passed with this latest state of the PR. Since serviceability area requires 2 \
Reviewers, could I get one more Reviewer to review this please?

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

PR Comment: https://git.openjdk.org/jdk/pull/16805#issuecomment-1826229504


[prev in list] [next in list] [prev in thread] [next in thread]