mirror of https://github.com/sbt/sbt.git
[2.x] fix: Complete server teardown before logging so reboot works from sbtn (#9497)
Running reboot in the sbt shell dropped to the OS shell instead of rebooting. The break was in teardown: Server.shutdown opened with log.info, and during a client-initiated reboot the terminal in scope is that client's already-closed virtual terminal, so the log write throws ClosedChannelException through the terminal proxy. That aborted teardown before the portfile was deleted and the server socket closed, and the exception was swallowed by the shutdown hook (whose own error print goes to the same dead terminal). Server.shutdown now completes its state cleanup (portfile, tokenfile, running flag, server socket) before logging, and CommandExchange.shutdown wraps each channel shutdown and the server shutdown individually so one failing step cannot skip the rest. Fixes #9095 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
9f0a459f81
commit
c78d6af748
|
|
@ -159,7 +159,6 @@ private[sbt] object Server {
|
||||||
}
|
}
|
||||||
|
|
||||||
override def shutdown(): Unit = {
|
override def shutdown(): Unit = {
|
||||||
log.info("shutting down sbt server")
|
|
||||||
if (portfile.exists) {
|
if (portfile.exists) {
|
||||||
IO.delete(portfile)
|
IO.delete(portfile)
|
||||||
}
|
}
|
||||||
|
|
@ -171,6 +170,7 @@ private[sbt] object Server {
|
||||||
case null =>
|
case null =>
|
||||||
case s => s.close()
|
case s => s.close()
|
||||||
}
|
}
|
||||||
|
log.info("shutting down sbt server")
|
||||||
}
|
}
|
||||||
|
|
||||||
private def writeTokenfile(): Unit = {
|
private def writeTokenfile(): Unit = {
|
||||||
|
|
|
||||||
|
|
@ -320,9 +320,9 @@ private[sbt] final class CommandExchange {
|
||||||
}
|
}
|
||||||
procFile = None
|
procFile = None
|
||||||
fastTrackThread.close()
|
fastTrackThread.close()
|
||||||
channels foreach (_.shutdown(true))
|
channels.foreach(c => Util.ignoreResult(Try(c.shutdown(true))))
|
||||||
// interrupt and kill the thread
|
// interrupt and kill the thread
|
||||||
server.foreach(_.shutdown())
|
server.foreach(s => Util.ignoreResult(Try(s.shutdown())))
|
||||||
server = None
|
server = None
|
||||||
EvaluateTask.onShutdown()
|
EvaluateTask.onShutdown()
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,15 @@
|
||||||
|
### `reboot` works from the thin client again
|
||||||
|
|
||||||
|
Running `reboot` in the sbt shell dropped to the OS shell ("sbt server connection
|
||||||
|
closed") instead of rebooting. The server's teardown began with a log line that
|
||||||
|
throws when the terminal in scope is the rebooting client's already-closed
|
||||||
|
virtual terminal, aborting teardown before the server socket was closed and the
|
||||||
|
portfile deleted; the relaunched instance then mistook the leaked socket for
|
||||||
|
another running sbt and never started its server, while the client latched onto
|
||||||
|
the stale portfile. Server teardown now completes its state cleanup before
|
||||||
|
logging, and one failing channel shutdown can no longer skip the rest of the
|
||||||
|
exchange teardown. `reboot` returns to a working prompt.
|
||||||
|
|
||||||
|
This addresses [#9095][i9095].
|
||||||
|
|
||||||
|
[i9095]: https://github.com/sbt/sbt/issues/9095
|
||||||
|
|
@ -27,8 +27,15 @@ final class SbtServer(
|
||||||
val baseDirectory: File,
|
val baseDirectory: File,
|
||||||
private val process: scala.sys.process.Process
|
private val process: scala.sys.process.Process
|
||||||
) {
|
) {
|
||||||
def close(): Unit =
|
def close(): Unit = {
|
||||||
session.shutdown(process.isAlive(), () => process.destroy()).get
|
val result = scala.util.Try(session.shutdown(process.isAlive(), () => process.destroy()).get)
|
||||||
|
if (process.isAlive()) process.destroy()
|
||||||
|
result match {
|
||||||
|
case scala.util.Failure(e) =>
|
||||||
|
System.err.println(s"server session shutdown failed (process destroyed): $e")
|
||||||
|
case _ =>
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
trait AbstractServerTest extends AnyFunSuite with BeforeAndAfterAll {
|
trait AbstractServerTest extends AnyFunSuite with BeforeAndAfterAll {
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,67 @@
|
||||||
|
/*
|
||||||
|
* sbt
|
||||||
|
* Copyright 2023, Scala center
|
||||||
|
* Copyright 2011 - 2022, Lightbend, Inc.
|
||||||
|
* Copyright 2008 - 2010, Mark Harrah
|
||||||
|
* Licensed under Apache License 2.0 (see LICENSE)
|
||||||
|
*/
|
||||||
|
|
||||||
|
package testpkg
|
||||||
|
|
||||||
|
import java.io.{ InputStream, PrintStream }
|
||||||
|
import java.util.concurrent.{ LinkedBlockingQueue, TimeUnit, TimeoutException }
|
||||||
|
import sbt.internal.client.NetworkClient
|
||||||
|
import sbt.internal.util.Util
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Regression for https://github.com/sbt/sbt/issues/9095: `reboot` from a client must bring the
|
||||||
|
* server back and complete instead of leaving a zombie server that drops the client.
|
||||||
|
*/
|
||||||
|
class RebootTest extends AbstractServerTest {
|
||||||
|
override val testDirectory: String = "client"
|
||||||
|
|
||||||
|
private object BlockingInputStream extends InputStream {
|
||||||
|
override def read(): Int = {
|
||||||
|
try Thread.sleep(Long.MaxValue)
|
||||||
|
catch { case _: InterruptedException => }
|
||||||
|
-1
|
||||||
|
}
|
||||||
|
}
|
||||||
|
private val nullPrintStream = new PrintStream(_ => {}, false)
|
||||||
|
|
||||||
|
private def background[R](f: => R): R = {
|
||||||
|
val result = new LinkedBlockingQueue[Either[Throwable, R]]
|
||||||
|
val thread = new Thread("reboot-test-client") {
|
||||||
|
setDaemon(true)
|
||||||
|
override def run(): Unit =
|
||||||
|
try Util.ignoreResult(result.put(Right(f)))
|
||||||
|
catch { case e: Throwable => Util.ignoreResult(result.put(Left(e))) }
|
||||||
|
}
|
||||||
|
thread.start()
|
||||||
|
result.poll(3, TimeUnit.MINUTES) match {
|
||||||
|
case null =>
|
||||||
|
thread.interrupt()
|
||||||
|
thread.join(10000)
|
||||||
|
throw new TimeoutException("client did not complete within 3 minutes")
|
||||||
|
case Left(e) => throw e
|
||||||
|
case Right(r) => r
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private def client(args: String*): Int =
|
||||||
|
background(
|
||||||
|
NetworkClient.client(
|
||||||
|
testPath.toFile,
|
||||||
|
args.toArray,
|
||||||
|
BlockingInputStream,
|
||||||
|
nullPrintStream,
|
||||||
|
nullPrintStream,
|
||||||
|
false
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
test("reboot completes and the rebooted server serves the next command") {
|
||||||
|
assert(client("reboot") == 0, "reboot from a client must complete with exit 0")
|
||||||
|
assert(client("willSucceed") == 0, "the rebooted server must serve a new client connection")
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Reference in New Issue