RFR : JDK-8044122 MBean access to the PID

[Ujwal Vangapally]

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/6d3a98c5/attachment.html>