RFR : JDK-8044122 MBean access to the PID

[Roger Riggs]

Looks good to me.

Thanks, Roger

On 10/30/2017 12:50 PM, Ujwal Vangapally wrote:
>
> Hi Mandy,
>
> yes, this makes it more clear.
>
> kindly take a look at new webrev.
>
> webrev : 
> http://cr.openjdk.java.net/~uvangapally/webrev/2017/8044122/webrev.04/
>
> Thanks,
>
> Ujwal.
>
> On 10/28/2017 4:35 AM, mandy chung wrote:
>> This interface method provides a default implementation to throw UOE 
>> to ease migration but RuntimeMXBean::getPid is not intended to be 
>> optional.  In other words, the platform mbean (i.e. JDK 
>> implementation of RuntimeMXBean) must implement this method.  This 
>> deserves further clarification.
>>
>> What about:
>>
>> @throws UOE if this default implementation is not overridden.
>> RuntimeMXBean for the Java platform obtained from {@link 
>> ManagementFactory} supports this operation.
>>
>> Mandy
>>
>> On 10/26/17 9:36 AM, Ujwal Vangapally wrote:
>>>
>>> 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/20171030/f06f1cdd/attachment.html>