8185426: Jshell crashing on autocompletion
authorjlahoda
Fri, 25 Aug 2017 13:48:49 +0200
changeset 47050 72ec64aeaa57
parent 47049 53333492bb1c
child 47051 463256e0de15
8185426: Jshell crashing on autocompletion Summary: Properly canceling completion on <tab> if needed. Reviewed-by: rfield
langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java
langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContextTestSupport.java
langtools/test/jdk/jshell/ToolTabSnippetTest.java
--- a/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java	Fri Aug 25 05:02:57 2017 +0000
+++ b/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java	Fri Aug 25 13:48:49 2017 +0200
@@ -55,7 +55,6 @@
 import jdk.internal.jline.WindowsTerminal;
 import jdk.internal.jline.console.ConsoleReader;
 import jdk.internal.jline.console.KeyMap;
-import jdk.internal.jline.console.Operation;
 import jdk.internal.jline.console.UserInterruptException;
 import jdk.internal.jline.console.history.History;
 import jdk.internal.jline.console.history.MemoryHistory;
@@ -94,14 +93,14 @@
             term = new JShellUnixTerminal(input);
         }
         term.init();
-        List<CompletionTask> completionTODO = new ArrayList<>();
+        CompletionState completionState = new CompletionState();
         in = new ConsoleReader(cmdin, cmdout, term) {
             @Override public KeyMap getKeys() {
-                return new CheckCompletionKeyMap(super.getKeys(), completionTODO);
+                return new CheckCompletionKeyMap(super.getKeys(), completionState);
             }
             @Override
             protected boolean complete() throws IOException {
-                return ConsoleIOContext.this.complete(completionTODO);
+                return ConsoleIOContext.this.complete(completionState);
             }
         };
         in.setExpandEvents(false);
@@ -201,15 +200,19 @@
     private static final String LINE_SEPARATORS2 = LINE_SEPARATOR + LINE_SEPARATOR;
 
     @SuppressWarnings("fallthrough")
