8156537: Tools using MonitoredVmUtil do not parse module in cmdline correctly
Reviewed-by: dsamersoff, sla
--- a/jdk/src/jdk.jcmd/share/classes/sun/tools/common/ProcessArgumentMatcher.java Thu Jun 02 14:58:53 2016 -0700
+++ b/jdk/src/jdk.jcmd/share/classes/sun/tools/common/ProcessArgumentMatcher.java Fri Jun 03 10:05:04 2016 +0200
@@ -25,6 +25,7 @@
package sun.tools.common;
+import java.lang.reflect.Module;
import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.Collection;
@@ -45,13 +46,10 @@
* the process identifiers.
*/
public class ProcessArgumentMatcher {
- private String excludeCls;
- private String matchClass = null;
- private String singlePid = null;
- private boolean matchAll = false;
+ private String matchClass;
+ private String singlePid;
- public ProcessArgumentMatcher(String pidArg, Class<?> excludeClass) {
- excludeCls = excludeClass.getName();
+ public ProcessArgumentMatcher(String pidArg) {
if (pidArg == null || pidArg.isEmpty()) {
throw new IllegalArgumentException("Pid string is invalid");
}
@@ -60,9 +58,7 @@
}
try {
long pid = Long.parseLong(pidArg);
- if (pid == 0) {
- matchAll = true;
- } else {
+ if (pid != 0) {
singlePid = String.valueOf(pid);
}
} catch (NumberFormatException nfe) {
@@ -70,7 +66,18 @@
}
}
- private boolean check(VirtualMachineDescriptor vmd) {
+ private static String getExcludeStringFrom(Class<?> excludeClass) {
+ if (excludeClass == null) {
+ return "";
+ }
+ Module m = excludeClass.getModule();
+ if (m.isNamed()) {
+ return m.getName() + "/" + excludeClass.getName();
+ }
+ return excludeClass.getName();
+ }
+
+ private static boolean check(VirtualMachineDescriptor vmd, String excludeClass, String partialMatch) {
String mainClass = null;
try {
VmIdentifier vmId = new VmIdentifier(vmd.id());
@@ -87,42 +94,55 @@
// Handle this gracefully....
return false;
} catch (MonitorException | URISyntaxException e) {
- if (e.getMessage() != null) {
- System.err.println(e.getMessage());
- } else {
- Throwable cause = e.getCause();
- if ((cause != null) && (cause.getMessage() != null)) {
- System.err.println(cause.getMessage());
- } else {
- e.printStackTrace();
- }
- }
return false;
}
- if (mainClass.equals(excludeCls)) {
+ if (excludeClass != null && mainClass.equals(excludeClass)) {
+ return false;
+ }
+
+ if (partialMatch != null && mainClass.indexOf(partialMatch) == -1) {
return false;
}
- if (matchAll || mainClass.indexOf(matchClass) != -1) {
- return true;
+ return true;
+ }
+
+ private static Collection<VirtualMachineDescriptor> getSingleVMD(String pid) {
+ Collection<VirtualMachineDescriptor> vids = new ArrayList<>();
+ List<VirtualMachineDescriptor> vmds = VirtualMachine.list();
+ for (VirtualMachineDescriptor vmd : vmds) {
+ if (check(vmd, null, null)) {
+ if (pid.equals(vmd.id())) {
+ vids.add(vmd);
+ }
+ }
}
-
- return false;
+ return vids;
}
- public Collection<String> getPids() {
- Collection<String> pids = new ArrayList<>();
- if (singlePid != null) {
- pids.add(singlePid);
- return pids;
- }
+ private static Collection<VirtualMachineDescriptor> getVMDs(Class<?> excludeClass, String partialMatch) {
+ String excludeCls = getExcludeStringFrom(excludeClass);
+ Collection<VirtualMachineDescriptor> vids = new ArrayList<>();
List<VirtualMachineDescriptor> vmds = VirtualMachine.list();
for (VirtualMachineDescriptor vmd : vmds) {
- if (check(vmd)) {
- pids.add(vmd.id());
+ if (check(vmd, excludeCls, partialMatch)) {
+ vids.add(vmd);
}
}
- return pids;
+ return vids;
}
+
+ public Collection<VirtualMachineDescriptor> getVirtualMachineDescriptors(Class<?> excludeClass) {
+ if (singlePid != null) {
+ return getSingleVMD(singlePid);
+ } else {
+ return getVMDs(excludeClass, matchClass);
+ }
+ }
+
+ public Collection<VirtualMachineDescriptor> getVirtualMachineDescriptors() {
+ return this.getVirtualMachineDescriptors(null);
+ }
+
}
--- a/jdk/src/jdk.jcmd/share/classes/sun/tools/jcmd/Arguments.java Thu Jun 02 14:58:53 2016 -0700
+++ b/jdk/src/jdk.jcmd/share/classes/sun/tools/jcmd/Arguments.java Fri Jun 03 10:05:04 2016 +0200
@@ -45,6 +45,8 @@
public Arguments(String[] args) {
if (args.length == 0 || args[0].equals("-l")) {
listProcesses = true;
+ /* list all processes */
+ processString = "0";
return;
}
--- a/jdk/src/jdk.jcmd/share/classes/sun/tools/jcmd/JCmd.java Thu Jun 02 14:58:53 2016 -0700
+++ b/jdk/src/jdk.jcmd/share/classes/sun/tools/jcmd/JCmd.java Fri Jun 03 10:05:04 2016 +0200
@@ -65,36 +65,37 @@
System.exit(1);
}
+ ProcessArgumentMatcher ap = null;
+ try {
+ ap = new ProcessArgumentMatcher(arg.getProcessString());
+ } catch (IllegalArgumentException iae) {
+ System.err.println("Invalid pid '" + arg.getProcessString() + "' specified");
+ System.exit(1);
+ }
+
if (arg.isListProcesses()) {
- List<VirtualMachineDescriptor> vmds = VirtualMachine.list();
- for (VirtualMachineDescriptor vmd : vmds) {
+ for (VirtualMachineDescriptor vmd : ap.getVirtualMachineDescriptors(/* include jcmd in listing */)) {
System.out.println(vmd.id() + " " + vmd.displayName());
}
System.exit(0);
}
- Collection<String> pids = Collections.emptyList();
- try {
- ProcessArgumentMatcher ap = new ProcessArgumentMatcher(arg.getProcessString(), JCmd.class);
- pids = ap.getPids();
- } catch (IllegalArgumentException iae) {
- System.err.println("Invalid pid specified");
- System.exit(1);
- }
- if (pids.isEmpty()) {
+ Collection<VirtualMachineDescriptor> vids = ap.getVirtualMachineDescriptors(JCmd.class);
+
+ if (vids.isEmpty()) {
System.err.println("Could not find any processes matching : '"
+ arg.getProcessString() + "'");
System.exit(1);
}
boolean success = true;
- for (String pid : pids) {
- System.out.println(pid + ":");
+ for (VirtualMachineDescriptor vid : vids) {
+ System.out.println(vid.id() + ":");
if (arg.isListCounters()) {
- listCounters(pid);
+ listCounters(vid.id());
} else {
try {
- executeCommandForPid(pid, arg.getCommand());
+ executeCommandForPid(vid.id(), arg.getCommand());
} catch(AttachOperationFailedException ex) {
System.err.println(ex.getMessage());
success = false;
--- a/jdk/src/jdk.jcmd/share/classes/sun/tools/jinfo/JInfo.java Thu Jun 02 14:58:53 2016 -0700
+++ b/jdk/src/jdk.jcmd/share/classes/sun/tools/jinfo/JInfo.java Fri Jun 03 10:05:04 2016 +0200
@@ -30,6 +30,7 @@
import java.util.Collection;
import com.sun.tools.attach.VirtualMachine;
+import com.sun.tools.attach.VirtualMachineDescriptor;
import sun.tools.attach.HotSpotVirtualMachine;
import sun.tools.common.ProcessArgumentMatcher;
@@ -50,6 +51,7 @@
boolean doFlag = false;
boolean doFlags = false;
boolean doSysprops = false;
+ int flag = -1;
// Parse the options (arguments starting with "-" )
int optionCount = 0;
@@ -67,65 +69,64 @@
if (arg.equals("-flag")) {
doFlag = true;
- continue;
+ // Consume the flag
+ if (optionCount < args.length) {
+ flag = optionCount++;
+ break;
+ }
+ usage(1);
}
if (arg.equals("-flags")) {
doFlags = true;
- continue;
+ break;
}
if (arg.equals("-sysprops")) {
doSysprops = true;
- continue;
+ break;
}
}
- // Next we check the parameter count. -flag allows extra parameters
int paramCount = args.length - optionCount;
- if ((doFlag && paramCount != 2) || ((!doFlag && paramCount != 1))) {
+ if (paramCount != 1) {
usage(1);
}
- if (!doFlag && !doFlags && !doSysprops) {
- // Print flags and sysporps if no options given
- ProcessArgumentMatcher ap = new ProcessArgumentMatcher(args[optionCount], JInfo.class);
- Collection<String> pids = ap.getPids();
- for (String pid : pids) {
- if (pids.size() > 1) {
- System.out.println("Pid:" + pid);
- }
- sysprops(pid);
- System.out.println();
- flags(pid);
- System.out.println();
- commandLine(pid);
- }
+ String parg = args[optionCount];
+
+ ProcessArgumentMatcher ap = new ProcessArgumentMatcher(parg);
+ Collection<VirtualMachineDescriptor> vids = ap.getVirtualMachineDescriptors(JInfo.class);
+
+ if (vids.isEmpty()) {
+ System.err.println("Could not find any processes matching : '" + parg + "'");
+ System.exit(1);
}
- if (doFlag) {
- ProcessArgumentMatcher ap = new ProcessArgumentMatcher(args[optionCount+1], JInfo.class);
- Collection<String> pids = ap.getPids();
- for (String pid : pids) {
- if (pids.size() > 1) {
- System.out.println("Pid:" + pid);
- }
- flag(pid, args[optionCount]);
+ for (VirtualMachineDescriptor vid : vids) {
+ if (vids.size() > 1) {
+ System.out.println("Pid:" + vid.id());
+ }
+ if (!doFlag && !doFlags && !doSysprops) {
+ // Print flags and sysporps if no options given
+ sysprops(vid.id());
+ System.out.println();
+ flags(vid.id());
+ System.out.println();
+ commandLine(vid.id());
}
- }
- else if (doFlags || doSysprops) {
- ProcessArgumentMatcher ap = new ProcessArgumentMatcher(args[optionCount], JInfo.class);
- Collection<String> pids = ap.getPids();
- for (String pid : pids) {
- if (pids.size() > 1) {
- System.out.println("Pid:" + pid);
+ if (doFlag) {
+ if (flag < 0) {
+ System.err.println("Missing flag");
+ usage(1);
}
- if (doFlags) {
- flags(pid);
- }
- else if (doSysprops) {
- sysprops(pid);
- }
+ flag(vid.id(), args[flag]);
+ }
+ if (doFlags) {
+ flags(vid.id());
+ }
+ if (doSysprops) {
+ sysprops(vid.id());
}
}
}
--- a/jdk/src/jdk.jcmd/share/classes/sun/tools/jmap/JMap.java Thu Jun 02 14:58:53 2016 -0700
+++ b/jdk/src/jdk.jcmd/share/classes/sun/tools/jmap/JMap.java Fri Jun 03 10:05:04 2016 +0200
@@ -32,6 +32,7 @@
import java.util.Collection;
import com.sun.tools.attach.VirtualMachine;
+import com.sun.tools.attach.VirtualMachineDescriptor;
import com.sun.tools.attach.AttachNotSupportedException;
import sun.tools.attach.HotSpotVirtualMachine;
import sun.tools.common.ProcessArgumentMatcher;
@@ -89,10 +90,17 @@
// Here we handle the built-in options
// As more options are added we should create an abstract tool class and
// have a table to map the options
- ProcessArgumentMatcher ap = new ProcessArgumentMatcher(pidArg, JMap.class);
- Collection<String> pids = ap.getPids();
- for (String pid : pids) {
- if (pids.size() > 1) {
+ ProcessArgumentMatcher ap = new ProcessArgumentMatcher(pidArg);
+ Collection<VirtualMachineDescriptor> vids = ap.getVirtualMachineDescriptors(JMap.class);
+
+ if (vids.isEmpty()) {
+ System.err.println("Could not find any processes matching : '" + pidArg + "'");
+ System.exit(1);
+ }
+
+ for (VirtualMachineDescriptor vid : vids) {
+ String pid = vid.id();
+ if (vids.size() > 1) {
System.out.println("Pid:" + pid);
}
if (option.equals("-histo")) {
--- a/jdk/src/jdk.jcmd/share/classes/sun/tools/jstack/JStack.java Thu Jun 02 14:58:53 2016 -0700
+++ b/jdk/src/jdk.jcmd/share/classes/sun/tools/jstack/JStack.java Fri Jun 03 10:05:04 2016 +0200
@@ -29,6 +29,7 @@
import java.util.Collection;
import com.sun.tools.attach.VirtualMachine;
+import com.sun.tools.attach.VirtualMachineDescriptor;
import sun.tools.attach.HotSpotVirtualMachine;
import sun.tools.common.ProcessArgumentMatcher;
@@ -82,13 +83,19 @@
} else {
params = new String[0];
}
- ProcessArgumentMatcher ap = new ProcessArgumentMatcher(pidArg, JStack.class);
- Collection<String> pids = ap.getPids();
- for (String pid : pids) {
- if (pids.size() > 1) {
- System.out.println("Pid:" + pid);
+ ProcessArgumentMatcher ap = new ProcessArgumentMatcher(pidArg);
+ Collection<VirtualMachineDescriptor> vids = ap.getVirtualMachineDescriptors(JStack.class);
+
+ if (vids.isEmpty()) {
+ System.err.println("Could not find any processes matching : '" + pidArg + "'");
+ System.exit(1);
+ }
+
+ for (VirtualMachineDescriptor vid : vids) {
+ if (vids.size() > 1) {
+ System.out.println("Pid:" + vid.id());
}
- runThreadDump(pid, params);
+ runThreadDump(vid.id(), params);
}
}
--- a/jdk/src/jdk.jvmstat/share/classes/sun/jvmstat/monitor/MonitoredVmUtil.java Thu Jun 02 14:58:53 2016 -0700
+++ b/jdk/src/jdk.jvmstat/share/classes/sun/jvmstat/monitor/MonitoredVmUtil.java Fri Jun 03 10:05:04 2016 +0200
@@ -102,35 +102,42 @@
*/
public static String mainClass(MonitoredVm vm, boolean fullPath)
throws MonitorException {
- String commandLine = commandLine(vm);
- String arg0 = commandLine;
-
- int firstSpace = commandLine.indexOf(' ');
+ String cmdLine = commandLine(vm);
+ int firstSpace = cmdLine.indexOf(' ');
if (firstSpace > 0) {
- arg0 = commandLine.substring(0, firstSpace);
+ cmdLine = cmdLine.substring(0, firstSpace);
+ }
+ if (fullPath) {
+ return cmdLine;
}
- if (!fullPath) {
+ /*
+ * Can't use File.separator() here because the separator for the target
+ * jvm may be different than the separator for the monitoring jvm.
+ * And we also strip embedded module e.g. "module/MainClass"
+ */
+ int lastSlash = cmdLine.lastIndexOf("/");
+ int lastBackslash = cmdLine.lastIndexOf("\\");
+ int lastSeparator = lastSlash > lastBackslash ? lastSlash : lastBackslash;
+ if (lastSeparator > 0) {
+ cmdLine = cmdLine.substring(lastSeparator + 1);
+ }
+
+ int lastPackageSeparator = cmdLine.lastIndexOf('.');
+ if (lastPackageSeparator > 0) {
+ String lastPart = cmdLine.substring(lastPackageSeparator + 1);
/*
- * can't use File.separator() here because the separator
- * for the target jvm may be different than the separator
- * for the monitoring jvm.
+ * We could have a relative path "my.module" or
+ * a module called "my.module" and a jar file called "my.jar" or
+ * class named "jar" in package "my", e.g. "my.jar".
+ * We can never be sure here, but we assume *.jar is a jar file
*/
- int lastFileSeparator = arg0.lastIndexOf('/');
- if (lastFileSeparator > 0) {
- return arg0.substring(lastFileSeparator + 1);
+ if (lastPart.equals("jar")) {
+ return cmdLine; /* presumably a file name without path */
}
+ return lastPart; /* presumably a class name without package */
+ }
- lastFileSeparator = arg0.lastIndexOf('\\');
- if (lastFileSeparator > 0) {
- return arg0.substring(lastFileSeparator + 1);
- }
-
- int lastPackageSeparator = arg0.lastIndexOf('.');
- if (lastPackageSeparator > 0) {
- return arg0.substring(lastPackageSeparator + 1);
- }
- }
- return arg0;
+ return cmdLine;
}
/**
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/sun/tools/jinfo/JInfoTest.java Fri Jun 03 10:05:04 2016 +0200
@@ -0,0 +1,124 @@
+/*
+ * Copyright (c) 2005, 2016, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.io.IOException;
+
+import jdk.test.lib.JDKToolLauncher;
+import jdk.test.lib.process.OutputAnalyzer;
+import jdk.test.lib.process.ProcessTools;
+import jdk.test.lib.apps.LingeredApp;
+
+/*
+ * @test
+ * @summary Unit test for jinfo utility
+ * @modules java.base/jdk.internal.misc
+ * @library /test/lib/share/classes
+ * @build jdk.test.lib.*
+ * @build jdk.test.lib.apps.*
+ * @build jdk.test.lib.process.*
+ * @run main JInfoTest
+ */
+public class JInfoTest {
+
+ private static ProcessBuilder processBuilder = new ProcessBuilder();
+
+ public static void main(String[] args) throws Exception {
+ classNameMatch();
+ setMultipleFlags();
+ setFlag();
+ }
+
+ private static void setFlag() throws Exception {
+ System.out.println("#### setFlag ####");
+ LingeredApp app1 = new JInfoTestLingeredApp();
+ LingeredApp app2 = new JInfoTestLingeredApp();
+ try {
+ ArrayList<String> params = new ArrayList<String>();
+ LingeredApp.startApp(params, app1);
+ LingeredApp.startApp(params, app2);
+ OutputAnalyzer output = jinfo("-flag", "MinHeapFreeRatio=1", "JInfoTestLingeredApp");
+ output.shouldHaveExitValue(0);
+ output = jinfo("-flag", "MinHeapFreeRatio", "JInfoTestLingeredApp");
+ output.shouldHaveExitValue(0);
+ documentMatch(output.getStdout(), ".*MinHeapFreeRatio=1.*MinHeapFreeRatio=1.*");
+ } finally {
+ JInfoTestLingeredApp.stopApp(app1);
+ JInfoTestLingeredApp.stopApp(app2);
+ }
+ }
+
+ private static void setMultipleFlags() throws Exception {
+ System.out.println("#### setMultipleFlags ####");
+ OutputAnalyzer output = jinfo("-sysprops", "-flag", "MinHeapFreeRatio=1", "-flags", "JInfoTestLingeredApp");
+ output.shouldHaveExitValue(1);
+ }
+
+ private static void classNameMatch() throws Exception {
+ System.out.println("#### classNameMatch ####");
+ LingeredApp app1 = new JInfoTestLingeredApp();
+ LingeredApp app2 = new JInfoTestLingeredApp();
+ try {
+ ArrayList<String> params = new ArrayList<String>();
+ LingeredApp.startApp(params, app1);
+ LingeredApp.startApp(params, app2);
+ OutputAnalyzer output = jinfo("JInfoTestLingeredApp");
+ output.shouldHaveExitValue(0);
+ // "HotSpot(TM)" written once per proc
+ documentMatch(output.getStdout(), ".*HotSpot\\(TM\\).*HotSpot\\(TM\\).*");
+ } finally {
+ JInfoTestLingeredApp.stopApp(app1);
+ JInfoTestLingeredApp.stopApp(app2);
+ }
+ }
+
+ private static void documentMatch(String data, String pattern){
+ Matcher matcher = Pattern.compile(pattern, Pattern.DOTALL).matcher(data);
+ if (!matcher.find()) {
+ throw new RuntimeException("'" + pattern + "' missing from stdout \n");
+ }
+ }
+
+ private static OutputAnalyzer jinfo(String... toolArgs) throws Exception {
+ JDKToolLauncher launcher = JDKToolLauncher.createUsingTestJDK("jinfo");
+ if (toolArgs != null) {
+ for (String toolArg : toolArgs) {
+ launcher.addToolArg(toolArg);
+ }
+ }
+
+ processBuilder.command(launcher.getCommand());
+ OutputAnalyzer output = ProcessTools.executeProcess(processBuilder);
+
+ return output;
+ }
+}
+
+// Sometime there is LingeredApp's from other test still around
+class JInfoTestLingeredApp extends LingeredApp {
+}
+
--- a/jdk/test/sun/tools/jps/TestJpsSanity.java Thu Jun 02 14:58:53 2016 -0700
+++ b/jdk/test/sun/tools/jps/TestJpsSanity.java Fri Jun 03 10:05:04 2016 +0200
@@ -23,15 +23,17 @@
import jdk.testlibrary.Asserts;
import jdk.testlibrary.OutputAnalyzer;
+import jdk.test.lib.apps.LingeredApp;
/*
* @test
* @summary This test verifies jps usage and checks that appropriate error message is shown
* when running jps with illegal arguments.
- * @library /lib/testlibrary
+ * @library /lib/testlibrary /test/lib/share/classes
* @modules jdk.jartool/sun.tools.jar
* java.management
- * @build jdk.testlibrary.* JpsHelper
+ * java.base/jdk.internal.misc
+ * @build jdk.testlibrary.* jdk.test.lib.apps.* JpsHelper
* @run driver TestJpsSanity
*/
public class TestJpsSanity {
@@ -40,6 +42,42 @@
testJpsUsage();
testJpsVersion();
testJpsUnknownHost();
+ testJpsShort();
+ testJpsLong();
+ testJpsShortPkg();
+ testJpsLongPkg();
+ }
+
+ private static void testJpsShort() throws Exception {
+ OutputAnalyzer output = JpsHelper.jps();
+ output.shouldMatch("^[0-9]+ Jps$");
+ }
+
+ private static void testJpsLong() throws Exception {
+ OutputAnalyzer output = JpsHelper.jps("-l");
+ output.shouldMatch("^[0-9]+ jdk\\.jcmd/sun\\.tools\\.jps\\.Jps$");
+ }
+
+ private static void testJpsShortPkg() throws Exception {
+ LingeredApp app = null;
+ try {
+ app = LingeredApp.startApp();
+ OutputAnalyzer output = JpsHelper.jps();
+ output.shouldMatch("^[0-9]+ LingeredApp$");
+ } finally {
+ LingeredApp.stopApp(app);
+ }
+ }
+
+ private static void testJpsLongPkg() throws Exception {
+ LingeredApp app = null;
+ try {
+ app = LingeredApp.startApp();
+ OutputAnalyzer output = JpsHelper.jps("-l");
+ output.shouldMatch("^[0-9]+ jdk\\.test\\.lib\\.apps\\.LingeredApp$");
+ } finally {
+ LingeredApp.stopApp(app);
+ }
}
private static void testJpsUsage() throws Exception {