Proposal:JdpController.getProcessId() VM compatibility improvement
Roger Riggs
Roger.Riggs at Oracle.com
Mon Jun 26 18:58:45 UTC 2017
Hi Andrew,
Redirecting back to serviceability-dev at 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_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_
> <mailto:andrew_m_leonard at uk.ibm.com>
>
>
>
>
> From: Roger Riggs _<Roger.Riggs at Oracle.com>_
> <mailto:Roger.Riggs at Oracle.com>
> To: _core-libs-dev at openjdk.java.net_
> <mailto: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>_
> <mailto: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_
> <mailto:andrew_m_leonard at uk.ibm.com>
> >
> >
> >
> >
> > From: Alan Bateman _<Alan.Bateman at oracle.com>_
> <mailto:Alan.Bateman at oracle.com>
> > To: Andrew Leonard _<andrew_m_leonard at uk.ibm.com>_
> <mailto:andrew_m_leonard at uk.ibm.com>,
> > _core-libs-dev at openjdk.java.net_ <mailto: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_
> <mailto: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