Proposal:JdpController.getProcessId() VM compatibility improvement

Andrew Leonard andrew_m_leonard at uk.ibm.com
Mon Jun 26 14:37:59 UTC 2017


Hi Roger,
I have now updated to the latest openjdk9 and examined the existing jdp 
tests. I have added to those tests to assert the JDP packet processId, 
which was in fact "null", so it was failing to set it before... Hence, 
with this testcase update they fail without my patch, and succeed with the 
new patch.
Being new to this contribution process, where do I go from here please?
Thanks
Andrew

diff -r 2425838cfb5e 
src/jdk.management.agent/share/classes/sun/management/jdp/JdpController.java
--- 
a/src/jdk.management.agent/share/classes/sun/management/jdp/JdpController.java 
Fri Jun 23 14:32:59 2017 -0400
+++ 
b/src/jdk.management.agent/share/classes/sun/management/jdp/JdpController.java 
Mon Jun 26 15:31:15 2017 +0100
@@ -133,20 +133,8 @@
 
     // Get the process id of the current running Java process
     private static Integer getProcessId() {
-        try {
-            // Get the current process id using a reflection hack
-            RuntimeMXBean runtime = ManagementFactory.getRuntimeMXBean();
-            Field jvm = runtime.getClass().getDeclaredField("jvm");
-            jvm.setAccessible(true);
-
-            VMManagement mgmt = (sun.management.VMManagement) 
jvm.get(runtime);
-            Method pid_method = 
mgmt.getClass().getDeclaredMethod("getProcessId");
-            pid_method.setAccessible(true);
-            Integer pid = (Integer) pid_method.invoke(mgmt);
-            return pid;
-        } catch(Exception ex) {
-            return null;
-        }
+        // Get the current process id
+        return (int)ProcessHandle.current().pid();
     }

diff -r 2425838cfb5e test/sun/management/jdp/JdpOnTestCase.java
--- a/test/sun/management/jdp/JdpOnTestCase.java        Fri Jun 23 
14:32:59 2017 -0400
+++ b/test/sun/management/jdp/JdpOnTestCase.java        Mon Jun 26 
15:30:51 2017 +0100
@@ -31,6 +31,7 @@
 
 import java.net.SocketTimeoutException;
 import java.util.Map;
+import static jdk.testlibrary.Asserts.assertNotEquals;
 
 public class JdpOnTestCase extends JdpTestCase {
 
@@ -58,6 +59,7 @@
         final String jdpName = payload.get("INSTANCE_NAME");
         log.fine("Received correct JDP packet #" + 
String.valueOf(receivedJDPpackets) +
                 ", jdp.name=" + jdpName);
+        assertNotEquals(null, payload.get("PROCESS_ID"), "Expected 
payload.get(\"PROCESS_ID\") to be not null.");
     }




Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leonard at uk.ibm.com 




From:   Roger Riggs <roger.riggs at oracle.com>
To:     Andrew Leonard <andrew_m_leonard at uk.ibm.com>
Cc:     core-libs-dev at openjdk.java.net
Date:   23/06/2017 13:57
Subject:        Re: Proposal:JdpController.getProcessId() VM compatibility 
improvement



Hi Leonard,

No need to stray from your intended focus.  
The other is just another cleanup.

Thanks, Roger


On 6/23/17 6:57 AM, Andrew Leonard wrote:
Thanks Roger, 
So yes that issue does look similar, although I was only looking in the 
management interfaces. I'll have a peek at those references to see if they 
are similar, it would make sense to clean those up too, to be consistent. 
Your thoughts? 
I'll also see if there's a junit for this method, if not i'll see if I can 
add one. 
Cheers 
Andrew 

Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leonard at uk.ibm.com 




From:        Roger Riggs <Roger.Riggs at Oracle.com> 
To:        core-libs-dev at openjdk.java.net 
Date:        22/06/2017 18:03 
Subject:        Re: Proposal:JdpController.getProcessId() VM compatibility 
improvement 
Sent by:        "core-libs-dev" <core-libs-dev-bounces at openjdk.java.net> 



HI,

This looks like:

JDK-8074569 <https://bugs.openjdk.java.net/browse/JDK-8074569> Update 
implementation to use ProcessHandle.current() to get the PID

I'm happy to sponsor.

Roger

On 6/22/2017 12:27 PM, Andrew Leonard wrote:
> Thanks Alan,
> Yes, you're right the exception swallowing can be removed too.
> I'll re-post to serviceability-dev...
> Cheers
> Andrew
>
> Andrew Leonard
> Java Runtimes Development
> IBM Hursley
> IBM United Kingdom Ltd
> Phone internal: 245913, external: 01962 815913
> internet email: andrew_m_leonard at uk.ibm.com
>
>
>
>
> From:   Alan Bateman <Alan.Bateman at oracle.com>
> To:     Andrew Leonard <andrew_m_leonard at uk.ibm.com>,
> core-libs-dev at openjdk.java.net
> Date:   22/06/2017 17:21
> Subject:        Re: Proposal:JdpController.getProcessId() VM 
compatibility
> improvement
>
>
>
>
> This seems a good clean-up, esp. as java.management doesn't open
> sun.management to jdk.management.agent so I'll bet this wasn't working
> any. I assume the exception swallowing can be removed too.
>
> Can you bring this to serviceability-dev as this is where this code is
> maintained?
>
> On 22/06/2017 14:34, Andrew Leonard wrote:
>> Hello,
>> I would like to propose the change below to the
>> JdpController.getProcessId() method which currently uses a reflection
> hack
>> and the VMManagement interface. Instead for JDK9+ we can use the
>> ProcessHandle.current().getPid() method, which will also allow better
>> compatibility with other pluggable VMs.
>>
>>
> 
jdk/src/jdk.management.agent/share/classes/sun/management/jdp/JdpController.java
>> 36d35
>> < import sun.management.VMManagement;
>> 137,146c136,137
>> <             // Get the current process id using a reflection hack
>> <             RuntimeMXBean runtime =
>> ManagementFactory.getRuntimeMXBean();
>> <             Field jvm = runtime.getClass().getDeclaredField("jvm");
>> <             jvm.setAccessible(true);
>> <
>> <             VMManagement mgmt = (sun.management.VMManagement)
>> jvm.get(runtime);
>> <             Method pid_method =
>> mgmt.getClass().getDeclaredMethod("getProcessId");
>> <             pid_method.setAccessible(true);
>> <             Integer pid = (Integer) pid_method.invoke(mgmt);
>> <             return pid;
>> ---
>>>               // Get the current process id
>>>               return (int)ProcessHandle.current().getPid();
>> I'd appreciate any feedback please, and how I would go about obtaining 
a
>> sponsor and contributor?
>>
>> Thanks
>> Andrew
>>
>> Andrew Leonard
>> Java Runtimes Development
>> IBM Hursley
>> IBM United Kingdom Ltd
>> Phone internal: 245913, external: 01962 815913
>> internet email: andrew_m_leonard at uk.ibm.com
>>
>> Unless stated otherwise above:
>> IBM United Kingdom Limited - Registered in England and Wales with 
number
>> 741598.
>> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6
> 3AU
>
>
>
>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 
3AU




Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU



Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU


More information about the core-libs-dev mailing list