8167461: jshell tool: Scanner#next() hangs tool
Summary: PipeInputStream.read(byte[]...) should only read available bytes; properly resending exceptions for snippet's System.in and properly closing it; more reliable way to cancel user input while waiting in System.in.
Reviewed-by: rfield
--- a/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java Wed Jul 05 22:21:06 2017 +0200
+++ b/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java Tue Oct 18 16:00:32 2016 +0200
@@ -31,6 +31,7 @@
import java.awt.event.ActionListener;
import java.io.IOException;
import java.io.InputStream;
+import java.io.InterruptedIOException;
import java.io.PrintStream;
import java.io.UncheckedIOException;
import java.lang.reflect.Method;
@@ -390,7 +391,7 @@
private int inputBytesPointer;
@Override
- public synchronized int readUserInput() {
+ public synchronized int readUserInput() throws IOException {
while (inputBytes == null || inputBytes.length <= inputBytesPointer) {
boolean prevHandleUserInterrupt = in.getHandleUserInterrupt();
History prevHistory = in.getHistory();
@@ -401,12 +402,8 @@
in.setHistory(userInputHistory);
inputBytes = (in.readLine("") + System.getProperty("line.separator")).getBytes();
inputBytesPointer = 0;
- } catch (IOException ex) {
- ex.printStackTrace();
- return -1;
} catch (UserInterruptException ex) {
- repl.state.stop();
- return -1;
+ throw new InterruptedIOException();
} finally {
in.setHistory(prevHistory);
in.setHandleUserInterrupt(prevHandleUserInterrupt);
--- a/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/IOContext.java Wed Jul 05 22:21:06 2017 +0200
+++ b/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/IOContext.java Tue Oct 18 16:00:32 2016 +0200
@@ -54,7 +54,7 @@
public abstract void replaceLastHistoryEntry(String source);
- public abstract int readUserInput();
+ public abstract int readUserInput() throws IOException;
class InputInterruptedException extends Exception {
private static final long serialVersionUID = 1L;
--- a/langtools/src/jdk.jshell/share/classes/jdk/jshell/JShell.java Wed Jul 05 22:21:06 2017 +0200
+++ b/langtools/src/jdk.jshell/share/classes/jdk/jshell/JShell.java Tue Oct 18 16:00:32 2016 +0200
@@ -28,6 +28,7 @@
import jdk.jshell.spi.ExecutionControl;
import java.io.ByteArrayInputStream;
import java.io.InputStream;
+import java.io.InterruptedIOException;
import java.io.PrintStream;
import java.text.MessageFormat;
import java.util.ArrayList;
@@ -167,6 +168,10 @@
* user input cannot use {@code System.in} as the input stream for
* the remote process.
* <p>
+ * The {@code read} method of the {@code InputStream} may throw the {@link InterruptedIOException}
+ * to signal the user canceled the input. The currently running snippet will be automatically
+ * {@link JShell#stop() stopped}.
+ * <p>
* The default, if this is not set, is to provide an empty input stream
* -- {@code new ByteArrayInputStream(new byte[0])}.
*
--- a/langtools/src/jdk.jshell/share/classes/jdk/jshell/execution/PipeInputStream.java Wed Jul 05 22:21:06 2017 +0200
+++ b/langtools/src/jdk.jshell/share/classes/jdk/jshell/execution/PipeInputStream.java Tue Oct 18 16:00:32 2016 +0200
@@ -42,7 +42,7 @@
@Override
public synchronized int read() throws IOException {
- if (start == end) {
+ if (start == end && !closed) {
inputNeeded();
}
while (start == end) {
@@ -62,6 +62,32 @@
}
}
+ @Override
+ public synchronized int read(byte[] b, int off, int len) throws IOException {
+ if (b == null) {
+ throw new NullPointerException();
+ } else if (off < 0 || len < 0 || len > b.length - off) {
+ throw new IndexOutOfBoundsException();
+ } else if (len == 0) {
+ return 0;
+ }
+
+ int c = read();
+ if (c == -1) {
+ return -1;
+ }
+ b[off] = (byte)c;
+
+ int totalRead = 1;
+ while (totalRead < len && start != end) {
+ int r = read();
+ if (r == (-1))
+ break;
+ b[off + totalRead++] = (byte) r;
+ }
+ return totalRead;
+ }
+
protected void inputNeeded() throws IOException {}
private synchronized void write(int b) {
--- a/langtools/src/jdk.jshell/share/classes/jdk/jshell/execution/Util.java Wed Jul 05 22:21:06 2017 +0200
+++ b/langtools/src/jdk.jshell/share/classes/jdk/jshell/execution/Util.java Tue Oct 18 16:00:32 2016 +0200
@@ -28,6 +28,7 @@
import java.io.IOException;
import java.io.InputStream;
+import java.io.InterruptedIOException;
import java.io.ObjectInput;
import java.io.ObjectInputStream;
import java.io.ObjectOutput;
@@ -42,6 +43,7 @@
import com.sun.jdi.VirtualMachine;
import jdk.jshell.spi.ExecutionControl;
+import jdk.jshell.spi.ExecutionControl.ExecutionControlException;
/**
@@ -54,6 +56,10 @@
*/
public class Util {
+ private static final int TAG_DATA = 0;
+ private static final int TAG_CLOSED = 1;
+ private static final int TAG_EXCEPTION = 2;
+
// never instanciated
private Util() {}
@@ -131,6 +137,25 @@
inputSignal.write('1');
inputSignal.flush();
}
+ @Override
+ public synchronized int read() throws IOException {
+ int tag = super.read();
+ switch (tag) {
+ case TAG_DATA: return super.read();
+ case TAG_CLOSED: close(); return -1;
+ case TAG_EXCEPTION:
+ int len = (super.read() << 0) + (super.read() << 8) + (super.read() << 16) + (super.read() << 24);
+ byte[] message = new byte[len];
+ for (int i = 0; i < len; i++) {
+ message[i] = (byte) super.read();
+ }
+ throw new IOException(new String(message, "UTF-8"));
+ case -1:
+ return -1;
+ default:
+ throw new IOException("Internal error: unrecognized message tag: " + tag);
+ }
+ }
};
inputs.put(e.getKey(), inputPipe.createOutput());
e.getValue().accept(inputPipe);
@@ -163,6 +188,7 @@
public static ExecutionControl remoteInputOutput(InputStream input, OutputStream output,
Map<String, OutputStream> outputStreamMap, Map<String, InputStream> inputStreamMap,
BiFunction<ObjectInput, ObjectOutput, ExecutionControl> factory) throws IOException {
+ ExecutionControl[] result = new ExecutionControl[1];
Map<String, OutputStream> augmentedStreamMap = new HashMap<>(outputStreamMap);
ObjectOutput commandOut = new ObjectOutputStream(Util.multiplexingOutputStream("$command", output));
for (Entry<String, InputStream> e : inputStreamMap.entrySet()) {
@@ -172,7 +198,28 @@
@Override
public void write(int b) throws IOException {
//value ignored, just a trigger to read from the input
- inTarget.write(in.read());
+ try {
+ int r = in.read();
+ if (r == (-1)) {
+ inTarget.write(TAG_CLOSED);
+ } else {
+ inTarget.write(new byte[] {TAG_DATA, (byte) r});
+ }
+ } catch (InterruptedIOException exc) {
+ try {
+ result[0].stop();
+ } catch (ExecutionControlException ex) {
+ debug(ex, "$" + e.getKey() + "-input-requested.write");
+ }
+ } catch (IOException exc) {
+ byte[] message = exc.getMessage().getBytes("UTF-8");
+ inTarget.write(TAG_EXCEPTION);
+ inTarget.write((message.length >> 0) & 0xFF);
+ inTarget.write((message.length >> 8) & 0xFF);
+ inTarget.write((message.length >> 16) & 0xFF);
+ inTarget.write((message.length >> 24) & 0xFF);
+ inTarget.write(message);
+ }
}
});
}
@@ -180,7 +227,7 @@
OutputStream commandInTarget = commandIn.createOutput();
augmentedStreamMap.put("$command", commandInTarget);
new DemultiplexInput(input, augmentedStreamMap, Arrays.asList(commandInTarget)).start();
- return factory.apply(new ObjectInputStream(commandIn), commandOut);
+ return result[0] = factory.apply(new ObjectInputStream(commandIn), commandOut);
}
/**
@@ -198,4 +245,13 @@
}
}
+ /**
+ * Log a serious unexpected internal exception.
+ *
+ * @param ex the exception
+ * @param where a description of the context of the exception
+ */
+ private static void debug(Throwable ex, String where) {
+ // Reserved for future logging
+ }
}
--- a/langtools/test/jdk/jshell/KullaTesting.java Wed Jul 05 22:21:06 2017 +0200
+++ b/langtools/test/jdk/jshell/KullaTesting.java Tue Oct 18 16:00:32 2016 +0200
@@ -21,7 +21,10 @@
* questions.
*/
+import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
import java.io.PrintStream;
import java.io.StringWriter;
import java.lang.reflect.Method;
@@ -83,7 +86,7 @@
private SourceCodeAnalysis analysis = null;
private JShell state = null;
- private TestingInputStream inStream = null;
+ private InputStream inStream = null;
private ByteArrayOutputStream outStream = null;
private ByteArrayOutputStream errStream = null;
@@ -106,7 +109,11 @@
}
public void setInput(String s) {
- inStream.setInput(s);
+ setInput(new ByteArrayInputStream(s.getBytes()));
+ }
+
+ public void setInput(InputStream in) {
+ inStream = in;
}
public String getOutput() {
@@ -159,11 +166,27 @@
}
public void setUp(Consumer<JShell.Builder> bc) {
- inStream = new TestingInputStream();
+ InputStream in = new InputStream() {
+ @Override
+ public int read() throws IOException {
+ assertNotNull(inStream);
+ return inStream.read();
+ }
+ @Override
+ public int read(byte[] b) throws IOException {
+ assertNotNull(inStream);
+ return inStream.read(b);
+ }
+ @Override
+ public int read(byte[] b, int off, int len) throws IOException {
+ assertNotNull(inStream);
+ return inStream.read(b, off, len);
+ }
+ };
outStream = new ByteArrayOutputStream();
errStream = new ByteArrayOutputStream();
JShell.Builder builder = JShell.builder()
- .in(inStream)
+ .in(in)
.out(new PrintStream(outStream))
.err(new PrintStream(errStream));
bc.accept(builder);
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/jdk/jshell/PipeInputStreamTest.java Tue Oct 18 16:00:32 2016 +0200
@@ -0,0 +1,71 @@
+/*
+ * Copyright (c) 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.
+ */
+
+/*
+ * @test 8167461
+ * @summary Verify PipeInputStream works.
+ * @modules jdk.compiler/com.sun.tools.javac.util
+ * jdk.jshell
+ * @run testng PipeInputStreamTest
+ */
+
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Method;
+
+import org.testng.annotations.Test;
+
+import com.sun.tools.javac.util.Pair;
+
+import static org.testng.Assert.*;
+
+@Test
+public class PipeInputStreamTest {
+
+ public void testReadArrayNotBlocking() throws Exception {
+ Pair<InputStream, OutputStream> streams = createPipeStream();
+ InputStream in = streams.fst;
+ OutputStream out = streams.snd;
+ out.write('a');
+ byte[] data = new byte[12];
+ assertEquals(in.read(data), 1);
+ assertEquals(data[0], 'a');
+ out.write('a'); out.write('b'); out.write('c');
+ assertEquals(in.read(data), 3);
+ assertEquals(data[0], 'a');
+ assertEquals(data[1], 'b');
+ assertEquals(data[2], 'c');
+ }
+
+ private Pair<InputStream, OutputStream> createPipeStream() throws Exception {
+ Class<?> pipeStreamClass = Class.forName("jdk.jshell.execution.PipeInputStream");
+ Constructor<?> c = pipeStreamClass.getDeclaredConstructor();
+ c.setAccessible(true);
+ Object pipeStream = c.newInstance();
+ Method createOutputStream = pipeStreamClass.getDeclaredMethod("createOutput");
+ createOutputStream.setAccessible(true);
+ return Pair.of((InputStream) pipeStream, (OutputStream) createOutputStream.invoke(pipeStream));
+ }
+
+}
--- a/langtools/test/jdk/jshell/UserInputTest.java Wed Jul 05 22:21:06 2017 +0200
+++ b/langtools/test/jdk/jshell/UserInputTest.java Tue Oct 18 16:00:32 2016 +0200
@@ -23,12 +23,15 @@
/*
* @test
- * @bug 8131023
+ * @bug 8131023 8167461
* @summary Verify that the user's code can read System.in
* @build KullaTesting TestingInputStream
* @run testng UserInputTest
*/
+import java.io.IOException;
+import java.io.InputStream;
+
import org.testng.annotations.Test;
@Test
@@ -37,8 +40,61 @@
public void testReadInput() {
setInput("AB\n");
assertEval("System.in.read()", "65");
- setInput("BC\n");
- assertEval("System.in.read()", "66");
+ setInput("CD\n");
+ assertEval("System.in.read()", "67");
+ }
+
+ public void testScanner() {
+ assertEval("import java.util.Scanner;");
+ assertEval("Scanner s = new Scanner(System.in);");
+ setInput("12\n");
+ assertEval("s.nextInt();", "12");
}
+ public void testClose() {
+ setInput(new InputStream() {
+ private final byte[] data = new byte[] {0, 1, 2};
+ private int cursor;
+ @Override public int read() throws IOException {
+ if (cursor < data.length) {
+ return data[cursor++];
+ } else {
+ return -1;
+ }
+ }
+ });
+ assertEval("int read;", "0");
+ assertEval("System.in.read();", "0");
+ assertEval("System.in.read();", "1");
+ assertEval("System.in.read();", "2");
+ assertEval("System.in.read();", "-1");
+ assertEval("System.in.read();", "-1");
+ assertEval("System.in.read();", "-1");
+ }
+
+ public void testException() {
+ setInput(new InputStream() {
+ private final int[] data = new int[] {0, 1, -2, 2};
+ private int cursor;
+ @Override public int read() throws IOException {
+ if (cursor < data.length) {
+ int d = data[cursor++];
+ if (d == (-2)) {
+ throw new IOException("Crashed");
+ }
+ return d;
+ } else {
+ return -1;
+ }
+ }
+ });
+ assertEval("int read;", "0");
+ assertEval("System.in.read();", "0");
+ assertEval("System.in.read();", "1");
+ assertEval("java.io.IOException e;");
+ assertEval("try { System.in.read(); } catch (java.io.IOException exc) { e = exc; }");
+ assertEval("e", "java.io.IOException: Crashed");
+ assertEval("System.in.read();", "2");
+ assertEval("System.in.read();", "-1");
+ }
}