8178023: jshell tool: crash with ugly message on attempt to add non-existant module path
Reviewed-by: jlahoda
--- a/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java Tue Apr 11 14:03:16 2017 +0100
+++ b/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java Tue Apr 11 17:26:52 2017 -0700
@@ -267,7 +267,17 @@
// compiler/runtime init option values
private static class Options {
- private Map<OptionKind, List<String>> optMap = new HashMap<>();
+ private final Map<OptionKind, List<String>> optMap;
+
+ // New blank Options
+ Options() {
+ optMap = new HashMap<>();
+ }
+
+ // Options as a copy
+ private Options(Options opts) {
+ optMap = new HashMap<>(opts.optMap);
+ }
private String[] selectOptions(Predicate<Entry<OptionKind, List<String>>> pred) {
return optMap.entrySet().stream()
@@ -293,17 +303,20 @@
.addAll(vals);
}
- void override(Options newer) {
+ // return a new Options, with parameter options overriding receiver options
+ Options override(Options newer) {
+ Options result = new Options(this);
newer.optMap.entrySet().stream()
.forEach(e -> {
if (e.getKey().onlyOne) {
// Only one allowed, override last
- optMap.put(e.getKey(), e.getValue());
+ result.optMap.put(e.getKey(), e.getValue());
} else {
// Additive
- addAll(e.getKey(), e.getValue());
+ result.addAll(e.getKey(), e.getValue());
}
});
+ return result;
}
}
@@ -874,7 +887,14 @@
// initialize editor settings
configEditor();
// initialize JShell instance
- resetState();
+ try {
+ resetState();
+ } catch (IllegalStateException ex) {
+ // Display just the cause (not a exception backtrace)
+ cmderr.println(ex.getMessage());
+ //abort
+ return;
+ }
// Read replay history from last jshell session into previous history
replayableHistoryPrevious = ReplayableHistory.fromPrevious(prefs);
// load snippet/command files given on command-line
@@ -2570,15 +2590,17 @@
}
private boolean cmdReset(String rawargs) {
+ Options oldOptions = rawargs.trim().isEmpty()? null : options;
if (!parseCommandLineLikeFlags(rawargs, new OptionParserBase())) {
return false;
}
live = false;
fluffmsg("jshell.msg.resetting.state");
- return true;
+ return doReload(null, false, oldOptions);
}
private boolean cmdReload(String rawargs) {
+ Options oldOptions = rawargs.trim().isEmpty()? null : options;
OptionParserReload ap = new OptionParserReload();
if (!parseCommandLineLikeFlags(rawargs, ap)) {
return false;
@@ -2595,7 +2617,7 @@
history = replayableHistory;
fluffmsg("jshell.err.reload.restarting.state");
}
- boolean success = doReload(history, !ap.quiet());
+ boolean success = doReload(history, !ap.quiet(), oldOptions);
if (success && ap.restore()) {
// if we are restoring from previous, then if nothing was added
// before time of exit, there is nothing to save
@@ -2622,17 +2644,32 @@
}
return false;
}
+ Options oldOptions = options;
if (!parseCommandLineLikeFlags(rawargs, new OptionParserBase())) {
return false;
}
fluffmsg("jshell.msg.set.restore");
- return doReload(replayableHistory, false);
+ return doReload(replayableHistory, false, oldOptions);
}
- private boolean doReload(ReplayableHistory history, boolean echo) {
- resetState();
- run(new ReloadIOContext(history.iterable(),
- echo ? cmdout : null));
+ private boolean doReload(ReplayableHistory history, boolean echo, Options oldOptions) {
+ if (oldOptions != null) {
+ try {
+ resetState();
+ } catch (IllegalStateException ex) {
+ currentNameSpace = mainNamespace; // back out of start-up (messages)
+ errormsg("jshell.err.restart.failed", ex.getMessage());
+ // attempt recovery to previous option settings
+ options = oldOptions;
+ resetState();
+ }
+ } else {
+ resetState();
+ }
+ if (history != null) {
+ run(new ReloadIOContext(history.iterable(),
+ echo ? cmdout : null));
+ }
return true;
}
@@ -2648,7 +2685,7 @@
errormsg("jshell.err.unexpected.at.end", ap.nonOptions(), rawargs);
return false;
}
- options.override(opts);
+ options = options.override(opts);
return true;
}
--- a/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties Tue Apr 11 14:03:16 2017 +0100
+++ b/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties Tue Apr 11 17:26:52 2017 -0700
@@ -77,6 +77,9 @@
jshell.err.reload.restarting.previous.state = Restarting and restoring from previous state.
jshell.err.reload.restarting.state = Restarting and restoring state.
+jshell.err.restart.failed = Restart failed: {0}\n\n\
+Reverting to previous settings and restarting...
+
jshell.msg.vars.not.active = (not-active)
jshell.err.out.of.range = Out of range
--- a/langtools/src/jdk.jshell/share/classes/jdk/jshell/execution/FailOverExecutionControlProvider.java Tue Apr 11 14:03:16 2017 +0100
+++ b/langtools/src/jdk.jshell/share/classes/jdk/jshell/execution/FailOverExecutionControlProvider.java Tue Apr 11 17:26:52 2017 -0700
@@ -133,7 +133,10 @@
private Logger logger() {
if (logger == null) {
logger = Logger.getLogger("jdk.jshell.execution");
- logger.setLevel(Level.ALL);
+ if (logger.getLevel() == null) {
+ // Logging has not been specifically requested, turn it off
+ logger.setLevel(Level.OFF);
+ }
}
return logger;
}
--- a/langtools/src/jdk.jshell/share/classes/jdk/jshell/execution/JdiInitiator.java Tue Apr 11 14:03:16 2017 +0100
+++ b/langtools/src/jdk.jshell/share/classes/jdk/jshell/execution/JdiInitiator.java Tue Apr 11 17:26:52 2017 -0700
@@ -26,6 +26,8 @@
import java.io.File;
import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
@@ -146,6 +148,9 @@
*/
private VirtualMachine listenTarget(int port, List<String> remoteVMOptions) {
ListeningConnector listener = (ListeningConnector) connector;
+ // Files to collection to output of a start-up failure
+ File crashErrorFile = createTempFile("error");
+ File crashOutputFile = createTempFile("output");
try {
// Start listening, get the JDI connection address
String addr = listener.startListening(connectorArgs);
@@ -163,23 +168,57 @@
args.add(remoteAgent);
args.add("" + port);
ProcessBuilder pb = new ProcessBuilder(args);
+ pb.redirectError(crashErrorFile);
+ pb.redirectOutput(crashOutputFile);
process = pb.start();
// Accept the connection from the remote agent
vm = timedVirtualMachineCreation(() -> listener.accept(connectorArgs),
() -> process.waitFor());
+ try {
+ listener.stopListening(connectorArgs);
+ } catch (IOException | IllegalConnectorArgumentsException ex) {
+ // ignore
+ }
+ crashErrorFile.delete();
+ crashOutputFile.delete();
return vm;
} catch (Throwable ex) {
if (process != null) {
process.destroyForcibly();
}
- throw reportLaunchFail(ex, "listen");
- } finally {
try {
listener.stopListening(connectorArgs);
- } catch (IOException | IllegalConnectorArgumentsException ex) {
+ } catch (IOException | IllegalConnectorArgumentsException iex) {
// ignore
}
+ String text = readFile(crashErrorFile) + readFile(crashOutputFile);
+ crashErrorFile.delete();
+ crashOutputFile.delete();
+ if (text.isEmpty()) {
+ throw reportLaunchFail(ex, "listen");
+ } else {
+ throw new IllegalArgumentException(text);
+ }
+ }
+ }
+
+ private File createTempFile(String label) {
+ try {
+ File f = File.createTempFile("remote", label);
+ f.deleteOnExit();
+ return f;
+ } catch (IOException ex) {
+ throw new InternalError("Failed create temp ", ex);
+ }
+ }
+
+ private String readFile(File f) {
+ try {
+ return new String(Files.readAllBytes(f.toPath()),
+ StandardCharsets.UTF_8);
+ } catch (IOException ex) {
+ return "error reading " + f + " : " + ex.toString();
}
}
--- a/langtools/test/jdk/jshell/DyingRemoteAgent.java Tue Apr 11 14:03:16 2017 +0100
+++ b/langtools/test/jdk/jshell/DyingRemoteAgent.java Tue Apr 11 17:26:52 2017 -0700
@@ -22,6 +22,8 @@
*/
import java.util.Map;
+import java.util.logging.Level;
+import java.util.logging.Logger;
import jdk.jshell.JShell;
import jdk.jshell.execution.JdiExecutionControlProvider;
import jdk.jshell.execution.RemoteExecutionControl;
@@ -45,6 +47,8 @@
pm.put(JdiExecutionControlProvider.PARAM_REMOTE_AGENT, DyingRemoteAgent.class.getName());
pm.put(JdiExecutionControlProvider.PARAM_HOST_NAME, host==null? "" : host);
pm.put(JdiExecutionControlProvider.PARAM_LAUNCH, ""+isLaunch);
+ // turn on logging of launch failures
+ Logger.getLogger("jdk.jshell.execution").setLevel(Level.ALL);
return JShell.builder()
.executionEngine(ecp, pm)
.build();
--- a/langtools/test/jdk/jshell/HangingRemoteAgent.java Tue Apr 11 14:03:16 2017 +0100
+++ b/langtools/test/jdk/jshell/HangingRemoteAgent.java Tue Apr 11 17:26:52 2017 -0700
@@ -22,6 +22,8 @@
*/
import java.util.Map;
+import java.util.logging.Level;
+import java.util.logging.Logger;
import jdk.jshell.JShell;
import jdk.jshell.execution.JdiExecutionControlProvider;
import jdk.jshell.execution.RemoteExecutionControl;
@@ -59,6 +61,8 @@
pm.put(JdiExecutionControlProvider.PARAM_HOST_NAME, host==null? "" : host);
pm.put(JdiExecutionControlProvider.PARAM_LAUNCH, ""+isLaunch);
pm.put(JdiExecutionControlProvider.PARAM_TIMEOUT, ""+TIMEOUT);
+ // turn on logging of launch failures
+ Logger.getLogger("jdk.jshell.execution").setLevel(Level.ALL);
return JShell.builder()
.executionEngine(ecp, pm)
.build();
--- a/langtools/test/jdk/jshell/HistoryTest.java Tue Apr 11 14:03:16 2017 +0100
+++ b/langtools/test/jdk/jshell/HistoryTest.java Tue Apr 11 17:26:52 2017 -0700
@@ -33,6 +33,8 @@
import java.lang.reflect.Field;
import java.util.Locale;
+import java.util.logging.Level;
+import java.util.logging.Logger;
import jdk.internal.jline.extra.EditingHistory;
import org.testng.annotations.Test;
import jdk.internal.jshell.tool.JShellTool;
@@ -45,6 +47,8 @@
@Override
protected void testRawRun(Locale locale, String[] args) {
+ // turn on logging of launch failures
+ Logger.getLogger("jdk.jshell.execution").setLevel(Level.ALL);
repl = ((JShellToolBuilder) builder(locale))
.rawTool();
try {
--- a/langtools/test/jdk/jshell/JdiBadOptionLaunchExecutionControlTest.java Tue Apr 11 14:03:16 2017 +0100
+++ b/langtools/test/jdk/jshell/JdiBadOptionLaunchExecutionControlTest.java Tue Apr 11 17:26:52 2017 -0700
@@ -29,6 +29,8 @@
* @run testng JdiBadOptionLaunchExecutionControlTest
*/
+import java.util.logging.Level;
+import java.util.logging.Logger;
import org.testng.annotations.Test;
import jdk.jshell.JShell;
import static org.testng.Assert.assertTrue;
@@ -42,6 +44,8 @@
public void badOptionLaunchTest() {
try {
+ // turn on logging of launch failures
+ Logger.getLogger("jdk.jshell.execution").setLevel(Level.ALL);
JShell.builder()
.executionEngine("jdi:launch(true)")
.remoteVMOptions("-BadBadOption")
--- a/langtools/test/jdk/jshell/JdiBadOptionListenExecutionControlTest.java Tue Apr 11 14:03:16 2017 +0100
+++ b/langtools/test/jdk/jshell/JdiBadOptionListenExecutionControlTest.java Tue Apr 11 17:26:52 2017 -0700
@@ -29,6 +29,8 @@
* @run testng JdiBadOptionListenExecutionControlTest
*/
+import java.util.logging.Level;
+import java.util.logging.Logger;
import org.testng.annotations.Test;
import jdk.jshell.JShell;
import static org.testng.Assert.assertTrue;
@@ -38,16 +40,18 @@
public class JdiBadOptionListenExecutionControlTest {
private static final String EXPECTED_ERROR =
- "Launching JShell execution engine threw: Failed remote listen:";
+ "Unrecognized option: -BadBadOption";
public void badOptionListenTest() {
try {
+ // turn on logging of launch failures
+ Logger.getLogger("jdk.jshell.execution").setLevel(Level.ALL);
JShell.builder()
.executionEngine("jdi")
.remoteVMOptions("-BadBadOption")
.build();
} catch (IllegalStateException ex) {
- assertTrue(ex.getMessage().startsWith(EXPECTED_ERROR), ex.getMessage());
+ assertTrue(ex.getMessage().contains(EXPECTED_ERROR), ex.getMessage());
return;
}
fail("Expected IllegalStateException");
--- a/langtools/test/jdk/jshell/JdiBogusHostListenExecutionControlTest.java Tue Apr 11 14:03:16 2017 +0100
+++ b/langtools/test/jdk/jshell/JdiBogusHostListenExecutionControlTest.java Tue Apr 11 17:26:52 2017 -0700
@@ -29,6 +29,8 @@
* @run testng JdiBogusHostListenExecutionControlTest
*/
+import java.util.logging.Level;
+import java.util.logging.Logger;
import org.testng.annotations.Test;
import jdk.jshell.JShell;
import static org.testng.Assert.assertTrue;
@@ -42,6 +44,8 @@
public void badOptionListenTest() {
try {
+ // turn on logging of launch failures
+ Logger.getLogger("jdk.jshell.execution").setLevel(Level.ALL);
JShell.builder()
.executionEngine("jdi:hostname(BattyRumbleBuckets-Snurfle-99-Blip)")
.build();
--- a/langtools/test/jdk/jshell/ReplToolTesting.java Tue Apr 11 14:03:16 2017 +0100
+++ b/langtools/test/jdk/jshell/ReplToolTesting.java Tue Apr 11 17:26:52 2017 -0700
@@ -34,6 +34,8 @@
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Predicate;
+import java.util.logging.Level;
+import java.util.logging.Logger;
import java.util.prefs.AbstractPreferences;
import java.util.prefs.BackingStoreException;
import java.util.regex.Matcher;
@@ -265,6 +267,8 @@
}
protected JavaShellToolBuilder builder(Locale locale) {
+ // turn on logging of launch failures
+ Logger.getLogger("jdk.jshell.execution").setLevel(Level.ALL);
return JavaShellToolBuilder
.builder()
.in(cmdin, userin)
--- a/langtools/test/jdk/jshell/StartOptionTest.java Tue Apr 11 14:03:16 2017 +0100
+++ b/langtools/test/jdk/jshell/StartOptionTest.java Tue Apr 11 17:26:52 2017 -0700
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017, 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
@@ -22,7 +22,7 @@
*/
/*
- * @test 8151754 8080883 8160089 8170162 8166581 8172102 8171343
+ * @test 8151754 8080883 8160089 8170162 8166581 8172102 8171343 8178023
* @summary Testing start-up options.
* @modules jdk.compiler/com.sun.tools.javac.api
* jdk.compiler/com.sun.tools.javac.main
@@ -43,6 +43,8 @@
import java.util.Locale;
import java.util.function.Consumer;
+import java.util.logging.Level;
+import java.util.logging.Logger;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
@@ -63,6 +65,8 @@
private InputStream cmdInStream;
private JavaShellToolBuilder builder() {
+ // turn on logging of launch failures
+ Logger.getLogger("jdk.jshell.execution").setLevel(Level.ALL);
return JavaShellToolBuilder
.builder()
.out(new PrintStream(cmdout), new PrintStream(console), new PrintStream(userout))
@@ -179,14 +183,11 @@
}
public void testStartupFailedOption() throws Exception {
- try {
- builder().run("-R-hoge-foo-bar");
- } catch (IllegalStateException ex) {
- String s = ex.getMessage();
- assertTrue(s.startsWith("Launching JShell execution engine threw: Failed remote"), s);
- return;
- }
- fail("Expected IllegalStateException");
+ start(
+ s -> assertEquals(s.trim(), "", "cmdout: "),
+ s -> assertEquals(s.trim(), "", "userout: "),
+ s -> assertTrue(s.contains("Unrecognized option: -hoge-foo-bar"), "cmderr: " + s),
+ "-R-hoge-foo-bar");
}
public void testStartupUnknown() throws Exception {
@@ -201,6 +202,14 @@
}
}
+ public void testUnknownModule() throws Exception {
+ start(
+ s -> assertEquals(s.trim(), "", "cmdout: "),
+ s -> assertEquals(s.trim(), "", "userout: "),
+ s -> assertTrue(s.contains("rror") && s.contains("unKnown"), "cmderr: " + s),
+ "--add-modules", "unKnown");
+ }
+
public void testFeedbackOptionConflict() throws Exception {
start("", "Only one feedback option (--feedback, -q, -s, or -v) may be used.",
"--feedback", "concise", "--feedback", "verbose");
--- a/langtools/test/jdk/jshell/ToolProviderTest.java Tue Apr 11 14:03:16 2017 +0100
+++ b/langtools/test/jdk/jshell/ToolProviderTest.java Tue Apr 11 17:26:52 2017 -0700
@@ -87,15 +87,6 @@
}
@Override
- public void testStartupFailedOption() throws Exception {
- if (runShellServiceLoader("-R-hoge-foo-bar") == 0) {
- fail("Expected tool failure");
- } else {
- check(cmderr, s -> s.startsWith("Launching JShell execution engine threw: Failed remote"), "cmderr");
- }
- }
-
- @Override
public void testShowVersion() throws Exception {
start(
s -> {
--- a/langtools/test/jdk/jshell/ToolReloadTest.java Tue Apr 11 14:03:16 2017 +0100
+++ b/langtools/test/jdk/jshell/ToolReloadTest.java Tue Apr 11 17:26:52 2017 -0700
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017, 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
@@ -24,7 +24,7 @@
/*
* @test
* @key intermittent
- * @bug 8081845 8147898 8143955 8165405
+ * @bug 8081845 8147898 8143955 8165405 8178023
* @summary Tests for /reload in JShell tool
* @modules jdk.compiler/com.sun.tools.javac.api
* jdk.compiler/com.sun.tools.javac.main
@@ -40,6 +40,7 @@
import java.util.function.Function;
import org.testng.annotations.Test;
+import static org.testng.Assert.assertTrue;
@Test
@@ -199,6 +200,27 @@
);
}
+ public void testEnvBadModule() {
+ test(
+ (a) -> assertVariable(a, "int", "x", "5", "5"),
+ (a) -> assertMethod(a, "int m(int z) { return z * z; }",
+ "(int)int", "m"),
+ (a) -> assertCommandCheckOutput(a, "/env --add-module unKnown",
+ s -> {
+ assertTrue(s.startsWith(
+ "| Setting new options and restoring state.\n" +
+ "| Restart failed:"));
+ assertTrue(s.contains("unKnown"),
+ "\"unKnown\" missing from: " + s);
+ assertTrue(s.contains("previous settings"),
+ "\"previous settings\" missing from: " + s);
+ }),
+ (a) -> evaluateExpression(a, "int", "m(x)", "25"),
+ (a) -> assertCommandCheckOutput(a, "/vars", assertVariables()),
+ (a) -> assertCommandCheckOutput(a, "/methods", assertMethods())
+ );
+ }
+
public void testReloadExitRestore() {
test(false, new String[]{"--no-startup"},
(a) -> assertVariable(a, "int", "x", "5", "5"),
--- a/langtools/test/jdk/jshell/UITesting.java Tue Apr 11 14:03:16 2017 +0100
+++ b/langtools/test/jdk/jshell/UITesting.java Tue Apr 11 17:26:52 2017 -0700
@@ -29,6 +29,8 @@
import java.io.Writer;
import java.util.HashMap;
import java.util.Locale;
+import java.util.logging.Level;
+import java.util.logging.Logger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -37,6 +39,9 @@
public class UITesting {
protected void doRunTest(Test test) throws Exception {
+ // turn on logging of launch failures
+ Logger.getLogger("jdk.jshell.execution").setLevel(Level.ALL);
+
PipeInputStream input = new PipeInputStream();
StringBuilder out = new StringBuilder();
PrintStream outS = new PrintStream(new OutputStream() {