The shutdown hook installed by jline has made l...

The shutdown hook installed by jline has made life difficult for sbt
for a while. This changes jline not to install it, and alters the scala
startup script to trap exit and re-enable echo on recognizably unix
platforms. In addition it no longer installs a shutdown hook to flush
the repl history to disk, instead flushing after every line.

Any bash reviewers out there? Unless someone raises a hand, no review.

5 files changed, 48 insertions, 25 deletions

diff --git a/lib/jline.jar.desired.sha1 b/lib/jline.jar.desired.sha1
index a7f48fb898..6857f5f24d 100644
--- a/lib/jline.jar.desired.sha1
+++ b/lib/jline.jar.desired.sha1
@@ -1 +1 @@
-a872abeca51b189640c8d1e9b70c053d52c71d67 ?jline.jar
+2a6d6dd4da89695f0c28ccc08cf388a4d84062b7 ?jline.jar
diff --git a/src/compiler/scala/tools/ant/templates/tool-unix.tmpl b/src/compiler/scala/tools/ant/templates/tool-unix.tmpl
index 4227e19a50..452324304e 100644
--- a/src/compiler/scala/tools/ant/templates/tool-unix.tmpl
+++ b/src/compiler/scala/tools/ant/templates/tool-unix.tmpl
@@ -8,6 +8,26 @@
 # PARTICULAR PURPOSE.
 ##############################################################################
 
+function onExit() {
+  if [[ -n "$scaladebug" ]]; then
+    echo "scala script runner trapped exit, running handler."
+  fi
+    
+  local exit_status=${1:-$?}
+
+  # reenable echo
+  case "$TERM" in
+      rxvt* | xterm*)
+          stty icanon echo
+          ;;
+  esac
+  
+  exit $exit_status
+}
+
+# install exit handler
+trap onExit EXIT
+
 cygwin=false;
 case "`uname`" in
     CYGWIN*) cygwin=true ;;
@@ -50,9 +70,6 @@ if [ -z "$TOOL_CLASSPATH" ] ; then
     done
 fi
 
-EXEC=exec
-CYGWIN_TERM_PRE=
-CYGWIN_TERM_POST=
 CYGWIN_JLINE_TERMINAL=
 if $cygwin; then
     if [ "$OS" = "Windows_NT" ] && cygpath -m .>/dev/null 2>/dev/null ; then
@@ -64,15 +81,10 @@ if $cygwin; then
     TOOL_CLASSPATH=`cygpath --path --$format "$TOOL_CLASSPATH"`
     case "$TERM" in
         rxvt* | xterm*)
-            EXEC=
-            CYGWIN_TERM_PRE="stty -icanon min 1 -echo"
-            CYGWIN_TERM_POST="stty icanon echo"
+            stty -icanon min 1 -echo
             CYGWIN_JLINE_TERMINAL="-Djline.terminal=jline.UnixTerminal"
-            trap "$CYGWIN_TERM_POST" SIGINT
         ;;
     esac
-else
-    trap "stty echo" SIGINT
 fi
 
 [ -n "$JAVA_OPTS" ] || JAVA_OPTS="@javaflags@"
@@ -117,9 +129,7 @@ if [ -z "$JAVACMD" -a -n "$JAVA_HOME" -a -x "$JAVA_HOME/bin/java" ]; then
     JAVACMD="$JAVA_HOME/bin/java"
 fi
 
-eval $CYGWIN_TERM_PRE
-
-$EXEC "${JAVACMD:=java}" \
+"${JAVACMD:=java}" \
   $JAVA_OPTS \
   "${java_args[@@]}" \
   $CPSWITCH \
@@ -128,5 +138,3 @@ $EXEC "${JAVACMD:=java}" \
   -Denv.emacs="$EMACS" \
   $CYGWIN_JLINE_TERMINAL \
   @properties@ @class@ @toolflags@ "$@@"
