OpenJDK / portola / portola
changeset 38608:691b607bbcd6
8157917: JShell: shutdown could cause remote JDWP errors to be visible to users
8157918: JShell tests: StartOptionTest displays insufficient information to diagnose failures
Reviewed-by: vromero
author | rfield |
---|---|
date | Thu, 26 May 2016 07:58:01 -0700 |
parents | eb4a0b7e67a1 |
children | a1c0c73078b3 |
files | langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/jdi/JDIConnection.java langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/jdi/JDIExecutionControl.java langtools/test/jdk/jshell/StartOptionTest.java |
diffstat | 3 files changed, 58 insertions(+), 51 deletions(-) [+] |
line wrap: on
line diff
--- a/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/jdi/JDIConnection.java Thu May 26 18:22:05 2016 +0530 +++ b/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/jdi/JDIConnection.java Thu May 26 07:58:01 2016 -0700 @@ -50,6 +50,7 @@ class JDIConnection { private VirtualMachine vm; + private boolean active = true; private Process process = null; private int outputCompleteCount = 0; @@ -175,6 +176,11 @@ return process != null && process.isAlive(); } + // Beginning shutdown, ignore any random dying squeals + void beginShutdown() { + active = false; + } + public synchronized void disposeVM() { try { if (vm != null) { @@ -233,14 +239,19 @@ int i; try { while ((i = in.read()) != -1) { - pStream.print((char) i); + // directly copy input to output, but skip if asked to close + if (active) { + pStream.print((char) i); + } } } catch (IOException ex) { String s = ex.getMessage(); - if (!s.startsWith("Bad file number")) { + if (active && !s.startsWith("Bad file number")) { throw ex; } - // else we got a Bad file number IOException which just means + // else we are being shutdown (and don't want any spurious death + // throws to ripple) or + // we got a Bad file number IOException which just means // that the debuggee has gone away. We'll just treat it the // same as if we got an EOF. }
--- a/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/jdi/JDIExecutionControl.java Thu May 26 18:22:05 2016 +0530 +++ b/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/jdi/JDIExecutionControl.java Thu May 26 07:58:01 2016 -0700 @@ -109,11 +109,14 @@ @Override public void close() { try { + JDIConnection c = jdiEnv.connection(); + if (c != null) { + c.beginShutdown(); + } if (remoteOut != null) { remoteOut.writeInt(CMD_EXIT); remoteOut.flush(); } - JDIConnection c = jdiEnv.connection(); if (c != null) { c.disposeVM(); }
--- a/langtools/test/jdk/jshell/StartOptionTest.java Thu May 26 18:22:05 2016 +0530 +++ b/langtools/test/jdk/jshell/StartOptionTest.java Thu May 26 07:58:01 2016 -0700 @@ -34,7 +34,6 @@ */ import java.io.ByteArrayOutputStream; -import java.io.OutputStream; import java.io.PrintStream; import java.nio.charset.StandardCharsets; import java.nio.file.Path; @@ -48,85 +47,76 @@ import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertTrue; -import static org.testng.Assert.fail; @Test public class StartOptionTest { - private ByteArrayOutputStream out; - private ByteArrayOutputStream err; + private ByteArrayOutputStream cmdout; + private ByteArrayOutputStream cmderr; + private ByteArrayOutputStream console; + private ByteArrayOutputStream userout; + private ByteArrayOutputStream usererr; private JShellTool getShellTool() { - class NoOutputAllowedStream extends OutputStream { - private final String label; - NoOutputAllowedStream(String label) { - this.label = label; - } - @Override - public void write(int b) { fail("Unexpected output to: " + label); } - } return new JShellTool( new TestingInputStream(), - new PrintStream(out), - new PrintStream(err), - new PrintStream(new NoOutputAllowedStream("console")), + new PrintStream(cmdout), + new PrintStream(cmderr), + new PrintStream(console), new TestingInputStream(), - new PrintStream(new NoOutputAllowedStream("userout")), - new PrintStream(new NoOutputAllowedStream("usererr")), + new PrintStream(userout), + new PrintStream(usererr), new ReplToolTesting.MemoryPreferences(), Locale.ROOT); } - private String getOutput() { - byte[] bytes = out.toByteArray(); - out.reset(); - return new String(bytes, StandardCharsets.UTF_8); - } - - private String getError() { - byte[] bytes = err.toByteArray(); - err.reset(); - return new String(bytes, StandardCharsets.UTF_8); + private void check(ByteArrayOutputStream str, Consumer<String> checkOut, String label) { + byte[] bytes = str.toByteArray(); + str.reset(); + String out = new String(bytes, StandardCharsets.UTF_8); + if (checkOut != null) { + checkOut.accept(out); + } else { + assertEquals("", out, label + ": Expected empty -- "); + } } private void start(Consumer<String> checkOutput, Consumer<String> checkError, String... args) throws Exception { JShellTool tool = getShellTool(); tool.start(args); - if (checkOutput != null) { - checkOutput.accept(getOutput()); - } else { - assertEquals("", getOutput(), "Output: "); - } - if (checkError != null) { - checkError.accept(getError()); - } else { - assertEquals("", getError(), "Error: "); - } + check(cmdout, checkOutput, "cmdout"); + check(cmderr, checkError, "cmderr"); + check(console, null, "console"); + check(userout, null, "userout"); + check(usererr, null, "usererr"); } private void start(String expectedOutput, String expectedError, String... args) throws Exception { - start(s -> assertEquals(s.trim(), expectedOutput, "Output: "), s -> assertEquals(s.trim(), expectedError, "Error: "), args); + start(s -> assertEquals(s.trim(), expectedOutput, "cmdout: "), s -> assertEquals(s.trim(), expectedError, "cmderr: "), args); } @BeforeMethod public void setUp() { - out = new ByteArrayOutputStream(); - err = new ByteArrayOutputStream(); + cmdout = new ByteArrayOutputStream(); + cmderr = new ByteArrayOutputStream(); + console = new ByteArrayOutputStream(); + userout = new ByteArrayOutputStream(); + usererr = new ByteArrayOutputStream(); } @Test public void testUsage() throws Exception { start(s -> { - assertTrue(s.split("\n").length >= 7, s); - assertTrue(s.startsWith("Usage: jshell <options>"), s); + assertTrue(s.split("\n").length >= 7, "Not enough usage lines: " + s); + assertTrue(s.startsWith("Usage: jshell <options>"), "Unexpect usage start: " + s); }, null, "-help"); } @Test public void testUnknown() throws Exception { start(s -> { - assertTrue(s.split("\n").length >= 7, s); - assertTrue(s.startsWith("Usage: jshell <options>"), s); + assertTrue(s.split("\n").length >= 7, "Not enough usage lines (unknown): " + s); + assertTrue(s.startsWith("Usage: jshell <options>"), "Unexpect usage start (unknown): " + s); }, s -> assertEquals(s.trim(), "Unknown option: -unknown"), "-unknown"); } @@ -157,12 +147,15 @@ @Test public void testVersion() throws Exception { - start(s -> assertTrue(s.startsWith("jshell")), null, "-version"); + start(s -> assertTrue(s.startsWith("jshell"), "unexpected version: " + s), null, "-version"); } @AfterMethod public void tearDown() { - out = null; - err = null; + cmdout = null; + cmderr = null; + console = null; + userout = null; + usererr = null; } }