Mercurial > hg > openjdk > jdk9 > langtools
changeset 4097:256d9fce6c53
8178023: jshell tool: crash with ugly message on attempt to add non-existant module path
Reviewed-by: jlahoda
line wrap: on
line diff
--- a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java Tue Apr 11 14:03:16 2017 +0100 +++ b/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/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties Tue Apr 11 14:03:16 2017 +0100 +++ b/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/src/jdk.jshell/share/classes/jdk/jshell/execution/FailOverExecutionControlProvider.java Tue Apr 11 14:03:16 2017 +0100 +++ b/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/src/jdk.jshell/share/classes/jdk/jshell/execution/JdiInitiator.java Tue Apr 11 14:03:16 2017 +0100 +++ b/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/test/jdk/jshell/DyingRemoteAgent.java Tue Apr 11 14:03:16 2017 +0100 +++ b/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/test/jdk/jshell/HangingRemoteAgent.java Tue Apr 11 14:03:16 2017 +0100 +++ b/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/test/jdk/jshell/HistoryTest.java Tue Apr 11 14:03:16 2017 +0100 +++ b/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/test/jdk/jshell/JdiBadOptionLaunchExecutionControlTest.java Tue Apr 11 14:03:16 2017 +0100 +++ b/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/test/jdk/jshell/JdiBadOptionListenExecutionControlTest.java Tue Apr 11 14:03:16 2017 +0100 +++ b/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/test/jdk/jshell/JdiBogusHostListenExecutionControlTest.java Tue Apr 11 14:03:16 2017 +0100 +++ b/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/test/jdk/jshell/ReplToolTesting.java Tue Apr 11 14:03:16 2017 +0100 +++ b/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/test/jdk/jshell/StartOptionTest.java Tue Apr 11 14:03:16 2017 +0100 +++ b/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/test/jdk/jshell/ToolProviderTest.java Tue Apr 11 14:03:16 2017 +0100 +++ b/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/test/jdk/jshell/ToolReloadTest.java Tue Apr 11 14:03:16 2017 +0100 +++ b/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/test/jdk/jshell/UITesting.java Tue Apr 11 14:03:16 2017 +0100 +++ b/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() {