RFR : JDK-8044122 MBean access to the PID

Langer, Christoph christoph.langer at sap.com
Mon Oct 23 18:27:38 UTC 2017


Hi Ujwal,

just a minor thing: there's one space too much that sneaked in in line 72 "    public default  long getPid() {" of src/java.management/share/classes/java/lang/management/RuntimeMXBean.java.

Best regards
Christoph

> -----Original Message-----
> From: serviceability-dev [mailto:serviceability-dev-
> bounces at openjdk.java.net] On Behalf Of Ujwal Vangapally
> Sent: Freitag, 20. Oktober 2017 11:08
> To: serviceability-dev at openjdk.java.net; Mandy Chung
> <mandy.chung at oracle.com>; Roger Riggs <Roger.Riggs at oracle.com>
> Subject: Re: RFR : JDK-8044122 MBean access to the PID
> 
> kindly see the new webrev incorporating review comments.
> 
> webrev:
> http://cr.openjdk.java.net/~uvangapally/webrev/2017/8044122/webrev.01/
> 
> Thanks,
> 
> Ujwal.
> 
> 
> On 10/11/2017 3:50 PM, Ujwal Vangapally wrote:
> > Thanks for the review and suggestions Mandy, Roger.
> >
> > kindly see my comments inline.
> >
> >
> > On 10/10/2017 11:25 PM, Roger Riggs wrote:
> >> Hi Ujwal,
> >>
> >> In the implementation RuntimeMXBean.java: 72:  Include a message
> >> "getProcessId" in the throw new Unsupported...
> >> In the text and @return change "PID" to "process ID" as Alan suggested.
> >> 66: the @implSpec should be on its own line so the text starts on a
> >> new line to make the source more readable.
> >>
> >> Adding a test for getProcessId() should fit into one of the existing
> >> tests that spawns and then checks
> >> the attributes of a vm.  Perhaps MXBeanInteropTest1.java
> > I will make changes as suggested.
> >>
> >> Roger
> >>
> >>
> >>
> >> On 10/10/2017 1:20 PM, mandy chung wrote:
> >>>
> >>>
> >>> On 10/10/17 4:47 AM, Ujwal Vangapally wrote:
> >>>> Kindly review the changes made.
> >>>>
> >>>> https://bugs.openjdk.java.net/browse/JDK-8044122
> >>>>
> >>>> webrev :
> >>>>
> http://cr.openjdk.java.net/~uvangapally/webrev/2017/8044122/webrev.00/
> >>>>
> >>>
> >>> RuntimeMXBean.java
> >>>    @since is missing
> >>>
> > I will add it.
> >>>    Process::pid is long rather than int. The javadoc for this method
> >>> should be consistent with Process::pid, as Alan points out.
> > will do it.
> >>>
> >>> VMManagementImpl.java
> >>>     I think getProcessId should probably be replaced to implement
> >>> with ProcessHandle.current().pid();
> >>>
> > you mean it would be better to use ProcessHandle.current().pid(); in
> > RuntimeImpl.java instead of jvm.getVmPid();
> >
> > kindly clarify.
> >>> Please include an unit test for it.
> >>>
> > will it be sufficient to add it to existing test MXBeanInteropTest1.java
> >
> >              System.out.println("getName\t\t"
> >                      + runtime.getName());
> > +            System.out.println("getPid\t\t"
> > +                    + runtime.getPid());
> >              System.out.println("getSpecName\t\t"
> >                      + runtime.getSpecName());
> >
> >>> Mandy
> >>
> >
> > Ujwal



More information about the serviceability-dev mailing list