[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)

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 {

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).

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")
+  }
+}