Gary,
What is the overhead of doing the validation? How do we list VMs? Do we
need to examine every running process to get the list of VMs? Wouldn't
it be better to check the given process is a VM rather than checking all
potential VM processes?
I think there is an onus of responsibility on the user to provide a
correct pid here, rather than trying to make this foolproof.
Thanks,
David
On 15/02/2019 5:12 am, Gary Adams wrote:
The following commands present a similar kill process behavior:
jcmd
jinfo
jmap
jstack
The following commands do not attach.
jstat sun.jvmstat.monitor.MonitorException "not found"
jps no pid arguments
This update moves the checkJavaPid method into the
common/ProcessArgumentsMatcher.java
and applies the check before the pid is used for an attach operation.
Revised webrev: http://cr.openjdk.java.net/~gadams/8149461/webrev.01/
On 2/14/19, 12:07 PM, Chris Plummer wrote:
Hi Gary,
What about the other tools that attach to a user specified process. Do
they also have this issue?
thanks,
Chris
On 2/14/19 8:35 AM, Gary Adams wrote:
There is an old reported problem that using jmap on a process that is
not running
Java could cause the process to terminate. This is due to the SIGQUIT
used
when attaching to the process.
It is a fairly simple operation to validate that the pid matches one
of the known
running Java processes using VirtualMachine.list().
Issue: https://bugs.openjdk.java.net/browse/JDK-8149461
Proposed fix:
diff --git a/src/jdk.jcmd/share/classes/sun/tools/jmap/JMap.java
b/src/jdk.jcmd/share/classes/sun/tools/jmap/JMap.java
--- a/src/jdk.jcmd/share/classes/sun/tools/jmap/JMap.java
+++ b/src/jdk.jcmd/share/classes/sun/tools/jmap/JMap.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2005, 2018, Oracle and/or its affiliates. All
rights reserved.
+ * Copyright (c) 2005, 2019, Oracle and/or its affiliates. All
rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -30,6 +30,7 @@
import java.io.InputStream;
import java.io.UnsupportedEncodingException;
import java.util.Collection;
+import java.util.List;
import com.sun.tools.attach.VirtualMachine;
import com.sun.tools.attach.VirtualMachineDescriptor;
@@ -125,6 +126,10 @@
private static void executeCommandForPid(String pid, String
command, Object ... args)
throws AttachNotSupportedException, IOException,
UnsupportedEncodingException {
+ // Before attaching, confirm that pid is a known Java process
+ if (!checkJavaPid(pid)) {
+ throw new AttachNotSupportedException();
+ }
VirtualMachine vm = VirtualMachine.attach(pid);
// Cast to HotSpotVirtualMachine as this is an
@@ -196,6 +201,19 @@
executeCommandForPid(pid, "dumpheap", filename, liveopt);
}
+ // Check that pid matches a known running Java process
+ static boolean checkJavaPid(String pid) {
+ List<VirtualMachineDescriptor> l = VirtualMachine.list();
+ boolean found = false;
+ for (VirtualMachineDescriptor vmd: l) {
+ if (vmd.id().equals(pid)) {
+ found = true;
+ break;
+ }
+ }
+ return found;
+ }
+
private static void checkForUnsupportedOptions(String[] args) {
// Check arguments for -F, -m, and non-numeric value
// and warn the user that SA is not supported anymore
