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-boun...@openjdk.java.net> on behalf of mandy chung <mandy.ch...@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
>





Reply via email to