From dbeeb199aa2acb7e2a72d5c58535a4e9daa17673 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Thu, 10 Sep 2026 09:55:11 -0700 Subject: [PATCH] [2.x] fix: Stop a displaced server (#9713) **Problem** If a client cannot reach the server, it deletes the portfile. Then it starts a second server. The first server is displaced: its socket path now belongs to the second server. A displaced server keeps running. A dropIfIdle notification cannot reach it, because the proc file it registered names the socket that the second server now owns. **Solution** The portfile holds the serverId of whichever server wrote it. Watch that file, and exit when the id is not this server's. Leave the portfile to the server that owns it. --------- Co-authored-by: Claude Opus 5 (1M context) --- .../sbt/internal/client/NetworkClient.scala | 107 +++++++++--------- .../scala/sbt/internal/server/Server.scala | 39 ++++--- .../sbt/internal/server/ServerIdSpec.scala | 45 ++++++++ .../scala/sbt/internal/CommandExchange.scala | 56 ++++++++- .../sbt/internal/protocol/PortFile.scala | 26 +++-- .../protocol/codec/PortFileFormats.scala | 4 +- protocol/src/main/contraband/portfile.contra | 3 + .../scala/sbt/protocol/ClientSocket.scala | 16 +-- .../scala/testpkg/AbstractServerTest.scala | 7 ++ .../scala/testpkg/PortfileReloadTest.scala | 31 +++++ .../src/test/scala/testpkg/PortfileTest.scala | 38 +++++++ 11 files changed, 279 insertions(+), 93 deletions(-) create mode 100644 main-command/src/test/scala/sbt/internal/server/ServerIdSpec.scala create mode 100644 server-test/src/test/scala/testpkg/PortfileReloadTest.scala create mode 100644 server-test/src/test/scala/testpkg/PortfileTest.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 4debab452..76e49bd2f 100644 --- a/main-command/src/main/scala/sbt/internal/client/NetworkClient.scala +++ b/main-command/src/main/scala/sbt/internal/client/NetworkClient.scala @@ -221,61 +221,60 @@ class NetworkClient( .map(NetworkClient.sysPropName) .toSet val current = NetworkClient.serverSysProps(arguments.sbtArguments) - Try(ClientSocket.portFile(portfile)).toOption match - case Some(pf) => - val (dropped, added, changed) = - NetworkClient.sysPropsDiff(pf.sysProps, current, deferred) - if (dropped ++ added ++ changed).nonEmpty then - // a server nothing recorded the options of may well have the ones this client - // carries already, and an editor's server is the usual one to be in that state, - // so it gets a word rather than a shutdown it never asked for - val known = pf.sysPropsRecorded.contains(true) - val restarts = known && serverAutoStart && serverAutoRestart - val level = if restarts then Level.Info else Level.Warn - // the values are what a credential would be hiding in, so only the names of - // the options are worth saying out loud - console.appendLog( - level, - if restarts then "sbt server is running with different JVM options; restarting it" - else if known then - "sbt server is running with different JVM options, which it cannot pick up" - else - "sbt server was started by something other than the thin client, so it may " - + "not have these JVM options" - ) - if dropped.nonEmpty then console.appendLog(level, s"dropped: ${dropped.mkString(" ")}") - if added.nonEmpty then console.appendLog(level, s"added: ${added.mkString(" ")}") - if changed.nonEmpty then console.appendLog(level, s"changed: ${changed.mkString(" ")}") - if !restarts then console.appendLog(level, "run 'sbt shutdown' for them to take effect") + ClientSocket.loadPortFile(portfile).foreach { pf => + val (dropped, added, changed) = + NetworkClient.sysPropsDiff(pf.sysProps, current, deferred) + if (dropped ++ added ++ changed).nonEmpty then + // a server nothing recorded the options of may well have the ones this client + // carries already, and an editor's server is the usual one to be in that state, + // so it gets a word rather than a shutdown it never asked for + val known = pf.sysPropsRecorded.contains(true) + val restarts = known && serverAutoStart && serverAutoRestart + val level = if restarts then Level.Info else Level.Warn + // the values are what a credential would be hiding in, so only the names of + // the options are worth saying out loud + console.appendLog( + level, + if restarts then "sbt server is running with different JVM options; restarting it" + else if known then + "sbt server is running with different JVM options, which it cannot pick up" else - shutdownRunningServer(pf.uri) match - case Some(true) => () - case Some(false) => - console.appendLog( - Level.Error, - "the sbt server did not shut down, it is most likely busy with another client" - ) - // the request is queued on it and stays there, so a second one buys nothing - console.appendLog( - Level.Error, - "it has the request and takes it once that work is done, which ends that" - + " client's session too" - ) - console.appendLog(Level.Error, "run this command again once the server is gone") - throw new ServerFailedException - case None => - // it answers no socket but is still there, which says nothing about how - // busy it is, so leaving it alone beats failing an invocation over it - console.appendLog( - Level.Warn, - "the sbt server could not be reached to restart it; it keeps the JVM" - + " options it was started with" - ) - console.appendLog( - Level.Warn, - "run 'sbt shutdown' for the ones passed here to take effect" - ) - case _ => () + "sbt server was started by something other than the thin client, so it may " + + "not have these JVM options" + ) + if dropped.nonEmpty then console.appendLog(level, s"dropped: ${dropped.mkString(" ")}") + if added.nonEmpty then console.appendLog(level, s"added: ${added.mkString(" ")}") + if changed.nonEmpty then console.appendLog(level, s"changed: ${changed.mkString(" ")}") + if !restarts then console.appendLog(level, "run 'sbt shutdown' for them to take effect") + else + shutdownRunningServer(pf.uri) match + case Some(true) => () + case Some(false) => + console.appendLog( + Level.Error, + "the sbt server did not shut down, it is most likely busy with another client" + ) + // the request is queued on it and stays there, so a second one buys nothing + console.appendLog( + Level.Error, + "it has the request and takes it once that work is done, which ends that" + + " client's session too" + ) + console.appendLog(Level.Error, "run this command again once the server is gone") + throw new ServerFailedException + case None => + // it answers no socket but is still there, which says nothing about how + // busy it is, so leaving it alone beats failing an invocation over it + console.appendLog( + Level.Warn, + "the sbt server could not be reached to restart it; it keeps the JVM" + + " options it was started with" + ) + console.appendLog( + Level.Warn, + "run 'sbt shutdown' for the ones passed here to take effect" + ) + } /** * Asks the running server to shut down and waits for it to let go of its socket, so that 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 1c9adc609..4b83766e5 100644 --- a/main-command/src/main/scala/sbt/internal/server/Server.scala +++ b/main-command/src/main/scala/sbt/internal/server/Server.scala @@ -28,10 +28,12 @@ import sbt.internal.util.ErrorHandling import sbt.internal.util.Util.isWindows import org.scalasbt.ipcsocket.* import sbt.internal.bsp.BuildServerConnection +import sbt.protocol.ClientSocket import xsbti.AppConfiguration private[sbt] sealed trait ServerInstance { def shutdown(): Unit + def serverId: String def ready: Future[Unit] def authenticate(challenge: String): Boolean } @@ -43,6 +45,10 @@ private[sbt] object Server { with TokenFileFormats object JsonProtocol extends JsonProtocol + /** The id the portfile names, None when it names none, and a failure when unreadable. */ + private[sbt] def serverIdOf(portfile: File): Try[Option[String]] = + ClientSocket.loadPortFile(portfile).map(_.serverId) + def start( connection: ServerConnection, onIncomingSocket: (AtomicReference[Socket], ServerInstance) => Unit, @@ -56,6 +62,7 @@ private[sbt] object Server { private val rand = new SecureRandom private var token: String = nextToken private val serverSocketHolder = AtomicCloseable[ServerSocket]() + override val serverId: String = java.util.UUID.randomUUID().toString val serverThread = new Thread("sbt-socket-server") { override def run(): Unit = { @@ -157,12 +164,8 @@ private[sbt] object Server { } override def shutdown(): Unit = { - if (portfile.exists) { - IO.delete(portfile) - } - if (tokenfile.exists) { - IO.delete(tokenfile) - } + if (serverIdOf(portfile).getOrElse(None).contains(serverId)) IO.delete(portfile) + IO.delete(tokenfile) running.set(false) serverSocketHolder.close() log.info("shutting down sbt server") @@ -197,20 +200,16 @@ private[sbt] object Server { // which of the two this is: a client can restart a server over the first but has no // business taking down one whose options it never saw val sysPropsRecorded = Option(startedByThisBuild) - val p = - auth match { - case _ if auth(ServerAuthentication.Token) => - writeTokenfile() - PortFile( - uri, - Option(tokenfile.toString), - Option(IO.toURI(tokenfile).toString), - sysProps, - sysPropsRecorded - ) - case _ => - PortFile(uri, None, None, sysProps, sysPropsRecorded) - } + val authOK = auth(ServerAuthentication.Token) + if (authOK) writeTokenfile() + val p = PortFile( + uri, + if (authOK) Some(tokenfile.toString) else None, + if (authOK) Some(IO.toURI(tokenfile).toString) else None, + sysProps, + sysPropsRecorded, + Some(serverId) + ) val json = Converter.toJson(p).get IO.writeFileAtomically(portfile)(tmp => IO.write(tmp, CompactPrinter(json))) } diff --git a/main-command/src/test/scala/sbt/internal/server/ServerIdSpec.scala b/main-command/src/test/scala/sbt/internal/server/ServerIdSpec.scala new file mode 100644 index 000000000..d63542a30 --- /dev/null +++ b/main-command/src/test/scala/sbt/internal/server/ServerIdSpec.scala @@ -0,0 +1,45 @@ +/* + * sbt + * Copyright 2023, Scala center + * Copyright 2011 - 2022, Lightbend, Inc. + * Copyright 2008 - 2010, Mark Harrah + * Licensed under Apache License 2.0 (see LICENSE) + */ + +package sbt +package internal +package server + +import java.io.{ File, FileNotFoundException } +import java.nio.file.Files + +import scala.util.Success + +import verify.BasicTestSuite + +// check() tells these outcomes apart, so each one has to stay distinguishable +object ServerIdSpec extends BasicTestSuite: + private def withPortfile(content: Option[String])(f: File => Unit): Unit = + val dir = Files.createTempDirectory("portfile").toFile + val portfile = new File(dir, "active.json") + content.foreach(sbt.io.IO.write(portfile, _)) + try f(portfile) + finally sbt.io.IO.delete(dir) + + test("a portfile that is not there"): + withPortfile(None): portfile => + val failure = Server.serverIdOf(portfile).failed.get + assert(failure.isInstanceOf[FileNotFoundException]) + + test("a portfile that is not json"): + withPortfile(Some("this is not json")): portfile => + assert(Server.serverIdOf(portfile).isFailure) + + test("a portfile that names no server"): + withPortfile(Some("""{"uri":"local:///sock"}""")): portfile => + assert(Server.serverIdOf(portfile) == Success(None)) + + test("a portfile that names a server"): + withPortfile(Some("""{"uri":"local:///sock","serverId":"an-id"}""")): portfile => + assert(Server.serverIdOf(portfile) == Success(Some("an-id"))) +end ServerIdSpec diff --git a/main/src/main/scala/sbt/internal/CommandExchange.scala b/main/src/main/scala/sbt/internal/CommandExchange.scala index 17b87bc23..7f79ee8c3 100644 --- a/main/src/main/scala/sbt/internal/CommandExchange.scala +++ b/main/src/main/scala/sbt/internal/CommandExchange.scala @@ -30,6 +30,8 @@ import sbt.internal.util.* import sbt.io.syntax.* import sbt.io.{ Hash, IO } import sbt.nio.Watch.NullLogger +import sbt.internal.nio.FileTreeRepository +import sbt.nio.file.FileAttributes import sbt.protocol.Serialization.attach import sbt.protocol.{ ExecStatusEvent, LogEvent } import sbt.util.Logger @@ -53,6 +55,8 @@ private[sbt] final class CommandExchange { private var server: Option[ServerInstance] = None private val firstInstance: AtomicBoolean = new AtomicBoolean(true) private val monitoringActiveJson: AtomicBoolean = new AtomicBoolean(false) + private val watchedRepository = new AtomicReference[AnyRef] + private val portfileWatch = AtomicCloseable[AutoCloseable]() private val commandQueue: LinkedBlockingQueue[Exec] = new LinkedBlockingQueue[Exec] private val channelBuffer: ListBuffer[CommandChannel] = new ListBuffer() private val channelBufferLock = new AnyRef {} @@ -307,9 +311,54 @@ private[sbt] final class CommandExchange { case _ => } } + server.foreach { instance => + s.get(sbt.nio.Keys.globalFileTreeRepository).foreach { repo => + if (watchedRepository.get ne repo) watchPortfile(instance, portfile, repo) + } + } s.remove(Keys.bootServerSocket) } + /** + * Registers a watch on the portfile. A project load closes the file tree repository, so the + * caller registers again on the one that replaced it, and this reads the file once because no + * event arrived while the old watch was closed. + */ + private def watchPortfile( + instance: ServerInstance, + portfile: File, + repo: FileTreeRepository[FileAttributes] + ): Unit = { + def check(): Unit = Server.serverIdOf(portfile) match { + case Success(id) if !id.contains(instance.serverId) => + exitServer("another sbt server took over this build") + case _ => + } + if (replaceWatch(portfile, repo, portfileWatch)(_.addObserver(_ => check()))) + watchedRepository.set(repo) + check() + } + + private def replaceWatch[A]( + portfile: File, + repo: nio.Registerable[A], + watch: AtomicCloseable[AutoCloseable] + )(setUp: nio.Observable[A] => Unit): Boolean = { + watch.close() + // a repository that a failed load left closed throws instead of returning a Left + Try(repo.register(sbt.nio.file.Glob(portfile))).flatMap(_.toTry) match { + case Success(o) => setUp(o); watch.set(o); true + case Failure(e) => + Terminal.consoleLog(s"sbt server cannot watch $portfile: $e") + false + } + } + + private def exitServer(reason: String): Unit = { + Terminal.consoleLog(s"$reason; exiting") + shutdown(ConsoleChannel.defaultName) + } + def shutdown(): Unit = { shuttingDown.set(true) procFile.foreach { pf => @@ -318,6 +367,7 @@ private[sbt] final class CommandExchange { } procFile = None fastTrackThread.close() + portfileWatch.close() channels.foreach(c => Util.ignoreTry(c.shutdown(true))) // interrupt and kill the thread server.foreach(s => Util.ignoreTry(s.shutdown())) @@ -520,10 +570,8 @@ private[sbt] final class CommandExchange { case nc: NetworkChannel => nc.isInitialized case _ => false } - if (idle && !hasClients) { - Terminal.consoleLog("dropping idle server (requested by another sbt instance)") - commandQueue.add(Exec(TerminateAction, Some(CommandSource(ConsoleChannel.defaultName)))) - } + if (idle && !hasClients) + exitServer("dropping idle server (requested by another sbt instance)") } /** Notify other sbt servers to drop if idle. Runs on a daemon thread to avoid blocking. */ diff --git a/protocol/src/main/contraband-scala/sbt/internal/protocol/PortFile.scala b/protocol/src/main/contraband-scala/sbt/internal/protocol/PortFile.scala index 02fdd218f..69116981e 100644 --- a/protocol/src/main/contraband-scala/sbt/internal/protocol/PortFile.scala +++ b/protocol/src/main/contraband-scala/sbt/internal/protocol/PortFile.scala @@ -11,28 +11,32 @@ package sbt.internal.protocol * @param sysProps The -D options the thin client passed to this server, as names and salted digests. * @param sysPropsRecorded Whether sysProps is the whole story. Absent on a server no thin client started, whose options are its own business and unknown here. + * @param serverId Identifies the server that wrote this file. A server that reads an id other than its own + has been replaced, and no client can reach it any more. */ final class PortFile private ( val uri: String, val tokenfilePath: Option[String], val tokenfileUri: Option[String], val sysProps: Vector[String], - val sysPropsRecorded: Option[Boolean]) extends Serializable { + val sysPropsRecorded: Option[Boolean], + val serverId: Option[String]) extends Serializable { - private def this(uri: String, tokenfilePath: Option[String], tokenfileUri: Option[String]) = this(uri, tokenfilePath, tokenfileUri, Vector(), None) + private def this(uri: String, tokenfilePath: Option[String], tokenfileUri: Option[String]) = this(uri, tokenfilePath, tokenfileUri, Vector(), None, None) + private def this(uri: String, tokenfilePath: Option[String], tokenfileUri: Option[String], sysProps: Vector[String], sysPropsRecorded: Option[Boolean]) = this(uri, tokenfilePath, tokenfileUri, sysProps, sysPropsRecorded, None) override def equals(o: Any): Boolean = this.eq(o.asInstanceOf[AnyRef]) || (o match { - case x: PortFile => (this.uri == x.uri) && (this.tokenfilePath == x.tokenfilePath) && (this.tokenfileUri == x.tokenfileUri) && (this.sysProps == x.sysProps) && (this.sysPropsRecorded == x.sysPropsRecorded) + case x: PortFile => (this.uri == x.uri) && (this.tokenfilePath == x.tokenfilePath) && (this.tokenfileUri == x.tokenfileUri) && (this.sysProps == x.sysProps) && (this.sysPropsRecorded == x.sysPropsRecorded) && (this.serverId == x.serverId) case _ => false }) override def hashCode: Int = { - 37 * (37 * (37 * (37 * (37 * (37 * (17 + "sbt.internal.protocol.PortFile".##) + uri.##) + tokenfilePath.##) + tokenfileUri.##) + sysProps.##) + sysPropsRecorded.##) + 37 * (37 * (37 * (37 * (37 * (37 * (37 * (17 + "sbt.internal.protocol.PortFile".##) + uri.##) + tokenfilePath.##) + tokenfileUri.##) + sysProps.##) + sysPropsRecorded.##) + serverId.##) } override def toString: String = { - "PortFile(" + uri + ", " + tokenfilePath + ", " + tokenfileUri + ", " + sysProps + ", " + sysPropsRecorded + ")" + "PortFile(" + uri + ", " + tokenfilePath + ", " + tokenfileUri + ", " + sysProps + ", " + sysPropsRecorded + ", " + serverId + ")" } - private def copy(uri: String = uri, tokenfilePath: Option[String] = tokenfilePath, tokenfileUri: Option[String] = tokenfileUri, sysProps: Vector[String] = sysProps, sysPropsRecorded: Option[Boolean] = sysPropsRecorded): PortFile = { - new PortFile(uri, tokenfilePath, tokenfileUri, sysProps, sysPropsRecorded) + private def copy(uri: String = uri, tokenfilePath: Option[String] = tokenfilePath, tokenfileUri: Option[String] = tokenfileUri, sysProps: Vector[String] = sysProps, sysPropsRecorded: Option[Boolean] = sysPropsRecorded, serverId: Option[String] = serverId): PortFile = { + new PortFile(uri, tokenfilePath, tokenfileUri, sysProps, sysPropsRecorded, serverId) } def withUri(uri: String): PortFile = { copy(uri = uri) @@ -58,6 +62,12 @@ final class PortFile private ( def withSysPropsRecorded(sysPropsRecorded: Boolean): PortFile = { copy(sysPropsRecorded = Option(sysPropsRecorded)) } + def withServerId(serverId: Option[String]): PortFile = { + copy(serverId = serverId) + } + def withServerId(serverId: String): PortFile = { + copy(serverId = Option(serverId)) + } } object PortFile { @@ -65,4 +75,6 @@ object PortFile { def apply(uri: String, tokenfilePath: String, tokenfileUri: String): PortFile = new PortFile(uri, Option(tokenfilePath), Option(tokenfileUri)) def apply(uri: String, tokenfilePath: Option[String], tokenfileUri: Option[String], sysProps: Vector[String], sysPropsRecorded: Option[Boolean]): PortFile = new PortFile(uri, tokenfilePath, tokenfileUri, sysProps, sysPropsRecorded) def apply(uri: String, tokenfilePath: String, tokenfileUri: String, sysProps: Vector[String], sysPropsRecorded: Boolean): PortFile = new PortFile(uri, Option(tokenfilePath), Option(tokenfileUri), sysProps, Option(sysPropsRecorded)) + def apply(uri: String, tokenfilePath: Option[String], tokenfileUri: Option[String], sysProps: Vector[String], sysPropsRecorded: Option[Boolean], serverId: Option[String]): PortFile = new PortFile(uri, tokenfilePath, tokenfileUri, sysProps, sysPropsRecorded, serverId) + def apply(uri: String, tokenfilePath: String, tokenfileUri: String, sysProps: Vector[String], sysPropsRecorded: Boolean, serverId: String): PortFile = new PortFile(uri, Option(tokenfilePath), Option(tokenfileUri), sysProps, Option(sysPropsRecorded), Option(serverId)) } diff --git a/protocol/src/main/contraband-scala/sbt/internal/protocol/codec/PortFileFormats.scala b/protocol/src/main/contraband-scala/sbt/internal/protocol/codec/PortFileFormats.scala index 383d57d97..02721d7bd 100644 --- a/protocol/src/main/contraband-scala/sbt/internal/protocol/codec/PortFileFormats.scala +++ b/protocol/src/main/contraband-scala/sbt/internal/protocol/codec/PortFileFormats.scala @@ -16,8 +16,9 @@ given PortFileFormat: JsonFormat[sbt.internal.protocol.PortFile] = new JsonForma val tokenfileUri = unbuilder.readField[Option[String]]("tokenfileUri") val sysProps = unbuilder.readField[Vector[String]]("sysProps") val sysPropsRecorded = unbuilder.readField[Option[Boolean]]("sysPropsRecorded") + val serverId = unbuilder.readField[Option[String]]("serverId") unbuilder.endObject() - sbt.internal.protocol.PortFile(uri, tokenfilePath, tokenfileUri, sysProps, sysPropsRecorded) + sbt.internal.protocol.PortFile(uri, tokenfilePath, tokenfileUri, sysProps, sysPropsRecorded, serverId) case None => deserializationError("Expected JsObject but found None") } @@ -29,6 +30,7 @@ given PortFileFormat: JsonFormat[sbt.internal.protocol.PortFile] = new JsonForma builder.addField("tokenfileUri", obj.tokenfileUri) builder.addField("sysProps", obj.sysProps) builder.addField("sysPropsRecorded", obj.sysPropsRecorded) + builder.addField("serverId", obj.serverId) builder.endObject() } } diff --git a/protocol/src/main/contraband/portfile.contra b/protocol/src/main/contraband/portfile.contra index f230f9dd7..5aa68ef73 100644 --- a/protocol/src/main/contraband/portfile.contra +++ b/protocol/src/main/contraband/portfile.contra @@ -14,6 +14,9 @@ type PortFile { ## Whether sysProps is the whole story. Absent on a server no thin client started, whose ## options are its own business and unknown here. sysPropsRecorded: Boolean @since("2.0.7") + ## Identifies the server that wrote this file. A server that reads an id other than its own + ## has been replaced, and no client can reach it any more. + serverId: String @since("2.0.9") } type TokenFile { diff --git a/protocol/src/main/scala/sbt/protocol/ClientSocket.scala b/protocol/src/main/scala/sbt/protocol/ClientSocket.scala index 939db0949..f55f224ef 100644 --- a/protocol/src/main/scala/sbt/protocol/ClientSocket.scala +++ b/protocol/src/main/scala/sbt/protocol/ClientSocket.scala @@ -21,6 +21,8 @@ import sbt.internal.protocol.codec.{ PortFileFormats, TokenFileFormats } import sbt.internal.util.Util.isWindows import org.scalasbt.ipcsocket.* +import scala.util.{ Failure, Success, Try } + object ClientSocket { private lazy val fileFormats = new BasicJsonProtocol with PortFileFormats with TokenFileFormats {} @@ -31,19 +33,19 @@ object ClientSocket { def socket(portfile: File): (Socket, Option[String]) = socket(portfile, false) /** Parses the connection file written by the server. */ - private[sbt] def portFile(portfile: File): PortFile = { + private[sbt] def loadPortFile(portfile: File): Try[PortFile] = { import fileFormats.given - try - val json: JValue = Parser.parseFromString(sbt.io.IO.read(portfile)).get - Converter.fromJson[PortFile](json).get - catch case NonFatal(e) => throw new ConnectionFileReadException(portfile, e) + val parsed = Try(sbt.io.IO.read(portfile)).flatMap(Parser.parseFromString) + parsed.flatMap(Converter.fromJson[PortFile]) } def socket(portfile: File, useJNI: Boolean): (Socket, Option[String]) = { import fileFormats.given - val p = portFile(portfile) + val p = loadPortFile(portfile) match { + case Success(p) => p + case Failure(e) => throw new ConnectionFileReadException(portfile, e) + } val uri = new URI(p.uri) - // println(uri) val token = p.tokenfilePath map { tp => val tokeFile = new File(tp) try diff --git a/server-test/src/test/scala/testpkg/AbstractServerTest.scala b/server-test/src/test/scala/testpkg/AbstractServerTest.scala index 0ea4ad509..20c0129c5 100644 --- a/server-test/src/test/scala/testpkg/AbstractServerTest.scala +++ b/server-test/src/test/scala/testpkg/AbstractServerTest.scala @@ -39,6 +39,7 @@ final class SbtServer( case _ => } } + def isAlive: Boolean = process.isAlive() } trait AbstractServerTest extends AnyFunSuite with BeforeAndAfterAll { @@ -139,6 +140,12 @@ trait AbstractServerTest extends AnyFunSuite with BeforeAndAfterAll { ) ) + protected def waitUntil(timeout: FiniteDuration)(p: => Boolean): Boolean = { + val deadline = timeout.fromNow + while (!p && deadline.hasTimeLeft()) Thread.sleep(100) + p + } + override protected def afterAll(): Unit = { svr.close() svr = null diff --git a/server-test/src/test/scala/testpkg/PortfileReloadTest.scala b/server-test/src/test/scala/testpkg/PortfileReloadTest.scala new file mode 100644 index 000000000..2eab36cf4 --- /dev/null +++ b/server-test/src/test/scala/testpkg/PortfileReloadTest.scala @@ -0,0 +1,31 @@ +/* + * 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 sbt.io.IO +import sbt.io.syntax.* + +import scala.concurrent.duration.* + +/** + * A project load closes the file tree repository and installs a new one, so a portfile watch that + * a load does not outlive stops working after the first reload. + */ +class PortfileReloadTest extends AbstractServerTest { + override val testDirectory: String = "client" + + private val settle = 30.seconds + + test("a portfile that names another server, after a reload") { + assert(runBatchClient("reload") == 0, "reload must succeed") + val portfile = svr.baseDirectory / "project" / "target" / "active.json" + IO.write(portfile, """{"uri":"local:///displaced","serverId":"another-server"}""") + assert(waitUntil(settle)(!svr.isAlive), "the displaced server exits") + } +} diff --git a/server-test/src/test/scala/testpkg/PortfileTest.scala b/server-test/src/test/scala/testpkg/PortfileTest.scala new file mode 100644 index 000000000..30fcabda9 --- /dev/null +++ b/server-test/src/test/scala/testpkg/PortfileTest.scala @@ -0,0 +1,38 @@ +/* + * 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 sbt.io.IO +import sbt.io.syntax.* + +import scala.concurrent.duration.* + +/** + * The portfile names the server that owns the socket. A second server binds the same socket path, + * which leaves the first server holding a socket that no client can reach. + */ +class PortfileTest extends AbstractServerTest { + override val testDirectory: String = "client" + + private val settle = 30.seconds + + private def portfile: File = svr.baseDirectory / "project" / "target" / "active.json" + + test("a portfile written by a server") { + val id = """"serverId":"([^"]+)"""".r.findFirstMatchIn(IO.read(portfile)).map(_.group(1)) + assert(id.exists(_.nonEmpty), s"the portfile names its writer: ${IO.read(portfile)}") + } + + test("a portfile that names another server") { + val replacement = """{"uri":"local:///displaced","serverId":"another-server"}""" + IO.write(portfile, replacement) + assert(waitUntil(settle)(!svr.isAlive), "the displaced server exits") + assert(IO.read(portfile) == replacement, "the displaced server does not delete the portfile") + } +}