RFR : JDK-8044122 MBean access to the PID

[Roger Riggs]

Hi Ujwal,

In RuntimeMXBean:

Please add @throws UnsupportedOperationException if the process id is 
not available

Otherwise, looks fine.

Thanks, Roger


On 10/26/2017 4:29 AM, Ujwal Vangapally wrote:
>
> Thanks for the review Mandy, Roger, Harsha, Christoph.
>
> kindly see the new webrev incorporating review comments.
>
> webrev: 
> http://cr.openjdk.java.net/~uvangapally/webrev/2017/8044122/webrev.02/
>
> csr : https://bugs.openjdk.java.net/browse/JDK-8189091
>
> Ujwal.
>
>
> On 10/24/2017 10:50 PM, mandy chung wrote:
>> The permission check should be done separately, if needed. Otherwise, 
>> the frames in the stack when this method is called must have the 
>> security permission granted.
>>
>> Mandy
>>
>> On 10/23/17 1:00 PM, Bernd Eckenfels wrote:
>>> Hello,
>>>
>>> When running this privileged it means one can bypass the permission 
>>> by using the MBean, is that intentional? (Besides it is already 
>>> available as the JMVID)
>>>
>>> Gruss
>>> Bernd
>>> -- 
>>> http://bernd.eckenfels.net
>>> ------------------------------------------------------------------------
>>> *From:* serviceability-dev 
>>> <serviceability-dev-bounces at openjdk.java.net> on behalf of mandy 
>>> chung <mandy.chung at oracle.com>
>>> *Sent:* Friday, October 20, 2017 4:42:00 PM
>>> *To:* Ujwal Vangapally; Roger Riggs
>>> *Cc:* serviceability-dev
>>> *Subject:* Re: RFR : JDK-8044122 MBean access to the PID
>>> Process::pid may throw SecurityException.  You have to wrap the call
>>> with doPrivileged.   Process::pid can throw UOE on platform that 
>>> doesn't
>>> support this operation.  RuntimeMXBean::getPid should specify when the
>>> platform does not support this operation.
>>>
>>> ProcessIdTest - can you also check if getName contains the pid matching
>>> the value of getPid()?
>>>
>>> Mandy
>>>
>>> On 10/20/17 2:07 AM, Ujwal Vangapally wrote:
>>> > kindly see the new webrev incorporating review comments.
>>> >
>>> > webrev:
>>> > 
>>> http://cr.openjdk.java.net/~uvangapally/webrev/2017/8044122/webrev.01/ 
>>> <http://cr.openjdk.java.net/%7Euvangapally/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/ 
>>> <http://cr.openjdk.java.net/%7Euvangapally/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
>>> >
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20171026/1c34536a/attachment.html>