changeset 3709:0a91e3198c97

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
author jlahoda
date Tue, 18 Oct 2016 16:00:32 +0200
parents 296c87505118
children 3fc90eaaf922
files src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java src/jdk.jshell/share/classes/jdk/internal/jshell/tool/IOContext.java src/jdk.jshell/share/classes/jdk/jshell/JShell.java src/jdk.jshell/share/classes/jdk/jshell/execution/PipeInputStream.java src/jdk.jshell/share/classes/jdk/jshell/execution/Util.java test/jdk/jshell/KullaTesting.java test/jdk/jshell/PipeInputStreamTest.java test/jdk/jshell/UserInputTest.java
diffstat 8 files changed, 251 insertions(+), 17 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java	Mon Oct 17 23:23:11 2016 +0900
+++ b/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/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/IOContext.java	Mon Oct 17 23:23:11 2016 +0900
+++ b/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/src/jdk.jshell/share/classes/jdk/jshell/JShell.java	Mon Oct 17 23:23:11 2016 +0900
+++ b/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/src/jdk.jshell/share/classes/jdk/jshell/execution/PipeInputStream.java	Mon Oct 17 23:23:11 2016 +0900
+++ b/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/src/jdk.jshell/share/classes/jdk/jshell/execution/Util.java	Mon Oct 17 23:23:11 2016 +0900
+++ b/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/test/jdk/jshell/KullaTesting.java	Mon Oct 17 23:23:11 2016 +0900
+++ b/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/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/test/jdk/jshell/UserInputTest.java	Mon Oct 17 23:23:11 2016 +0900
+++ b/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");
+    }
 }