RFR : JDK-8044122 MBean access to the PID

[Ujwal Vangapally]

Hi Roger,

made changes as suggested.

webrev : 
http://cr.openjdk.java.net/~uvangapally/webrev/2017/8044122/webrev.03/

Thanks,
Ujwal.

On 10/26/2017 6:54 PM, Roger Riggs wrote:
> 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/2b6af468/attachment.html>