From c78d6af74825dea2cd57c9d039e10a4e14ca602a Mon Sep 17 00:00:00 2001 From: BrianHotopp Date: Sun, 26 Jul 2026 01:06:36 -0400 Subject: [PATCH] [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) --- .../scala/sbt/internal/server/Server.scala | 2 +- .../scala/sbt/internal/CommandExchange.scala | 4 +- notes/2.0.0/reboot-reconnect.md | 15 +++++ .../scala/testpkg/AbstractServerTest.scala | 11 ++- .../src/test/scala/testpkg/RebootTest.scala | 67 +++++++++++++++++++ 5 files changed, 94 insertions(+), 5 deletions(-) create mode 100644 notes/2.0.0/reboot-reconnect.md create mode 100644 server-test/src/test/scala/testpkg/RebootTest.scala diff --git a/main-command/src/main/scala/sbt/internal/server/Server.scala b/main-command/src/main/scala/sbt/internal/server/Server.scala index d7ee44284..e71d6e643 100644 --- a/main-command/src/main/scala/sbt/internal/server/Server.scala +++ b/main-command/src/main/scala/sbt/internal/server/Server.scala @@ -159,7 +159,6 @@ private[sbt] object Server { } override def shutdown(): Unit = { - log.info("shutting down sbt server") if (portfile.exists) { IO.delete(portfile) } @@ -171,6 +170,7 @@ private[sbt] object Server { case null => case s => s.close() } + log.info("shutting down sbt server") } private def writeTokenfile(): Unit = { diff --git a/main/src/main/scala/sbt/internal/CommandExchange.scala b/main/src/main/scala/sbt/internal/CommandExchange.scala index 3a5979e9f..8b2010c1f 100644 --- a/main/src/main/scala/sbt/internal/CommandExchange.scala +++ b/main/src/main/scala/sbt/internal/CommandExchange.scala @@ -320,9 +320,9 @@ private[sbt] final class CommandExchange { } procFile = None fastTrackThread.close() - channels foreach (_.shutdown(true)) + channels.foreach(c => Util.ignoreResult(Try(c.shutdown(true)))) // interrupt and kill the thread - server.foreach(_.shutdown()) + server.foreach(s => Util.ignoreResult(Try(s.shutdown()))) server = None EvaluateTask.onShutdown() } diff --git a/notes/2.0.0/reboot-reconnect.md b/notes/2.0.0/reboot-reconnect.md new file mode 100644 index 000000000..251be0527 --- /dev/null +++ b/notes/2.0.0/reboot-reconnect.md @@ -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 diff --git a/server-test/src/test/scala/testpkg/AbstractServerTest.scala b/server-test/src/test/scala/testpkg/AbstractServerTest.scala index ca66b95b1..11f438db7 100644 --- a/server-test/src/test/scala/testpkg/AbstractServerTest.scala +++ b/server-test/src/test/scala/testpkg/AbstractServerTest.scala @@ -27,8 +27,15 @@ final class SbtServer( val baseDirectory: File, private val process: scala.sys.process.Process ) { - def close(): Unit = - session.shutdown(process.isAlive(), () => process.destroy()).get + def close(): Unit = { + 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 { diff --git a/server-test/src/test/scala/testpkg/RebootTest.scala b/server-test/src/test/scala/testpkg/RebootTest.scala new file mode 100644 index 000000000..23a7c60db --- /dev/null +++ b/server-test/src/test/scala/testpkg/RebootTest.scala @@ -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") + } +}