Thanks Roger for taking a look, i'll see what the serviceability community
think,
cheers
Andrew
Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leon...@uk.ibm.com
From: Roger Riggs <roger.ri...@oracle.com>
To: Andrew Leonard <andrew_m_leon...@uk.ibm.com>
Cc: serviceability-dev@openjdk.java.net, Core-Libs-Dev
<core-libs-...@openjdk.java.net>
Date: 26/06/2017 19:59
Subject: Re: Proposal:JdpController.getProcessId() VM compatibility
improvement
Hi Andrew,
Redirecting back to serviceability-dev@openjdk.java.net, it is the better
list for this topic.
I don't know these tests but it seems odd to stick that assert into every
callback to packetFromThisVMReceived. Perhaps someone more familiar can
review and sponsor.
Thanks, Roger
On 6/26/2017 10:37 AM, Andrew Leonard wrote:
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_leon...@uk.ibm.com
From: Roger Riggs <roger.ri...@oracle.com>
To: Andrew Leonard <andrew_m_leon...@uk.ibm.com>
Cc: core-libs-...@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_leon...@uk.ibm.com
From: Roger Riggs <roger.ri...@oracle.com>
To: core-libs-...@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-boun...@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_leon...@uk.ibm.com
>
>
>
>
> From: Alan Bateman <alan.bate...@oracle.com>
> To: Andrew Leonard <andrew_m_leon...@uk.ibm.com>,
> core-libs-...@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_leon...@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
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
