From 8f7b607b14d624e8445b58a0f246f872408765c3 Mon Sep 17 00:00:00 2001 From: Allan Renucci Date: Fri, 19 Oct 2018 19:56:39 +0200 Subject: [PATCH 1/2] Remove Worksheet dependency on JLine The worksheet used to spawn a REPL with JLine just to be able to read from an InputStream. This is alternative solution that simply uses java.util.Scanner to read from an InputStream. For that, we need to agree on a delimiter. We choose a character within the private user area to not conflict with the output produced by the REPL. --- .../src/dotty/tools/repl/JLineTerminal.scala | 4 +- compiler/src/dotty/tools/repl/Main.scala | 7 +-- .../src/dotty/tools/repl/ReplDriver.scala | 4 +- .../languageserver/worksheet/Evaluator.scala | 20 ++++---- .../worksheet/InputStreamConsumer.scala | 25 ++++++++++ .../worksheet/ReplProcess.scala | 17 +++++++ .../languageserver/worksheet/ReplReader.scala | 47 ------------------- .../languageserver/worksheet/Worksheet.scala | 2 +- sbt-bridge/src/xsbt/ConsoleInterface.scala | 2 +- 9 files changed, 59 insertions(+), 69 deletions(-) create mode 100644 language-server/src/dotty/tools/languageserver/worksheet/InputStreamConsumer.scala create mode 100644 language-server/src/dotty/tools/languageserver/worksheet/ReplProcess.scala delete mode 100644 language-server/src/dotty/tools/languageserver/worksheet/ReplReader.scala diff --git a/compiler/src/dotty/tools/repl/JLineTerminal.scala b/compiler/src/dotty/tools/repl/JLineTerminal.scala index 2e6b0e216f89..c45acf88ff1b 100644 --- a/compiler/src/dotty/tools/repl/JLineTerminal.scala +++ b/compiler/src/dotty/tools/repl/JLineTerminal.scala @@ -13,13 +13,13 @@ import org.jline.reader.impl.history.DefaultHistory import org.jline.terminal.TerminalBuilder import org.jline.utils.AttributedString -final class JLineTerminal(needsTerminal: Boolean) extends java.io.Closeable { +final class JLineTerminal extends java.io.Closeable { // import java.util.logging.{Logger, Level} // Logger.getLogger("org.jline").setLevel(Level.FINEST) private val terminal = TerminalBuilder.builder() - .dumb(!needsTerminal) // fail early if we need a terminal and can't create one + .dumb(false) // fail early if not able to create a terminal .build() private val history = new DefaultHistory diff --git a/compiler/src/dotty/tools/repl/Main.scala b/compiler/src/dotty/tools/repl/Main.scala index adf8d9a6de91..725395dcdb3c 100644 --- a/compiler/src/dotty/tools/repl/Main.scala +++ b/compiler/src/dotty/tools/repl/Main.scala @@ -3,10 +3,5 @@ package dotty.tools.repl /** Main entry point to the REPL */ object Main { def main(args: Array[String]): Unit = - new ReplDriver(args).runUntilQuit(needsTerminal = true) -} - -object WorksheetMain { - def main(args: Array[String]): Unit = - new ReplDriver(args).runUntilQuit(needsTerminal = false) + new ReplDriver(args).runUntilQuit() } diff --git a/compiler/src/dotty/tools/repl/ReplDriver.scala b/compiler/src/dotty/tools/repl/ReplDriver.scala index 4717d38deef5..fa1af31c157f 100644 --- a/compiler/src/dotty/tools/repl/ReplDriver.scala +++ b/compiler/src/dotty/tools/repl/ReplDriver.scala @@ -101,8 +101,8 @@ class ReplDriver(settings: Array[String], * observable outside of the CLI, for this reason, most helper methods are * `protected final` to facilitate testing. */ - final def runUntilQuit(needsTerminal: Boolean, initialState: State = initialState): State = { - val terminal = new JLineTerminal(needsTerminal) + final def runUntilQuit(initialState: State = initialState): State = { + val terminal = new JLineTerminal /** Blockingly read a line, getting back a parse result */ def readLine(state: State): ParseResult = { diff --git a/language-server/src/dotty/tools/languageserver/worksheet/Evaluator.scala b/language-server/src/dotty/tools/languageserver/worksheet/Evaluator.scala index afa303d4b8a7..caf5007e2f01 100644 --- a/language-server/src/dotty/tools/languageserver/worksheet/Evaluator.scala +++ b/language-server/src/dotty/tools/languageserver/worksheet/Evaluator.scala @@ -2,7 +2,8 @@ package dotty.tools.languageserver.worksheet import dotty.tools.dotc.core.Contexts.Context -import java.io.{File, PrintStream} +import java.io.{File, PrintStream, IOException} +import java.lang.ProcessBuilder.Redirect import org.eclipse.lsp4j.jsonrpc.CancelChecker @@ -57,7 +58,7 @@ private class Evaluator private (javaExec: String, new ProcessBuilder( javaExec, "-classpath", scala.util.Properties.javaClassPath, - dotty.tools.repl.WorksheetMain.getClass.getName.stripSuffix("$"), + ReplProcess.getClass.getName.stripSuffix("$"), "-classpath", userClasspath, "-color:never") .redirectErrorStream(true) @@ -67,15 +68,12 @@ private class Evaluator private (javaExec: String, private val processInput = new PrintStream(process.getOutputStream()) // Messages coming out of the REPL - private val processOutput = new ReplReader(process.getInputStream()) - processOutput.start() + private val processOutput = new InputStreamConsumer(process.getInputStream()) // The thread that monitors cancellation private val cancellationThread = new CancellationThread(cancelChecker, this) cancellationThread.start() - // Wait for the REPL to be ready - processOutput.next() /** Is the process that runs the REPL still alive? */ def isAlive(): Boolean = process.isAlive() @@ -86,10 +84,13 @@ private class Evaluator private (javaExec: String, * @param command The command to evaluate. * @return The result from the REPL. */ - def eval(command: String): Option[String] = { - processInput.println(command) + def eval(command: String): String = { + processInput.print(command) + processInput.print(InputStreamConsumer.delimiter) processInput.flush() - processOutput.next().map(_.trim) + + try processOutput.next().trim + catch { case _: IOException => "" } } /** @@ -102,7 +103,6 @@ private class Evaluator private (javaExec: String, /** Terminate this JVM. */ def exit(): Unit = { - processOutput.interrupt() process.destroyForcibly() Evaluator.previousEvaluator = None cancellationThread.interrupt() diff --git a/language-server/src/dotty/tools/languageserver/worksheet/InputStreamConsumer.scala b/language-server/src/dotty/tools/languageserver/worksheet/InputStreamConsumer.scala new file mode 100644 index 000000000000..871bb47eb09c --- /dev/null +++ b/language-server/src/dotty/tools/languageserver/worksheet/InputStreamConsumer.scala @@ -0,0 +1,25 @@ +package dotty.tools.languageserver.worksheet + +import java.io.{InputStream, IOException} +import java.util.Scanner + +class InputStreamConsumer(in: InputStream) { + private[this] val scanner = + new Scanner(in).useDelimiter(InputStreamConsumer.delimiter) + + /** Finds and returns the next complete token from this input stream. + * + * A complete token is preceded and followed by input that matches the delimiter pattern. + * This method may block while waiting for input + */ + def next(): String = + try scanner.next() + catch { + case _: NoSuchElementException => + throw new IOException("InputStream closed") + } +} + +object InputStreamConsumer { + def delimiter = "\uE000" // withing private use area +} diff --git a/language-server/src/dotty/tools/languageserver/worksheet/ReplProcess.scala b/language-server/src/dotty/tools/languageserver/worksheet/ReplProcess.scala new file mode 100644 index 000000000000..57f2bde33659 --- /dev/null +++ b/language-server/src/dotty/tools/languageserver/worksheet/ReplProcess.scala @@ -0,0 +1,17 @@ +package dotty.tools.languageserver.worksheet + +import dotty.tools.repl.ReplDriver + +object ReplProcess { + def main(args: Array[String]): Unit = { + val driver = new ReplDriver(args) + val in = new InputStreamConsumer(System.in) + var state = driver.initialState + + while (true) { + val code = in.next() // blocking + state = driver.run(code)(state) + print(InputStreamConsumer.delimiter) // needed to mark the end of REPL output + } + } +} diff --git a/language-server/src/dotty/tools/languageserver/worksheet/ReplReader.scala b/language-server/src/dotty/tools/languageserver/worksheet/ReplReader.scala deleted file mode 100644 index bd50f9c4faf8..000000000000 --- a/language-server/src/dotty/tools/languageserver/worksheet/ReplReader.scala +++ /dev/null @@ -1,47 +0,0 @@ -package dotty.tools.languageserver.worksheet - -import java.io.{InputStream, InputStreamReader} - -/** - * Reads the output from the REPL and makes it available via `next()`. - * - * @param stream The stream of messages coming out of the REPL. - */ -private class ReplReader(stream: InputStream) extends Thread { - private val in = new InputStreamReader(stream) - - private[this] var output: Option[String] = None - private[this] var closed: Boolean = false - - override def run(): Unit = synchronized { - val prompt = "scala> " - val buffer = new StringBuilder - val chars = new Array[Char](256) - var read = 0 - - while (!Thread.interrupted() && { read = in.read(chars); read >= 0 }) { - buffer.appendAll(chars, 0, read) - if (buffer.endsWith(prompt)) { - output = Some(buffer.toString.stripSuffix(prompt)) - buffer.clear() - notify() - wait() - } - } - closed = true - notify() - } - - /** Block until the next message is ready. */ - def next(): Option[String] = synchronized { - - while (!closed && output.isEmpty) { - wait() - } - - val result = output - notify() - output = None - result - } -} diff --git a/language-server/src/dotty/tools/languageserver/worksheet/Worksheet.scala b/language-server/src/dotty/tools/languageserver/worksheet/Worksheet.scala index 60e10688a924..88cd82368044 100644 --- a/language-server/src/dotty/tools/languageserver/worksheet/Worksheet.scala +++ b/language-server/src/dotty/tools/languageserver/worksheet/Worksheet.scala @@ -49,7 +49,7 @@ object Worksheet { } queries.foreach { (line, code) => cancelChecker.checkCanceled() - val res = evaluator.eval(code).getOrElse("") + val res = evaluator.eval(code) cancelChecker.checkCanceled() if (res.nonEmpty) sendMessage(line, res) diff --git a/sbt-bridge/src/xsbt/ConsoleInterface.scala b/sbt-bridge/src/xsbt/ConsoleInterface.scala index ba860e84d0ab..34004528fea4 100644 --- a/sbt-bridge/src/xsbt/ConsoleInterface.scala +++ b/sbt-bridge/src/xsbt/ConsoleInterface.scala @@ -41,7 +41,7 @@ class ConsoleInterface { val s1 = driver.run(initialCommands)(s0) // TODO handle failure during initialisation - val s2 = driver.runUntilQuit(needsTerminal = true, s1) + val s2 = driver.runUntilQuit(s1) driver.run(cleanupCommands)(s2) } } From 3865ba6f43243b64df312db60522df7d8bd5ad5e Mon Sep 17 00:00:00 2001 From: Allan Renucci Date: Wed, 24 Oct 2018 11:34:21 +0200 Subject: [PATCH 2/2] Replace CancellationThread by java.util.Timer --- .../worksheet/CancellationThread.scala | 29 ----------------- .../languageserver/worksheet/Evaluator.scala | 31 +++++++++++++------ 2 files changed, 21 insertions(+), 39 deletions(-) delete mode 100644 language-server/src/dotty/tools/languageserver/worksheet/CancellationThread.scala diff --git a/language-server/src/dotty/tools/languageserver/worksheet/CancellationThread.scala b/language-server/src/dotty/tools/languageserver/worksheet/CancellationThread.scala deleted file mode 100644 index f4dbf500e810..000000000000 --- a/language-server/src/dotty/tools/languageserver/worksheet/CancellationThread.scala +++ /dev/null @@ -1,29 +0,0 @@ -package dotty.tools.languageserver.worksheet - -import org.eclipse.lsp4j.jsonrpc.CancelChecker - -import java.util.concurrent.CancellationException - -/** - * Regularly check whether execution has been cancelled, kill REPL if it is. - */ -private class CancellationThread(@volatile private[this] var cancelChecker: CancelChecker, - evaluator: Evaluator) extends Thread { - private final val checkCancelledDelayMs = 50 - - override def run(): Unit = { - try { - while (evaluator.isAlive() && !Thread.interrupted()) { - cancelChecker.checkCanceled() - Thread.sleep(checkCancelledDelayMs) - } - } catch { - case _: CancellationException => evaluator.exit() - case _: InterruptedException => evaluator.exit() - } - } - - def setCancelChecker(cancelChecker: CancelChecker): Unit = { - this.cancelChecker = cancelChecker - } -} diff --git a/language-server/src/dotty/tools/languageserver/worksheet/Evaluator.scala b/language-server/src/dotty/tools/languageserver/worksheet/Evaluator.scala index caf5007e2f01..b1949f201d15 100644 --- a/language-server/src/dotty/tools/languageserver/worksheet/Evaluator.scala +++ b/language-server/src/dotty/tools/languageserver/worksheet/Evaluator.scala @@ -4,6 +4,8 @@ import dotty.tools.dotc.core.Contexts.Context import java.io.{File, PrintStream, IOException} import java.lang.ProcessBuilder.Redirect +import java.util.concurrent.CancellationException +import java.util.{Timer, TimerTask} import org.eclipse.lsp4j.jsonrpc.CancelChecker @@ -32,7 +34,7 @@ private object Evaluator { def get(cancelChecker: CancelChecker)(implicit ctx: Context): Option[Evaluator] = synchronized { val classpath = ctx.settings.classpath.value previousEvaluator match { - case Some(cp, evaluator) if evaluator.isAlive() && cp == classpath => + case Some(cp, evaluator) if evaluator.isAlive && cp == classpath => evaluator.reset(cancelChecker) Some(evaluator) case _ => @@ -53,7 +55,7 @@ private object Evaluator { */ private class Evaluator private (javaExec: String, userClasspath: String, - cancelChecker: CancelChecker) { + private var cancelChecker: CancelChecker) { private val process = new ProcessBuilder( javaExec, @@ -70,13 +72,24 @@ private class Evaluator private (javaExec: String, // Messages coming out of the REPL private val processOutput = new InputStreamConsumer(process.getInputStream()) - // The thread that monitors cancellation - private val cancellationThread = new CancellationThread(cancelChecker, this) - cancellationThread.start() + // A timer that monitors cancellation + private val cancellationTimer = new Timer() + locally { + val task = new TimerTask { + def run(): Unit = + if (!isAlive) + cancellationTimer.cancel() + else + try cancelChecker.checkCanceled() + catch { case _: CancellationException => exit() } + } + val checkCancelledDelayMs = 50L + cancellationTimer.schedule(task, checkCancelledDelayMs, checkCancelledDelayMs) + } /** Is the process that runs the REPL still alive? */ - def isAlive(): Boolean = process.isAlive() + def isAlive: Boolean = process.isAlive /** * Submit `command` to the REPL, wait for the result. @@ -97,7 +110,7 @@ private class Evaluator private (javaExec: String, * Reset the REPL to its initial state, update the cancel checker. */ def reset(cancelChecker: CancelChecker): Unit = { - cancellationThread.setCancelChecker(cancelChecker) + this.cancelChecker = cancelChecker eval(":reset") } @@ -105,8 +118,6 @@ private class Evaluator private (javaExec: String, def exit(): Unit = { process.destroyForcibly() Evaluator.previousEvaluator = None - cancellationThread.interrupt() + cancellationTimer.cancel() } - } -