[GR-26176] Migrate from JLine 2 to JLine 3

PullRequest: truffleruby/1996

Author: eregon

CHANGELOG.md

@@ -42,6 +42,10 @@ Performance:
 
 * Calls with a literal block are no longer always split but instead the decision is made by the Truffle splitting heuristic.
 
+Changes:
+
+* Migrated from JLine 2 to JLine 3 for the `readline` standard library.
+
 # 20.2.0
 
 New features:

common.json

@@ -2,15 +2,18 @@
   "README": "This file contains definitions that are useful for the hocon and jsonnet CI files of multiple repositories.",
 
   "jdks": {
-    "oraclejdk8":      {"name": "oraclejdk", "version": "8u261+33-jvmci-20.2-b03", "platformspecific": true },
-    "openjdk8":        {"name": "openjdk",   "version": "8u262+10-jvmci-20.2-b03", "platformspecific": true },
-    "oraclejdk8Debug": {"name": "oraclejdk", "version": "8u261+33-jvmci-20.2-b03-fastdebug", "platformspecific": true },
-    "oraclejdk11":     {"name": "oraclejdk", "version": "11.0.6+8", "platformspecific": true },
-    "oraclejdk15":     {"name": "oraclejdk", "version": "15.0.1+6", "platformspecific": true },
-    "oraclejdk15Debug":{"name": "oraclejdk", "version": "15.0.1+6-debug", "platformspecific": true },
-    "openjdk11":       {"name": "openjdk",   "version": "11.0.3+7", "platformspecific": true },
-    "labsjdk-ce-11":   {"name": "labsjdk",   "version": "ce-11.0.8+10-jvmci-20.2-b03", "platformspecific": true },
-    "labsjdk-ee-11":   {"name": "labsjdk",   "version": "ee-11.0.8.0.2+1-jvmci-20.2-b03", "platformspecific": true }
+    "oraclejdk8":         {"name": "oraclejdk", "version": "8u261+33-jvmci-20.3-b01", "platformspecific": true },
+    "openjdk8":           {"name": "openjdk",   "version": "8u262+10-jvmci-20.3-b01", "platformspecific": true },
+    "oraclejdk8Debug":    {"name": "oraclejdk", "version": "8u261+33-jvmci-20.3-b01-fastdebug", "platformspecific": true },
+    "oraclejdk11":        {"name": "oraclejdk", "version": "11.0.6+8", "platformspecific": true },
+    "oraclejdk15":        {"name": "oraclejdk", "version": "15.0.1+6", "platformspecific": true },
+    "openjdk11":          {"name": "openjdk",   "version": "11.0.3+7", "platformspecific": true },
+    "labsjdk-ce-11":      {"name": "labsjdk",   "version": "ce-11.0.8+10-jvmci-20.3-b01", "platformspecific": true },
+    "labsjdk-ee-11":      {"name": "labsjdk",   "version": "ee-11.0.8.0.2+1-jvmci-20.3-b01", "platformspecific": true },
+    "labsjdk-ce-15":      {"name": "labsjdk",   "version": "ce-15+36-jvmci-20.3-b01", "platformspecific": true },
+    "labsjdk-ee-15":      {"name": "labsjdk",   "version": "ee-15.0.1+7-jvmci-20.3-b01", "platformspecific": true },
+    "labsjdk-ce-15Debug": {"name": "labsjdk",   "version": "ce-15.+36-jvmci-20.3-b01", "platformspecific": true },
+    "labsjdk-ee-15Debug": {"name": "labsjdk",   "version": "ee-15.0.1+7-jvmci-20.3-b01", "platformspecific": true }
   },
 
   "sulong": {

lib/truffle/readline.rb

@@ -32,6 +32,7 @@ module Readline
     pre_input_hook
     pre_input_hook=
     redisplay
+    refresh_line
     set_screen_size
     special_prefixes
     special_prefixes=

mx.truffleruby/suite.py

@@ -7,7 +7,7 @@
             {
                 "name": "sulong",
                 "subdir": True,
-                "version": "df007aafea9720ec5a05757301a1b240b0b8cb93",
+                "version": "71072b3da8fc0930502a7fa0c915d1b1451a0838",
                 "urls": [
                     {"url": "https://github.com/oracle/graal.git", "kind": "git"},
                     {"url": "https://curio.ssw.jku.at/nexus/content/repositories/snapshots", "kind": "binary"},
@@ -166,7 +166,7 @@
                 "truffleruby:TRUFFLERUBY-ANNOTATIONS",
                 "truffleruby:TRUFFLERUBY-SHARED",
                 "truffle:TRUFFLE_API",
-                "truffle:JLINE",
+                "sdk:JLINE3",
                 "sulong:SULONG_API",
                 "JONI",
                 "JCODINGS",

spec/truffle/irb/arrows_spec.rb

@@ -14,9 +14,22 @@
     IO.popen([*ruby_exe, "-S", "irb", "-f", "--prompt=simple", "--readline"], "r+") do |io|
       io.gets.should == "Switch to inspect mode.\n"
 
-      io.puts "\e[A" # up arrow
-      # JLine seems to add a bell character
-      [">> \n", ">> \a\n"].should include(io.gets)
+      if RUBY_ENGINE == "truffleruby"
+        io.puts "B = 42"
+        io.gets.should == ">> B = 42\n"
+        io.gets.should == "=> 42\n"
+      end
+
+      io.puts "\e[B" # down arrow
+      # JLine adds a bell character (since there is no history)
+      echo = io.gets
+      [">> \n", ">> [B\n"].should include(echo)
+
+      if echo == ">> [B\n" # "[B" (echo'd by JLine) looks like a legal expression to IRB so we need to complete it
+        io.puts "]"
+        io.gets.should == ">> ]\n"
+        io.gets.should == "=> [42]\n"
+      end
 
       io.puts "22 + 33"
       # The extra bell character causes a continuation line

spec/truffle/irb/interrupt_spec.rb

@@ -23,7 +23,7 @@
       io.gets.should == ">> ^C\n"
 
       io.puts "exit"
-      io.gets.should == ">> exit\n"
+      io.gets.should == "\n"
     end
   end
 end

src/main/.checkstyle_checks.xml

@@ -202,10 +202,6 @@
       <property name="format" value='(?!=)\S\s*new\s+\w+\[0\]'/>
       <property name="message" value='Use a EMPTY_type_ARRAY constant instead.'/>
     </module>
-    <module name="RegexpSinglelineJava">
-      <property name="format" value='\bPrintWriter\b'/>
-      <property name="message" value='Use PrintStream instead, PrintWriter does not consistently flush, even when writing \n.'/>
-    </module>
     <module name="RegexpSinglelineJava">
       <property name="format" value="transferToInterpreterOnException"/>
       <property name="message" value="Don't use transferToInterpreterOnException"/>
@@ -219,7 +215,8 @@
       <property name="message" value="Type the argument as Nil instead of using a positive isNil() guard."/>
     </module>
     <module name="IllegalType">
-      <property name="illegalClassNames" value="TruffleObject,DynamicObject"/>
+      <!-- Use PrintStream instead of PrintWriter, PrintWriter does not consistently flush, even when writing \n.-->
+      <property name="illegalClassNames" value="TruffleObject,DynamicObject,PrintWriter"/>
     </module>
   </module>
   <module name="RegexpMultiline">

src/main/java/org/truffleruby/RubyContext.java

@@ -13,6 +13,7 @@
 import java.nio.ByteBuffer;
 import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
+import java.util.Objects;
 import java.util.concurrent.locks.ReentrantLock;
 import java.util.logging.Level;
 
@@ -466,6 +467,10 @@ private void dispose() {
             return;
         }
 
+        if (consoleHolder != null) {
+            consoleHolder.close();
+        }
+
         threadManager.cleanupMainThread();
         safepointManager.checkNoRunningThreads();
 
@@ -684,6 +689,8 @@ public ConsoleHolder getConsoleHolder() {
 
     public void setConsoleHolder(ConsoleHolder consoleHolder) {
         synchronized (this) {
+            final ConsoleHolder previous = Objects.requireNonNull(this.consoleHolder);
+            previous.close();
             this.consoleHolder = consoleHolder;
         }
     }

src/main/java/org/truffleruby/core/VMPrimitiveNodes.java

@@ -90,6 +90,7 @@
 import com.oracle.truffle.api.profiles.ConditionProfile;
 
 import sun.misc.Signal;
+import sun.misc.SignalHandler;
 
 @CoreModule(value = "VMPrimitives", isClass = true)
 public abstract class VMPrimitiveNodes {
@@ -229,34 +230,34 @@ public static abstract class VMWatchSignalNode extends PrimitiveArrayArgumentsNo
 
         @TruffleBoundary
         @Specialization
-        protected boolean restoreDefault(RubyString signalName, RubyString action) {
+        protected boolean restoreDefault(RubyString signalString, RubyString action) {
             final String actionString = StringOperations.getString(action);
-            final String signal = StringOperations.getString(signalName);
+            final String signalName = StringOperations.getString(signalString);
 
             switch (actionString) {
                 case "DEFAULT":
-                    return restoreDefaultHandler(signal);
+                    return restoreDefaultHandler(signalName);
                 case "SYSTEM_DEFAULT":
-                    return restoreSystemHandler(signal);
+                    return restoreSystemHandler(signalName);
                 case "IGNORE":
-                    return registerIgnoreHandler(signal);
+                    return registerIgnoreHandler(signalName);
                 default:
                     throw new UnsupportedOperationException(actionString);
             }
         }
 
         @TruffleBoundary
         @Specialization
-        protected boolean watchSignalProc(RubyString signalName, RubyProc action) {
+        protected boolean watchSignalProc(RubyString signalString, RubyProc action) {
             if (getContext().getThreadManager().getCurrentThread() != getContext().getThreadManager().getRootThread()) {
                 // The proc will be executed on the main thread
                 SharedObjects.writeBarrier(getContext(), action);
             }
 
             final RubyContext context = getContext();
 
-            final String signal = StringOperations.getString(signalName);
-            return registerHandler(signal, () -> {
+            String signalName = StringOperations.getString(signalString);
+            return registerHandler(signalName, signal -> {
                 if (context.getOptions().SINGLE_THREADED) {
                     RubyLanguage.LOGGER.severe(
                             "signal " + signal + " caught but can't create a thread to handle it so ignoring");
@@ -279,9 +280,9 @@ protected boolean watchSignalProc(RubyString signalName, RubyProc action) {
                     printStream.println(
                             "[ruby] SEVERE: signal " + signal +
                                     " caught but can't attach a thread to handle it so restoring the default handler and re-raising the signal");
-                    Signals.restoreDefaultHandler(signal);
+                    Signals.restoreDefaultHandler(signalName);
                     try {
-                        Signal.raise(new Signal(signal));
+                        Signal.raise(signal);
                     } catch (IllegalArgumentException illegalArgumentException) {
                         illegalArgumentException.printStackTrace(printStream);
                     }
@@ -351,7 +352,7 @@ private boolean registerIgnoreHandler(String signalName) {
         }
 
         @TruffleBoundary
-        private boolean registerHandler(String signalName, Runnable newHandler) {
+        private boolean registerHandler(String signalName, SignalHandler newHandler) {
             if (getContext().getOptions().EMBEDDED) {
                 RubyLanguage.LOGGER.warning(
                         "trapping signal " + signalName +

src/main/java/org/truffleruby/core/adapters/InputStreamAdapter.java

@@ -9,17 +9,20 @@
  */
 package org.truffleruby.core.adapters;
 
-import java.io.InputStream;
-
+import org.graalvm.shadowed.org.jline.utils.NonBlockingInputStream;
 import org.truffleruby.RubyContext;
 import org.truffleruby.core.support.RubyIO;
 import org.truffleruby.language.Nil;
 
-public class InputStreamAdapter extends InputStream {
+/** extends NonBlockingInputStream so JLine does not create an extra thread and calls read() on the same thread as
+ * calling readLine(). */
+public class InputStreamAdapter extends NonBlockingInputStream {
 
     private final RubyContext context;
     private final RubyIO object;
 
+    private int peeked = EOF;
+
     public InputStreamAdapter(RubyContext context, RubyIO object) {
         this.context = context;
         this.object = object;
@@ -30,9 +33,24 @@ public int read() {
         final Object result = context.send(object, "getbyte");
 
         if (result == Nil.INSTANCE) {
-            return -1;
+            return EOF;
         }
 
         return (int) result;
     }
+
+    @Override
+    public int read(long timeout, boolean isPeek) {
+        if (peeked != EOF) {
+            int c = peeked;
+            this.peeked = EOF;
+            return c;
+        }
+
+        int character = read();
+        if (isPeek) {
+            peeked = character;
+        }
+        return character;
+    }
 }