From 1937acd6e0f5b0d946c2c100a52dad4227e4d8c8 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 24 Nov 2024 18:25:27 +0800 Subject: [PATCH 01/13] wip --- main/client/src/mill/main/client/ServerLauncher.java | 4 +++- main/server/src/mill/main/server/Server.scala | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/main/client/src/mill/main/client/ServerLauncher.java b/main/client/src/mill/main/client/ServerLauncher.java index 64cb9b75404..1b6e50f386c 100644 --- a/main/client/src/mill/main/client/ServerLauncher.java +++ b/main/client/src/mill/main/client/ServerLauncher.java @@ -116,6 +116,8 @@ public Result acquireLocksAndRun(String outDir) throws Exception { } int run(Path serverDir, boolean setJnaNoSys, Locks locks) throws Exception { + Files.deleteIfExists(serverDir.resolve(ServerFiles.exitCode)); + Files.deleteIfExists(serverDir.resolve(ServerFiles.socketPort)); String serverPath = serverDir + "/" + ServerFiles.runArgs; try (OutputStream f = Files.newOutputStream(Paths.get(serverPath))) { f.write(System.console() != null ? 1 : 0); @@ -166,7 +168,7 @@ int run(Path serverDir, boolean setJnaNoSys, Locks locks) throws Exception { try { return Integer.parseInt( - Files.readAllLines(Paths.get(serverDir + "/" + ServerFiles.exitCode)).get(0)); + Files.readAllLines(serverDir.resolve(ServerFiles.exitCode)).get(0)); } catch (Throwable e) { return Util.ExitClientCodeCannotReadFromExitCodeFile(); } finally { diff --git a/main/server/src/mill/main/server/Server.scala b/main/server/src/mill/main/server/Server.scala index 08700759c5d..a3101e7e279 100644 --- a/main/server/src/mill/main/server/Server.scala +++ b/main/server/src/mill/main/server/Server.scala @@ -196,8 +196,9 @@ abstract class Server[T]( ) stateCache = newStateCache - serverLog("exitCode " + ServerFiles.exitCode) - os.write.over(serverDir / ServerFiles.exitCode, if (result) "0" else "1") + val exitCode = if (result) "0" else "1" + serverLog("exitCode " + exitCode) + os.write.over(serverDir / ServerFiles.exitCode, exitCode) } finally { done = true idle = true From a681dd0b518797e277a59bb58158ed3c35ee7a99 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 24 Nov 2024 18:26:47 +0800 Subject: [PATCH 02/13] wip --- main/client/src/mill/main/client/ServerLauncher.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/main/client/src/mill/main/client/ServerLauncher.java b/main/client/src/mill/main/client/ServerLauncher.java index 1b6e50f386c..8ffd8c5662a 100644 --- a/main/client/src/mill/main/client/ServerLauncher.java +++ b/main/client/src/mill/main/client/ServerLauncher.java @@ -118,8 +118,7 @@ public Result acquireLocksAndRun(String outDir) throws Exception { int run(Path serverDir, boolean setJnaNoSys, Locks locks) throws Exception { Files.deleteIfExists(serverDir.resolve(ServerFiles.exitCode)); Files.deleteIfExists(serverDir.resolve(ServerFiles.socketPort)); - String serverPath = serverDir + "/" + ServerFiles.runArgs; - try (OutputStream f = Files.newOutputStream(Paths.get(serverPath))) { + try (OutputStream f = Files.newOutputStream(serverDir.resolve(ServerFiles.runArgs))) { f.write(System.console() != null ? 1 : 0); Util.writeString(f, BuildInfo.millVersion); Util.writeArgs(args, f); From 3f279990a3acddc8ddc39e7289ad8b9150f6e959 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 24 Nov 2024 19:00:50 +0800 Subject: [PATCH 03/13] wip --- main/client/src/mill/main/client/ServerLauncher.java | 12 ++++++++---- main/server/src/mill/main/server/Server.scala | 5 ++--- .../src/mill/runner/client/MillClientMain.java | 4 ++++ 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/main/client/src/mill/main/client/ServerLauncher.java b/main/client/src/mill/main/client/ServerLauncher.java index 8ffd8c5662a..c506fc81862 100644 --- a/main/client/src/mill/main/client/ServerLauncher.java +++ b/main/client/src/mill/main/client/ServerLauncher.java @@ -116,8 +116,14 @@ public Result acquireLocksAndRun(String outDir) throws Exception { } int run(Path serverDir, boolean setJnaNoSys, Locks locks) throws Exception { + // Clear out run-related files from the server folder to make sure we + // never hit issues where we are reading the files from a previous run Files.deleteIfExists(serverDir.resolve(ServerFiles.exitCode)); - Files.deleteIfExists(serverDir.resolve(ServerFiles.socketPort)); + Files.deleteIfExists(serverDir.resolve(ServerFiles.stdout)); + Files.deleteIfExists(serverDir.resolve(ServerFiles.stderr)); + Files.deleteIfExists(serverDir.resolve(ServerFiles.terminfo)); + Files.deleteIfExists(serverDir.resolve(ServerFiles.runArgs)); + try (OutputStream f = Files.newOutputStream(serverDir.resolve(ServerFiles.runArgs))) { f.write(System.console() != null ? 1 : 0); Util.writeString(f, BuildInfo.millVersion); @@ -140,7 +146,7 @@ int run(Path serverDir, boolean setJnaNoSys, Locks locks) throws Exception { ioSocket = new java.net.Socket("127.0.0.1", port); } catch (Throwable e) { socketThrowable = e; - Thread.sleep(10); + Thread.sleep(1); } } @@ -168,8 +174,6 @@ int run(Path serverDir, boolean setJnaNoSys, Locks locks) throws Exception { try { return Integer.parseInt( Files.readAllLines(serverDir.resolve(ServerFiles.exitCode)).get(0)); - } catch (Throwable e) { - return Util.ExitClientCodeCannotReadFromExitCodeFile(); } finally { ioSocket.close(); } diff --git a/main/server/src/mill/main/server/Server.scala b/main/server/src/mill/main/server/Server.scala index a3101e7e279..76c83edeb39 100644 --- a/main/server/src/mill/main/server/Server.scala +++ b/main/server/src/mill/main/server/Server.scala @@ -43,11 +43,10 @@ abstract class Server[T]( try Server.tryLockBlock(locks.processLock) { watchServerIdFile() - + val serverSocket = new java.net.ServerSocket(0, 0, InetAddress.getByName(null)) + os.write.over(serverDir / ServerFiles.socketPort, serverSocket.getLocalPort.toString) while ( running && { - val serverSocket = new java.net.ServerSocket(0, 0, InetAddress.getByName(null)) - os.write.over(serverDir / ServerFiles.socketPort, serverSocket.getLocalPort.toString) try interruptWithTimeout(() => serverSocket.close(), () => serverSocket.accept()) match { case None => false diff --git a/runner/client/src/mill/runner/client/MillClientMain.java b/runner/client/src/mill/runner/client/MillClientMain.java index 471b74713f2..aecee51404e 100644 --- a/runner/client/src/mill/runner/client/MillClientMain.java +++ b/runner/client/src/mill/runner/client/MillClientMain.java @@ -79,6 +79,10 @@ public void preRun(Path serverDir) throws Exception { "Loading Mill in-process isn't possible.\n" + "Please check your Mill installation!"); throw e; } + } catch (Exception e) { + System.err.println("Mill client failed with unknown exception"); + e.printStackTrace(); + System.exit(1); } } } From 1d0962c80d7aeb9a418a1c50168390b254010d36 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 24 Nov 2024 19:21:56 +0800 Subject: [PATCH 04/13] . --- main/client/src/mill/main/client/Util.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/main/client/src/mill/main/client/Util.java b/main/client/src/mill/main/client/Util.java index d6e88c6ad8e..fd6ececad09 100644 --- a/main/client/src/mill/main/client/Util.java +++ b/main/client/src/mill/main/client/Util.java @@ -10,7 +10,6 @@ import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; -import java.util.Base64; import java.util.HashMap; import java.util.LinkedList; import java.util.List; @@ -131,7 +130,7 @@ static String sha1Hash(String path) throws NoSuchAlgorithmException { byte[] pathBytes = path.getBytes(StandardCharsets.UTF_8); md.update(pathBytes); byte[] digest = md.digest(); - return Base64.getEncoder().encodeToString(digest); + return hexArray(digest); } /** From 5df256f764e4d498e36a4bb9da3cab4db0ea90b9 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 24 Nov 2024 19:29:13 +0800 Subject: [PATCH 05/13] . --- .../src/mill/main/client/ServerLauncher.java | 2 -- main/server/src/mill/main/server/Server.scala | 24 +++++++++---------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/main/client/src/mill/main/client/ServerLauncher.java b/main/client/src/mill/main/client/ServerLauncher.java index c506fc81862..4fddca0ec72 100644 --- a/main/client/src/mill/main/client/ServerLauncher.java +++ b/main/client/src/mill/main/client/ServerLauncher.java @@ -119,8 +119,6 @@ int run(Path serverDir, boolean setJnaNoSys, Locks locks) throws Exception { // Clear out run-related files from the server folder to make sure we // never hit issues where we are reading the files from a previous run Files.deleteIfExists(serverDir.resolve(ServerFiles.exitCode)); - Files.deleteIfExists(serverDir.resolve(ServerFiles.stdout)); - Files.deleteIfExists(serverDir.resolve(ServerFiles.stderr)); Files.deleteIfExists(serverDir.resolve(ServerFiles.terminfo)); Files.deleteIfExists(serverDir.resolve(ServerFiles.runArgs)); diff --git a/main/server/src/mill/main/server/Server.scala b/main/server/src/mill/main/server/Server.scala index 76c83edeb39..61697dabfdb 100644 --- a/main/server/src/mill/main/server/Server.scala +++ b/main/server/src/mill/main/server/Server.scala @@ -47,19 +47,17 @@ abstract class Server[T]( os.write.over(serverDir / ServerFiles.socketPort, serverSocket.getLocalPort.toString) while ( running && { - try - interruptWithTimeout(() => serverSocket.close(), () => serverSocket.accept()) match { - case None => false - case Some(sock) => - serverLog("handling run") - try handleRun(sock, initialSystemProperties) - catch { - case e: Throwable => - serverLog(e.toString + "\n" + e.getStackTrace.mkString("\n")) - } finally sock.close(); - true - } - finally serverSocket.close() + interruptWithTimeout(() => serverSocket.close(), () => serverSocket.accept()) match { + case None => false + case Some(sock) => + serverLog("handling run") + try handleRun(sock, initialSystemProperties) + catch { + case e: Throwable => + serverLog(e.toString + "\n" + e.getStackTrace.mkString("\n")) + } finally sock.close(); + true + } } ) () From ea2e688af5d96cfbac745c2be452b61379652e68 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 24 Nov 2024 20:02:47 +0800 Subject: [PATCH 06/13] wip --- main/server/src/mill/main/server/Server.scala | 17 ++++++++++------- .../mill/main/server/ClientServerTests.scala | 16 +++++++--------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/main/server/src/mill/main/server/Server.scala b/main/server/src/mill/main/server/Server.scala index 61697dabfdb..f57733fc797 100644 --- a/main/server/src/mill/main/server/Server.scala +++ b/main/server/src/mill/main/server/Server.scala @@ -20,7 +20,6 @@ abstract class Server[T]( serverDir: os.Path, acceptTimeoutMillis: Int, locks: Locks, - testLogEvenWhenServerIdWrong: Boolean = false ) { @volatile var running = true @@ -30,9 +29,7 @@ abstract class Server[T]( val serverId: String = java.lang.Long.toHexString(scala.util.Random.nextLong()) def serverLog0(s: String): Unit = { - if (running && (testLogEvenWhenServerIdWrong || checkServerIdFile().isEmpty)) { - os.write.append(serverDir / ServerFiles.serverLog, s"$s\n", createFolders = true) - } + os.write.append(serverDir / ServerFiles.serverLog, s"$s\n", createFolders = true) } def serverLog(s: String): Unit = serverLog0(s"$serverId $s") @@ -41,10 +38,13 @@ abstract class Server[T]( serverLog("running server in " + serverDir) val initialSystemProperties = sys.props.toMap - try Server.tryLockBlock(locks.processLock) { + try { + Server.tryLockBlock(locks.processLock) { + serverLog("server file locked") watchServerIdFile() val serverSocket = new java.net.ServerSocket(0, 0, InetAddress.getByName(null)) os.write.over(serverDir / ServerFiles.socketPort, serverSocket.getLocalPort.toString) + serverLog("listening on port " + serverSocket.getLocalPort) while ( running && { interruptWithTimeout(() => serverSocket.close(), () => serverSocket.accept()) match { @@ -60,9 +60,12 @@ abstract class Server[T]( } } ) () - + serverLog("server loop ended") }.getOrElse(throw new Exception("Mill server process already present")) - finally exitServer() + } finally { + serverLog("exitServer") + exitServer() + } } def proxyInputStreamThroughPumper(in: InputStream): PipedInputStream = { diff --git a/main/server/test/src/mill/main/server/ClientServerTests.scala b/main/server/test/src/mill/main/server/ClientServerTests.scala index e240e90ef78..62e6805a074 100644 --- a/main/server/test/src/mill/main/server/ClientServerTests.scala +++ b/main/server/test/src/mill/main/server/ClientServerTests.scala @@ -22,8 +22,7 @@ object ClientServerTests extends TestSuite { override val serverId: String, serverDir: os.Path, locks: Locks, - testLogEvenWhenServerIdWrong: Boolean - ) extends Server[Option[Int]](serverDir, 1000, locks, testLogEvenWhenServerIdWrong) + ) extends Server[Option[Int]](serverDir, 1000, locks) with Runnable { override def exitServer() = { serverLog("exiting server") @@ -69,7 +68,7 @@ object ClientServerTests extends TestSuite { } } - class Tester(testLogEvenWhenServerIdWrong: Boolean) { + class Tester() { var nextServerId: Int = 0 val terminatedServers = collection.mutable.Set.empty[String] @@ -104,7 +103,6 @@ object ClientServerTests extends TestSuite { serverId, os.Path(serverDir, os.pwd), locks, - testLogEvenWhenServerIdWrong )).start() } }.acquireLocksAndRun(outDir.relativeTo(os.pwd).toString) @@ -139,7 +137,7 @@ object ClientServerTests extends TestSuite { test("hello") - retry(3) { // Continue logging when out folder is deleted so we can see the logs // and ensure the correct code path is taken as the server exits - val tester = new Tester(testLogEvenWhenServerIdWrong = true) + val tester = new Tester() val res1 = tester(args = Array("world")) assert( @@ -179,7 +177,7 @@ object ClientServerTests extends TestSuite { } } test("dontLogWhenOutFolderDeleted") - retry(3) { - val tester = new Tester(testLogEvenWhenServerIdWrong = false) + val tester = new Tester() val res1 = tester(args = Array("world")) assert( @@ -199,7 +197,7 @@ object ClientServerTests extends TestSuite { } test("concurrency") { - val tester = new Tester(testLogEvenWhenServerIdWrong = false) + val tester = new Tester() // Make sure concurrently running client commands results in multiple processes // being spawned, running in different folders import concurrent._ @@ -224,7 +222,7 @@ object ClientServerTests extends TestSuite { } test("clientLockReleasedOnFailure") { - val tester = new Tester(testLogEvenWhenServerIdWrong = false) + val tester = new Tester() // When the client gets interrupted via Ctrl-C, we exit the server immediately. This // is because Mill ends up executing arbitrary JVM code, and there is no generic way // to interrupt such an execution. The two options are to leave the server running @@ -250,7 +248,7 @@ object ClientServerTests extends TestSuite { } test("envVars") - retry(3) { - val tester = new Tester(testLogEvenWhenServerIdWrong = false) + val tester = new Tester() // Make sure the simple "have the client start a server and // exchange one message" workflow works from end to end. From f0f4a191d4a18f467d18be927a080cbaeccde9e6 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 24 Nov 2024 20:12:31 +0800 Subject: [PATCH 07/13] wip --- main/client/src/mill/main/client/ServerLauncher.java | 6 +++--- main/server/src/mill/main/server/Server.scala | 11 ++++++++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/main/client/src/mill/main/client/ServerLauncher.java b/main/client/src/mill/main/client/ServerLauncher.java index 4fddca0ec72..2e68a95089c 100644 --- a/main/client/src/mill/main/client/ServerLauncher.java +++ b/main/client/src/mill/main/client/ServerLauncher.java @@ -129,9 +129,9 @@ int run(Path serverDir, boolean setJnaNoSys, Locks locks) throws Exception { Util.writeMap(env, f); } - if (locks.processLock.probe()) { - initServer(serverDir, setJnaNoSys, locks); - } + boolean lockProbe = locks.processLock.probe(); + System.out.println("lockProbe: " + lockProbe); + if (lockProbe) initServer(serverDir, setJnaNoSys, locks); while (locks.processLock.probe()) Thread.sleep(3); diff --git a/main/server/src/mill/main/server/Server.scala b/main/server/src/mill/main/server/Server.scala index f57733fc797..9ede39cba11 100644 --- a/main/server/src/mill/main/server/Server.scala +++ b/main/server/src/mill/main/server/Server.scala @@ -29,7 +29,9 @@ abstract class Server[T]( val serverId: String = java.lang.Long.toHexString(scala.util.Random.nextLong()) def serverLog0(s: String): Unit = { - os.write.append(serverDir / ServerFiles.serverLog, s"$s\n", createFolders = true) + if (os.exists(serverDir / ServerFiles.serverLog)){ + os.write.append(serverDir / ServerFiles.serverLog, s"$s\n", createFolders = true) + } } def serverLog(s: String): Unit = serverLog0(s"$serverId $s") @@ -260,8 +262,11 @@ object Server { lock.tryLock() match { case null => None case l => - try Some(t) - finally l.release() + if (l.isLocked) { + try Some(t) finally l.release() + } else { + None + } } } } From a7b73aaa871a11d32f65f40c8afcd082f9f68178 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 24 Nov 2024 20:50:34 +0800 Subject: [PATCH 08/13] wip --- main/client/src/mill/main/client/InputPumper.java | 2 +- main/client/src/mill/main/client/ServerLauncher.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/main/client/src/mill/main/client/InputPumper.java b/main/client/src/mill/main/client/InputPumper.java index 3cf432a65b3..ef3d8e7f6d7 100644 --- a/main/client/src/mill/main/client/InputPumper.java +++ b/main/client/src/mill/main/client/InputPumper.java @@ -38,7 +38,7 @@ public void run() { while (running) { if (!runningCheck.getAsBoolean()) { running = false; - } else if (checkAvailable && src.available() == 0) Thread.sleep(2); + } else if (checkAvailable && src.available() == 0) Thread.sleep(1); else { int n; try { diff --git a/main/client/src/mill/main/client/ServerLauncher.java b/main/client/src/mill/main/client/ServerLauncher.java index 2e68a95089c..2f2b03dc374 100644 --- a/main/client/src/mill/main/client/ServerLauncher.java +++ b/main/client/src/mill/main/client/ServerLauncher.java @@ -133,7 +133,7 @@ int run(Path serverDir, boolean setJnaNoSys, Locks locks) throws Exception { System.out.println("lockProbe: " + lockProbe); if (lockProbe) initServer(serverDir, setJnaNoSys, locks); - while (locks.processLock.probe()) Thread.sleep(3); + while (locks.processLock.probe()) Thread.sleep(1); long retryStart = System.currentTimeMillis(); Socket ioSocket = null; From 5cfd96657458bd170f53748e6e73ead9fbe6e112 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 24 Nov 2024 21:05:07 +0800 Subject: [PATCH 09/13] wip --- kotlinlib/src/mill/kotlinlib/detekt/DetektModule.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kotlinlib/src/mill/kotlinlib/detekt/DetektModule.scala b/kotlinlib/src/mill/kotlinlib/detekt/DetektModule.scala index b21812cbfc3..f7280d7ab69 100644 --- a/kotlinlib/src/mill/kotlinlib/detekt/DetektModule.scala +++ b/kotlinlib/src/mill/kotlinlib/detekt/DetektModule.scala @@ -21,7 +21,7 @@ trait DetektModule extends KotlinModule { private def detekt0() = T.task { - val args = detektOptions() ++ Seq("-i", PathRef(T.workspace).path.toString()) ++ + val args = detektOptions() ++ Seq("-i", T.workspace.toString()) ++ Seq("-c", detektConfig().path.toString()) T.log.info("running detekt ...") From 73c8b07fd40698c6cb8cdaa5281fc52af07f81ec Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sun, 24 Nov 2024 21:06:48 +0800 Subject: [PATCH 10/13] wip --- main/client/src/mill/main/client/ServerLauncher.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/main/client/src/mill/main/client/ServerLauncher.java b/main/client/src/mill/main/client/ServerLauncher.java index 2f2b03dc374..a4fc61fcd4f 100644 --- a/main/client/src/mill/main/client/ServerLauncher.java +++ b/main/client/src/mill/main/client/ServerLauncher.java @@ -129,9 +129,7 @@ int run(Path serverDir, boolean setJnaNoSys, Locks locks) throws Exception { Util.writeMap(env, f); } - boolean lockProbe = locks.processLock.probe(); - System.out.println("lockProbe: " + lockProbe); - if (lockProbe) initServer(serverDir, setJnaNoSys, locks); + if (locks.processLock.probe()) initServer(serverDir, setJnaNoSys, locks); while (locks.processLock.probe()) Thread.sleep(1); From e1002d28ce1a1b54c7a6270d1a8edde721164119 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Mon, 25 Nov 2024 08:03:23 +0800 Subject: [PATCH 11/13] wip --- main/server/src/mill/main/server/Server.scala | 11 +++++--- .../mill/main/server/ClientServerTests.scala | 25 +++++++++++-------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/main/server/src/mill/main/server/Server.scala b/main/server/src/mill/main/server/Server.scala index 9ede39cba11..bd1a59f1d6d 100644 --- a/main/server/src/mill/main/server/Server.scala +++ b/main/server/src/mill/main/server/Server.scala @@ -20,6 +20,7 @@ abstract class Server[T]( serverDir: os.Path, acceptTimeoutMillis: Int, locks: Locks, + testLogEvenWhenServerIdWrong: Boolean = false ) { @volatile var running = true @@ -29,7 +30,7 @@ abstract class Server[T]( val serverId: String = java.lang.Long.toHexString(scala.util.Random.nextLong()) def serverLog0(s: String): Unit = { - if (os.exists(serverDir / ServerFiles.serverLog)){ + if (os.exists(serverDir) || testLogEvenWhenServerIdWrong) { os.write.append(serverDir / ServerFiles.serverLog, s"$s\n", createFolders = true) } } @@ -65,7 +66,7 @@ abstract class Server[T]( serverLog("server loop ended") }.getOrElse(throw new Exception("Mill server process already present")) } finally { - serverLog("exitServer") + serverLog("finally exitServer") exitServer() } } @@ -208,10 +209,11 @@ abstract class Server[T]( "MillServerActionRunner" ) t.start() + // We cannot simply use Lock#await here, because the filesystem doesn't // realize the clientLock/serverLock are held by different threads in the // two processes and gives a spurious deadlock error - while (!done && !locks.clientLock.probe()) Thread.sleep(3) + while (!done && !locks.clientLock.probe()) Thread.sleep(1) if (!idle) { serverLog("client interrupted while server was executing command") @@ -263,7 +265,8 @@ object Server { case null => None case l => if (l.isLocked) { - try Some(t) finally l.release() + try Some(t) + finally l.release() } else { None } diff --git a/main/server/test/src/mill/main/server/ClientServerTests.scala b/main/server/test/src/mill/main/server/ClientServerTests.scala index 62e6805a074..7f4aea70c27 100644 --- a/main/server/test/src/mill/main/server/ClientServerTests.scala +++ b/main/server/test/src/mill/main/server/ClientServerTests.scala @@ -22,7 +22,8 @@ object ClientServerTests extends TestSuite { override val serverId: String, serverDir: os.Path, locks: Locks, - ) extends Server[Option[Int]](serverDir, 1000, locks) + testLogEvenWhenServerIdWrong: Boolean = false + ) extends Server[Option[Int]](serverDir, 1000, locks, testLogEvenWhenServerIdWrong) with Runnable { override def exitServer() = { serverLog("exiting server") @@ -68,7 +69,7 @@ object ClientServerTests extends TestSuite { } } - class Tester() { + class Tester(testLogEvenWhenServerIdWrong: Boolean) { var nextServerId: Int = 0 val terminatedServers = collection.mutable.Set.empty[String] @@ -103,6 +104,7 @@ object ClientServerTests extends TestSuite { serverId, os.Path(serverDir, os.pwd), locks, + testLogEvenWhenServerIdWrong )).start() } }.acquireLocksAndRun(outDir.relativeTo(os.pwd).toString) @@ -137,7 +139,7 @@ object ClientServerTests extends TestSuite { test("hello") - retry(3) { // Continue logging when out folder is deleted so we can see the logs // and ensure the correct code path is taken as the server exits - val tester = new Tester() + val tester = new Tester(testLogEvenWhenServerIdWrong = true) val res1 = tester(args = Array("world")) assert( @@ -173,11 +175,11 @@ object ClientServerTests extends TestSuite { Thread.sleep(500) assert(res3.logsFor("serverId file missing") == Seq("server-1")) - assert(res3.logsFor("exiting server") == Seq("server-1")) + assert(res3.logsFor("exiting server") == Seq("server-1", "server-1")) } } test("dontLogWhenOutFolderDeleted") - retry(3) { - val tester = new Tester() + val tester = new Tester(testLogEvenWhenServerIdWrong = false) val res1 = tester(args = Array("world")) assert( @@ -197,7 +199,7 @@ object ClientServerTests extends TestSuite { } test("concurrency") { - val tester = new Tester() + val tester = new Tester(testLogEvenWhenServerIdWrong = false) // Make sure concurrently running client commands results in multiple processes // being spawned, running in different folders import concurrent._ @@ -222,7 +224,7 @@ object ClientServerTests extends TestSuite { } test("clientLockReleasedOnFailure") { - val tester = new Tester() + val tester = new Tester(testLogEvenWhenServerIdWrong = false) // When the client gets interrupted via Ctrl-C, we exit the server immediately. This // is because Mill ends up executing arbitrary JVM code, and there is no generic way // to interrupt such an execution. The two options are to leave the server running @@ -235,20 +237,23 @@ object ClientServerTests extends TestSuite { } val s"Force failure for testing: $pathStr" = res1.getMessage - Thread.sleep(100) // give a moment for logs to all turn up on disk + Thread.sleep(1000) // give a moment for logs to all turn up on disk val logLines = os.read.lines(os.Path(pathStr, os.pwd) / "server.log") assert( - logLines.takeRight(2) == + logLines.takeRight(5) == Seq( "server-0 client interrupted while server was executing command", + "server-0 exiting server", + "server-0 server loop ended", + "server-0 finally exitServer", "server-0 exiting server" ) ) } test("envVars") - retry(3) { - val tester = new Tester() + val tester = new Tester(testLogEvenWhenServerIdWrong = false) // Make sure the simple "have the client start a server and // exchange one message" workflow works from end to end. From faa3b0e1bf4c274071d4919f843100f9bfd80fa9 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Mon, 25 Nov 2024 08:15:28 +0800 Subject: [PATCH 12/13] wip --- .../watch-source-input/src/WatchSourceInputTests.scala | 1 + main/client/src/mill/main/client/ServerLauncher.java | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/integration/invalidation/watch-source-input/src/WatchSourceInputTests.scala b/integration/invalidation/watch-source-input/src/WatchSourceInputTests.scala index 8cea62874ac..732d8b4efad 100644 --- a/integration/invalidation/watch-source-input/src/WatchSourceInputTests.scala +++ b/integration/invalidation/watch-source-input/src/WatchSourceInputTests.scala @@ -51,6 +51,7 @@ object WatchSourceInputTests extends UtestIntegrationTestSuite { .filter(!_.contains("Watching for changes")) .filter(!_.contains("[info] compiling")) .filter(!_.contains("[info] done compiling")) + .filter(!_.contains("mill-server/ exitCode file not found")) assert(out == expectedOut) diff --git a/main/client/src/mill/main/client/ServerLauncher.java b/main/client/src/mill/main/client/ServerLauncher.java index a4fc61fcd4f..f619ba7b9f3 100644 --- a/main/client/src/mill/main/client/ServerLauncher.java +++ b/main/client/src/mill/main/client/ServerLauncher.java @@ -168,8 +168,13 @@ int run(Path serverDir, boolean setJnaNoSys, Locks locks) throws Exception { outPumperThread.join(); try { - return Integer.parseInt( - Files.readAllLines(serverDir.resolve(ServerFiles.exitCode)).get(0)); + Path exitCodeFile = serverDir.resolve(ServerFiles.exitCode); + if (Files.exists(exitCodeFile)) { + return Integer.parseInt(Files.readAllLines(exitCodeFile).get(0)); + } else { + System.err.println("mill-server/ exitCode file not found"); + return 1; + } } finally { ioSocket.close(); } From 0f261ce72325a5463e08ccb0defda78bb7202b6c Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Mon, 25 Nov 2024 08:17:50 +0800 Subject: [PATCH 13/13] wip --- main/server/test/src/mill/main/server/ClientServerTests.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main/server/test/src/mill/main/server/ClientServerTests.scala b/main/server/test/src/mill/main/server/ClientServerTests.scala index 7f4aea70c27..44ae7490bd3 100644 --- a/main/server/test/src/mill/main/server/ClientServerTests.scala +++ b/main/server/test/src/mill/main/server/ClientServerTests.scala @@ -22,7 +22,7 @@ object ClientServerTests extends TestSuite { override val serverId: String, serverDir: os.Path, locks: Locks, - testLogEvenWhenServerIdWrong: Boolean = false + testLogEvenWhenServerIdWrong: Boolean ) extends Server[Option[Int]](serverDir, 1000, locks, testLogEvenWhenServerIdWrong) with Runnable { override def exitServer() = { @@ -237,7 +237,7 @@ object ClientServerTests extends TestSuite { } val s"Force failure for testing: $pathStr" = res1.getMessage - Thread.sleep(1000) // give a moment for logs to all turn up on disk + Thread.sleep(100) // give a moment for logs to all turn up on disk val logLines = os.read.lines(os.Path(pathStr, os.pwd) / "server.log") assert(