From 1e80d811a1494ce94bc964b6415165ddc6306246 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Tue, 27 May 2014 18:36:08 -0400 Subject: [PATCH 1/3] Read stderr from server for a short time before exiting Previously we exited immediately without waiting to get any error output. This patch also adds the command line and directory to the exception message on failure, in case the failure is due to getting one of those wrong, for example. --- .../scala/xsbt/boot/ServerApplication.scala | 40 +++++++++++++------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/launch/src/main/scala/xsbt/boot/ServerApplication.scala b/launch/src/main/scala/xsbt/boot/ServerApplication.scala index 3080f1226..b867a73d1 100644 --- a/launch/src/main/scala/xsbt/boot/ServerApplication.scala +++ b/launch/src/main/scala/xsbt/boot/ServerApplication.scala @@ -95,9 +95,9 @@ object ServerLocator { class StreamDumper(in: java.io.BufferedReader, out: java.io.PrintStream) extends Thread { // Don't block the application for this thread. setDaemon(true) - private val running = new java.util.concurrent.atomic.AtomicBoolean(true) + val endTime = new java.util.concurrent.atomic.AtomicLong(Long.MaxValue) override def run(): Unit = { - def read(): Unit = if (running.get) in.readLine match { + def read(): Unit = if (endTime.get > System.currentTimeMillis) in.readLine match { case null => () case line => out.println(line) @@ -107,7 +107,19 @@ class StreamDumper(in: java.io.BufferedReader, out: java.io.PrintStream) extends out.close() } - def close(): Unit = running.set(false) + def close(waitForErrors: Boolean): Unit = { + // closing "in" blocks forever on Windows, so don't do it; + // just wait a couple seconds to read more stuff if there is + // any stuff. + if (waitForErrors) { + endTime.set(System.currentTimeMillis + 2000) + // let ourselves read more (thread should exit on earlier of endTime or EOF) + while (isAlive() && (endTime.get > System.currentTimeMillis)) + Thread.sleep(50) + } else { + endTime.set(System.currentTimeMillis) + } + } } object ServerLauncher { import ServerApplication.SERVER_SYNCH_TEXT @@ -146,16 +158,20 @@ object ServerLauncher { // Now we look for the URI synch value, and then make sure we close the output files. try readUntilSynch(new java.io.BufferedReader(new java.io.InputStreamReader(stdout))) match { case Some(uri) => uri - case _ => sys.error("Failed to start server!") + case _ => + // attempt to get rid of the server (helps prevent hanging / stuck locks, + // though this is not reliable) + try process.destroy() catch { case e: Exception => } + // block a second to try to get stuff from stderr + errorDumper.close(waitForErrors = true) + sys.error(s"Failed to start server process in ${pb.directory} command line ${pb.command}") } finally { - errorDumper.close() + errorDumper.close(waitForErrors = false) stdout.close() - // Note: Closing this causes windows to block waiting for the server - // to close, but it ne'er will, as it is obstinate, and not designed - // to close immediately, unlike this process. - // We leave it open because this JVM shold be shut down soon anyway, - // and that will clean up al this memory. - //stderr.close() + // Do not close stderr here because on Windows that will block, + // and since the child process has no reason to exit, it may + // block forever. errorDumper.close() instead owns the problem + // of deciding what to do with stderr. } } @@ -202,4 +218,4 @@ object ServerLauncher { } catch { case e: Throwable => throw new RuntimeException("Unable to find sbt-launch.jar.", e) } -} \ No newline at end of file +} From 007a8f2a70a4a48f72f72e183bc1af5c560e423f Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Wed, 28 May 2014 10:07:35 -0400 Subject: [PATCH 2/3] ServerApplication: yield before we check on the dumper thread --- launch/src/main/scala/xsbt/boot/ServerApplication.scala | 3 +++ 1 file changed, 3 insertions(+) diff --git a/launch/src/main/scala/xsbt/boot/ServerApplication.scala b/launch/src/main/scala/xsbt/boot/ServerApplication.scala index b867a73d1..9133fd29b 100644 --- a/launch/src/main/scala/xsbt/boot/ServerApplication.scala +++ b/launch/src/main/scala/xsbt/boot/ServerApplication.scala @@ -113,6 +113,9 @@ class StreamDumper(in: java.io.BufferedReader, out: java.io.PrintStream) extends // any stuff. if (waitForErrors) { endTime.set(System.currentTimeMillis + 2000) + // at this point we'd rather the dumper thread run + // before we check whether to sleep + Thread.yield() // let ourselves read more (thread should exit on earlier of endTime or EOF) while (isAlive() && (endTime.get > System.currentTimeMillis)) Thread.sleep(50) From d57f5ac180e09e11eb555e5e54b2982c1c9967db Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Thu, 29 May 2014 12:32:10 -0400 Subject: [PATCH 3/3] put backticks around `yield` in Thread.yield --- launch/src/main/scala/xsbt/boot/ServerApplication.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/launch/src/main/scala/xsbt/boot/ServerApplication.scala b/launch/src/main/scala/xsbt/boot/ServerApplication.scala index 9133fd29b..343b37185 100644 --- a/launch/src/main/scala/xsbt/boot/ServerApplication.scala +++ b/launch/src/main/scala/xsbt/boot/ServerApplication.scala @@ -115,7 +115,7 @@ class StreamDumper(in: java.io.BufferedReader, out: java.io.PrintStream) extends endTime.set(System.currentTimeMillis + 2000) // at this point we'd rather the dumper thread run // before we check whether to sleep - Thread.yield() + Thread.`yield`() // let ourselves read more (thread should exit on earlier of endTime or EOF) while (isAlive() && (endTime.get > System.currentTimeMillis)) Thread.sleep(50)