-    private boolean complete(List<CompletionTask> todo) {
+    private boolean complete(CompletionState completionState) {
         //The completion has multiple states (invoked by subsequent presses of <tab>).
         //On the first invocation in a given sequence, all steps are precomputed
-        //and placed into the todo list. The todo list is then followed on both the first
-        //and subsequent <tab> presses:
+        //and placed into the todo list (completionState.todo). The todo list is
+        //then followed on both the first and subsequent completion invocations:
         try {
             String text = in.getCursorBuffer().toString();
             int cursor = in.getCursorBuffer().cursor;
-            if (todo.isEmpty()) {
+
+            List<CompletionTask> todo = completionState.todo;
+
+            if (todo.isEmpty() || completionState.actionCount != 1) {
+                ConsoleIOContextTestSupport.willComputeCompletion();
                 int[] anchor = new int[] {-1};
                 List<Suggestion> suggestions;
                 List<String> doc;
@@ -237,6 +240,8 @@
                 CompletionTask ordinaryCompletion = new OrdinaryCompletionTask(suggestions, anchor[0], !command && !doc.isEmpty(), hasSmart);
                 CompletionTask allCompletion = new AllSuggestionsCompletionTask(suggestions, anchor[0]);
 
+                todo = new ArrayList<>();
+
                 //the main decission tree:
                 if (command) {
                     CompletionTask shortDocumentation = new CommandSynopsisTask(doc);
@@ -310,6 +315,9 @@
                 }
             }
 
+            completionState.actionCount = 0;
+            completionState.todo = todo;
+
             if (repaint) {
                 in.redrawLine();
                 in.flush();
@@ -1203,12 +1211,12 @@
     private static final class CheckCompletionKeyMap extends KeyMap {
 
         private final KeyMap del;
-        private final List<CompletionTask> completionTODO;
+        private final CompletionState completionState;
 
-        public CheckCompletionKeyMap(KeyMap del, List<CompletionTask> completionTODO) {
+        public CheckCompletionKeyMap(KeyMap del, CompletionState completionState) {
             super(del.getName(), del.isViKeyMap());
             this.del = del;
-            this.completionTODO = completionTODO;
+            this.completionState = completionState;
         }
 
         @Override
@@ -1233,13 +1241,9 @@
 
         @Override
         public Object getBound(CharSequence keySeq) {
-            Object res = del.getBound(keySeq);
+            this.completionState.actionCount++;
 
-            if (res != Operation.COMPLETE) {
-                completionTODO.clear();
-            }
-
-            return res;
+            return del.getBound(keySeq);
         }
 
         @Override
@@ -1252,4 +1256,12 @@
             return "check: " + del.toString();
         }
     }
+
+    private static final class CompletionState {
+        /**The number of actions since the last completion invocation. Will be 1 when completion is
+         * invoked immediately after the last completion invocation.*/
+        public int actionCount;
+        /**Precomputed completion actions. Should only be reused if actionCount == 1.*/
+        public List<CompletionTask> todo = Collections.emptyList();
     }
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContextTestSupport.java	Fri Aug 25 13:48:49 2017 +0200
@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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.
+ */
+package jdk.internal.jshell.tool;
+
+/**A support class to the ConsoleIOContext, containing callbacks called
+ * on important points in ConsoleIOContext.
+ */
+public abstract class ConsoleIOContextTestSupport {
+
+    public static ConsoleIOContextTestSupport IMPL;
+
+    public static void willComputeCompletion() {
+        if (IMPL != null)
+            IMPL.willComputeCompletionCallback();
+    }
+
+    protected abstract void willComputeCompletionCallback();
+
+}
--- a/langtools/test/jdk/jshell/ToolTabSnippetTest.java	Fri Aug 25 05:02:57 2017 +0000
+++ b/langtools/test/jdk/jshell/ToolTabSnippetTest.java	Fri Aug 25 13:48:49 2017 +0200
@@ -23,10 +23,11 @@
 
 /**
  * @test
- * @bug 8177076
+ * @bug 8177076 8185426
  * @modules
  *     jdk.compiler/com.sun.tools.javac.api
  *     jdk.compiler/com.sun.tools.javac.main
+ *     jdk.jshell/jdk.internal.jshell.tool
  *     jdk.jshell/jdk.internal.jshell.tool.resources:open
  *     jdk.jshell/jdk.jshell:open
  * @library /tools/lib
@@ -42,10 +43,12 @@
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Arrays;
+import java.util.concurrent.CountDownLatch;
 import java.util.jar.JarEntry;
 import java.util.jar.JarOutputStream;
 import java.util.regex.Pattern;
 
+import jdk.internal.jshell.tool.ConsoleIOContextTestSupport;
 import org.testng.annotations.Test;
 
 @Test
@@ -191,6 +194,39 @@
         });
     }
 
+    public void testCleaningCompletionTODO() throws Exception {
+        doRunTest((inputSink, out) -> {
+            CountDownLatch testCompleteComputationStarted = new CountDownLatch(1);
+            CountDownLatch testCompleteComputationContinue = new CountDownLatch(1);
+            ConsoleIOContextTestSupport.IMPL = new ConsoleIOContextTestSupport() {
+                @Override
+                protected void willComputeCompletionCallback() {
+                    if (testCompleteComputationStarted != null) {
+                        testCompleteComputationStarted.countDown();
+                    }
+                    if (testCompleteComputationContinue != null) {
+                        try {
+                            testCompleteComputationContinue.await();
+                        } catch (InterruptedException ex) {
+                            throw new IllegalStateException(ex);
+                        }
+                    }
+                }
+            };
+            //-> <tab>
+            inputSink.write("\011");
+            testCompleteComputationStarted.await();
+            //-> <tab><tab>
+            inputSink.write("\011\011");
+            testCompleteComputationContinue.countDown();
+            waitOutput(out, "\u0005");
+            //-> <tab>
+            inputSink.write("\011");
+            waitOutput(out, "\u0005");
+            ConsoleIOContextTestSupport.IMPL = null;
+        });
+    }
+
     private Path prepareZip() {
         String clazz1 =
                 "package jshelltest;\n" +