From b0c840105be127d1f4039404569b4ed427fa0bd3 Mon Sep 17 00:00:00 2001 From: BrianHotopp Date: Tue, 4 Aug 2026 15:59:48 -0400 Subject: [PATCH] [2.x] fix: Bound the client boot wait when the forked server never starts (#9549) The thin client's blockUntilStart loop recursed while the portfile was missing and the forked process looked valid, with no deadline: a forked server that died before writing project/target/active.json, or stayed alive but wedged, hung the client forever with no output. On Windows the check ignored process death entirely (Properties.isWin short-circuited the liveness test), making the hang unconditional there. The wait is now bounded (default 5 minutes, tunable with -Dsbt.client.boot.timeout.seconds). On expiry the client fails with a "did not start within N seconds" message and then prints the server's captured stderr, instead of hanging silently (#9484). The Windows liveness workaround is preserved; the deadline is what bounds it. Generated-by: kimi-code/k3 (Oh My Pi) --- .../sbt/internal/client/NetworkClient.scala | 18 +++- notes/2.0.0/client-boot-timeout.md | 3 + .../scala/testpkg/ClientBootTimeoutTest.scala | 84 +++++++++++++++++++ 3 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 notes/2.0.0/client-boot-timeout.md create mode 100644 server-test/src/test/scala/testpkg/ClientBootTimeoutTest.scala diff --git a/main-command/src/main/scala/sbt/internal/client/NetworkClient.scala b/main-command/src/main/scala/sbt/internal/client/NetworkClient.scala index 37da42b03..012cab5fe 100644 --- a/main-command/src/main/scala/sbt/internal/client/NetworkClient.scala +++ b/main-command/src/main/scala/sbt/internal/client/NetworkClient.scala @@ -343,6 +343,14 @@ class NetworkClient( Try(ClientSocket.localSocket(namedPipeName, useJNI)).toOption case _ => None + private def connectTimeout: FiniteDuration = + sys.env + .get("SBT_CLIENT_CONNECT_TIMEOUT") + .flatMap(_.toIntOption) + .map(_.seconds) + .getOrElse(5.minutes) + private var connectDeadlineExpired = false + /** * Forks another instance of sbt in the background. * This instance must be shutdown explicitly via `sbt -client shutdown` @@ -482,6 +490,7 @@ class NetworkClient( } catch { case e: IOException => e.printStackTrace(System.err) } } } + val connectDeadline = connectTimeout.fromNow @tailrec def blockUntilStart(): Unit = { val stop = @@ -525,9 +534,10 @@ class NetworkClient( */ val existsValidProcess = process.fold(readThreadAlive.get)(p => p.isAlive || (Properties.isWin || p.exitValue == 2)) - if (!portfile.exists && !stop && existsValidProcess) { + if (!portfile.exists && !stop && existsValidProcess && !connectDeadline.isOverdue()) { blockUntilStart() } else { + connectDeadlineExpired = connectDeadline.isOverdue() && !portfile.exists socket.foreach { s => s.getInputStream.close() s.getOutputStream.close() @@ -549,6 +559,12 @@ class NetworkClient( Util.ignoreResult(Runtime.getRuntime.removeShutdownHook(shutdown)) } if (!portfile.exists()) { + if (connectDeadlineExpired) { + errorStream.write( + s"sbt server did not start within ${connectTimeout.toSeconds} seconds\n".getBytes("UTF-8") + ) + errorStream.flush() + } // Print captured server stderr so users can see why the server failed to start for (errFile <- serverStderrFile) { try { diff --git a/notes/2.0.0/client-boot-timeout.md b/notes/2.0.0/client-boot-timeout.md new file mode 100644 index 000000000..3edbc41a3 --- /dev/null +++ b/notes/2.0.0/client-boot-timeout.md @@ -0,0 +1,3 @@ +### Fixes + +- Don't hang forever when a forked sbt server never writes its portfile. The thin client now bounds the connect wait to 5 minutes (tunable with the `SBT_CLIENT_CONNECT_TIMEOUT` environment variable, in seconds), then fails with a "did not start within N seconds" message followed by the server's captured stderr. Previously the wait was unbounded, and on Windows it ignored process death entirely. Addresses [#9484](https://github.com/sbt/sbt/issues/9484). diff --git a/server-test/src/test/scala/testpkg/ClientBootTimeoutTest.scala b/server-test/src/test/scala/testpkg/ClientBootTimeoutTest.scala new file mode 100644 index 000000000..d0f6c76e9 --- /dev/null +++ b/server-test/src/test/scala/testpkg/ClientBootTimeoutTest.scala @@ -0,0 +1,84 @@ +/* + * 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.{ File, InputStream, OutputStream, PrintStream } +import java.nio.file.{ Files, Path } +import java.util.concurrent.TimeUnit +import sbt.internal.client.NetworkClient +import sbt.internal.util.Util + +/** Runs the thin client against a fake sbt script; exits with the client's exit code. */ +object ClientBootTimeoutMain { + def main(args: Array[String]): Unit = { + val Array(base, script) = args + val code = NetworkClient.client( + new File(base), + Array(s"--sbt-script=$script", "willSucceed"), + new InputStream { override def read(): Int = -1 }, + new PrintStream(OutputStream.nullOutputStream), + new PrintStream(System.err, true), + false + ) + System.exit(code) + } +} + +/** + * A forked server that never writes its portfile must not hang the client forever: + * the connect wait is bounded, and the failure explains itself. The client runs in a + * forked JVM because the timeout is configured via the SBT_CLIENT_CONNECT_TIMEOUT + * environment variable, which cannot be set in-process. + */ +class ClientBootTimeoutTest extends AbstractServerTest { + override val testDirectory: String = "client" + + private def fakeServer(script: String): String = { + val f = Files.createTempFile("fake-sbt", ".sh") + Files.writeString(f, script) + f.toFile.setExecutable(true) + f.toString + } + + test("a forked server that never starts fails within the connect timeout") { + val base = Files.createTempDirectory("connect-timeout-project").toFile + Files.writeString( + base.toPath.resolve("build.sbt"), + "lazy val root = (project in file(\".\"))\n" + ) + val script = fakeServer("#!/usr/bin/env bash\necho fake-server-wedged >&2\nsleep 600\n") + val errFile = Files.createTempFile("client-err", ".log") + val javaBin = Path.of(sys.props("java.home"), "bin", if (Util.isWindows) "java.exe" else "java") + val testClasses = Path.of(getClass.getProtectionDomain.getCodeSource.getLocation.toURI) + val pb = new ProcessBuilder( + javaBin.toString, + "-cp", + TestProperties.classpath + File.pathSeparator + testClasses, + "testpkg.ClientBootTimeoutMain", + base.toString, + script + ) + pb.environment().put("SBT_CLIENT_CONNECT_TIMEOUT", "3") + pb.redirectError(errFile.toFile) + val started = System.nanoTime() + val p = pb.start() + val finished = p.waitFor(60, TimeUnit.SECONDS) + val elapsed = (System.nanoTime() - started) / 1000000000L + if (!finished) p.destroyForcibly() + assert(finished, "client is still hanging after 60 seconds") + assert(p.exitValue != 0, s"expected failure, got ${p.exitValue}") + assert(elapsed < 45, s"connect wait was not bounded by the timeout (${elapsed}s)") + val errText = Files.readString(errFile) + assert( + errText.contains("did not start within 3 seconds"), + s"missing timeout message: $errText" + ) + assert(errText.contains("fake-server-wedged"), s"server stderr not forwarded: $errText") + } +}