Improve serverPort handling in client server protocol

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)
 

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 {

main/client/src/mill/main/client/ServerLauncher.java

@@ -116,19 +116,22 @@ public Result acquireLocksAndRun(String outDir) throws Exception {
   }
 
   int run(Path serverDir, boolean setJnaNoSys, Locks locks) throws Exception {
-    String serverPath = serverDir + "/" + ServerFiles.runArgs;
-    try (OutputStream f = Files.newOutputStream(Paths.get(serverPath))) {
+    // 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.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);
       Util.writeArgs(args, f);
       Util.writeMap(env, f);
     }
 
-    if (locks.processLock.probe()) {
-      initServer(serverDir, setJnaNoSys, locks);
-    }
+    if (locks.processLock.probe()) 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;
@@ -139,7 +142,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);
       }
     }
 
@@ -165,10 +168,13 @@ int run(Path serverDir, boolean setJnaNoSys, Locks locks) throws Exception {
     outPumperThread.join();
 
     try {
-      return Integer.parseInt(
-          Files.readAllLines(Paths.get(serverDir + "/" + ServerFiles.exitCode)).get(0));
-    } catch (Throwable e) {
-      return Util.ExitClientCodeCannotReadFromExitCodeFile();
+      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();
     }

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);
   }
 
   /**

main/server/src/mill/main/server/Server.scala

@@ -30,7 +30,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)) {
+    if (os.exists(serverDir) || testLogEvenWhenServerIdWrong) {
       os.write.append(serverDir / ServerFiles.serverLog, s"$s\n", createFolders = true)
     }
   }
@@ -41,31 +41,34 @@ 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 && {
-            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
-                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
+            }
           }
         ) ()
-
+        serverLog("server loop ended")
       }.getOrElse(throw new Exception("Mill server process already present"))
-    finally exitServer()
+    } finally {
+      serverLog("finally exitServer")
+      exitServer()
+    }
   }
 
   def proxyInputStreamThroughPumper(in: InputStream): PipedInputStream = {
@@ -196,19 +199,21 @@ 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
           },
         "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")
@@ -259,8 +264,12 @@ 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
+        }
     }
   }
 }

main/server/test/src/mill/main/server/ClientServerTests.scala

@@ -175,7 +175,7 @@ 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) {
@@ -241,9 +241,12 @@ object ClientServerTests extends TestSuite {
       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"
           )
       )

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);
       }
   }
 }