-  
-eval $CYGWIN_TERM_POST
diff --git a/src/compiler/scala/tools/nsc/interpreter/History.scala b/src/compiler/scala/tools/nsc/interpreter/History.scala
index 426c1f6c35..12ef857175 100644
--- a/src/compiler/scala/tools/nsc/interpreter/History.scala
+++ b/src/compiler/scala/tools/nsc/interpreter/History.scala
@@ -34,7 +34,14 @@ object History {
   val ScalaHistoryFile = ".scala_history"
 
   def apply(): History = new History(
-    try new FileHistory(new File(userHome, ScalaHistoryFile))
+    try new FileHistory(new File(userHome, ScalaHistoryFile)) {
+      // flush after every add to avoid installing a shutdown hook.
+      // (The shutdown hook approach also loses history when they aren't run.)
+      override def add(item: CharSequence): Unit = {
+        super.add(item)
+        flush()
+      }
+    }
     catch {
       case x: Exception =>
         Console.println("Error creating file history: memory history only. " + x)
diff --git a/src/compiler/scala/tools/nsc/interpreter/JLineReader.scala b/src/compiler/scala/tools/nsc/interpreter/JLineReader.scala
index 652321faf4..941b70101f 100644
--- a/src/compiler/scala/tools/nsc/interpreter/JLineReader.scala
+++ b/src/compiler/scala/tools/nsc/interpreter/JLineReader.scala
@@ -9,20 +9,12 @@ package interpreter
 import java.io.File
 import jline.console.ConsoleReader
 import jline.console.completer._
-import jline.console.history.{ History => JHistory }
 
 /** Reads from the console using JLine */
 class JLineReader(interpreter: Interpreter) extends InteractiveReader {
   def this() = this(null)
 
-  override lazy val history    = {
-    val h = History()
-    system addShutdownHook {
-      repldbg("Flushing history")
-      h.flush()
-    }
-    Some(h)
-  }
+  override lazy val history    = Some(History())
   override lazy val completion = Option(interpreter) map (x => new Completion(x))
   override def reset()         = consoleReader.getTerminal().reset()
   override def init()          = consoleReader.getTerminal().init()
diff --git a/src/jline/src/main/java/jline/TerminalSupport.java b/src/jline/src/main/java/jline/TerminalSupport.java
index 9448b0d481..ee7d2008ca 100644
--- a/src/jline/src/main/java/jline/TerminalSupport.java
+++ b/src/jline/src/main/java/jline/TerminalSupport.java
@@ -8,6 +8,7 @@
 package jline;
 
 import jline.internal.Log;
+import jline.internal.Configuration;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -24,12 +25,16 @@ public abstract class TerminalSupport
 {
     public static String DEFAULT_KEYBINDINGS_PROPERTIES = "keybindings.properties";
 
+    public static final String JLINE_SHUTDOWNHOOK = "jline.shutdownhook";
+
     public static final int DEFAULT_WIDTH = 80;
 
     public static final int DEFAULT_HEIGHT = 24;
 
     private Thread shutdownHook;
 
+    private boolean shutdownHookEnabled;
+
     private boolean supported;
 
     private boolean echoEnabled;
@@ -38,6 +43,7 @@ public abstract class TerminalSupport
 
     protected TerminalSupport(final boolean supported) {
         this.supported = supported;
+        this.shutdownHookEnabled = Configuration.getBoolean(JLINE_SHUTDOWNHOOK, false);
     }
 
     public void init() throws Exception {
@@ -54,7 +60,14 @@ public abstract class TerminalSupport
         init();
     }
 
+    // Shutdown hooks causes classloader leakage in sbt,
+    // so they are only installed if -Djline.shutdownhook is true.
     protected void installShutdownHook(final Thread hook) {
+        if (!shutdownHookEnabled) {
+            Log.debug("Not install shutdown hook " + hook + " because they are disabled.");
+            return;
+        }
+
         assert hook != null;
 
         if (shutdownHook != null) {
@@ -72,6 +85,9 @@ public abstract class TerminalSupport
     }
 
     protected void removeShutdownHook() {
+        if (!shutdownHookEnabled)
+            return;
+
         if (shutdownHook != null) {
             try {
                 Runtime.getRuntime().removeShutdownHook(shutdownHook);