Fix prompt resizing when connecting to existing daemon (#5924)

Fixes #5919

This regressed in #5832, which
made us only start the `terminfo` writing thread when spawning a new
server, which neglects the case where we are connecting to an existing
server. Thus when you ran `./mill clean && ./mill __.compile`, the
`__.compile` runs with a launcher connected to the existing server, and
thus doesn't update when the terminal size changes

This PR makes us start the `terminfo` thread regardless of whether we
are spawning a new server or not. Tested manually

Also did some cleanup of `libs/daemon/client` to remove dead code.

Author: lihaoyi

libs/daemon/client/src/mill/client/LaunchedServer.java

@@ -3,8 +3,6 @@
 public abstract class LaunchedServer {
   public abstract boolean isAlive();
 
-  public abstract void kill();
-
   /// An operating system process was launched.
   public static class OsProcess extends LaunchedServer {
     public final ProcessHandle process;
@@ -22,13 +20,6 @@ public String toString() {
     public boolean isAlive() {
       return process.isAlive();
     }
-
-    @Override
-    public void kill() {
-      if (!process.destroy())
-        throw new IllegalStateException(
-            "Asked " + process + " to terminate, but the termination request failed.");
-    }
   }
 
   /// Code running in the same process.
@@ -50,10 +41,5 @@ public String toString() {
     public boolean isAlive() {
       return thread.isAlive();
     }
-
-    @Override
-    public void kill() {
-      _kill.run();
-    }
   }
 }

libs/daemon/client/src/mill/client/ServerLauncher.java

@@ -37,16 +37,6 @@
 ///   - Wait for `ProxyStream.END` packet or `clientSocket.close()`,
 ///     indicating server has finished execution and all data has been received
 public abstract class ServerLauncher {
-  public static class RunWithConnectionResult<A> {
-    public final A result;
-
-    public final int exitCode;
-
-    public RunWithConnectionResult(A result, int exitCode) {
-      this.result = result;
-      this.exitCode = exitCode;
-    }
-  }
 
   /// Run a client logic with a connection established to a Mill server (via [#connectToServer]).
   ///
@@ -56,13 +46,13 @@ public RunWithConnectionResult(A result, int exitCode) {
   // client logic
   /// @param runClientLogic the client logic to run
   /// @return the exit code that the server sent back
-  public static <A> RunWithConnectionResult<A> runWithConnection(
-      String debugName,
+  public static int runWithConnection(
       Socket connection,
       Streams streams,
       boolean closeConnectionAfterClientLogic,
       Consumer<OutputStream> sendInitData,
-      RunClientLogic<A> runClientLogic)
+      Runnable runClientLogic,
+      String debugName)
       throws Exception {
     // According to
     // https://pzemtsov.github.io/2015/01/19/on-the-benefits-of-stream-buffering-in-Java.html it
@@ -76,10 +66,10 @@ public static <A> RunWithConnectionResult<A> runWithConnection(
     socketOutputStream.flush();
     var pumperThread =
         startStreamPumpers(socketInputStream, socketOutputStream, streams, debugName);
-    var result = runClientLogic.run();
+    runClientLogic.run();
     if (closeConnectionAfterClientLogic) socketInputStream.close();
     pumperThread.join();
-    return new RunWithConnectionResult<>(result, pumperThread.exitCode());
+    return pumperThread.exitCode();
   }
 
   /// Run a client logic with a connection established to a Mill server (via [#connectToServer]).
@@ -90,7 +80,6 @@ public static <A> RunWithConnectionResult<A> runWithConnection(
   /// @param runClientLogic the client logic to run
   /// @return the exit code that the server sent back
   public static <A> A runWithConnection(
-      String debugName,
       Socket connection,
       boolean closeConnectionAfterClientLogic,
       Consumer<OutputStream> sendInitData,
@@ -266,11 +255,6 @@ public static ServerLaunchResult ensureServerIsRunning(
                       public boolean isAlive() {
                         throw new RuntimeException("not implemented, this should never happen");
                       }
-
-                      @Override
-                      public void kill() {
-                        throw new RuntimeException("not implemented, this should never happen");
-                      }
                     };
             return Optional.of(new ServerLaunchResult.AlreadyRunning(launchedServer));
           } catch (IOException | NumberFormatException e) {
@@ -353,26 +337,11 @@ public interface InitServer {
     LaunchedServer init() throws Exception;
   }
 
-  public interface RunClientLogic<A> {
-    /// Runs the client logic.
-    A run() throws Exception;
-  }
-
   public interface RunClientLogicWithStreams<A> {
     /// Runs the client logic.
     A run(BufferedInputStream inStream, BufferedOutputStream outStream) throws Exception;
   }
 
-  public static class Result {
-    public final int exitCode;
-    public final Path daemonDir;
-
-    public Result(int exitCode, Path daemonDir) {
-      this.exitCode = exitCode;
-      this.daemonDir = daemonDir;
-    }
-  }
-
   public static class Streams {
     /// The input stream to send to the server as the stdin.
     public final InputStream stdin;

libs/javalib/worker/src/mill/javalib/worker/JvmWorkerImpl.scala

@@ -340,7 +340,6 @@ class JvmWorkerImpl(args: JvmWorkerArgs) extends JvmWorkerApi with AutoCloseable
           val debugName =
             s"ZincWorker,TCP ${socket.getRemoteSocketAddress} -> ${socket.getLocalSocketAddress}"
           ServerLauncher.runWithConnection(
-            debugName,
             socket,
             /* closeConnectionAfterCommand */ true,
             /* sendInitData */ _ => {},

runner/client/src/mill/client/MillServerLauncher.java

@@ -43,7 +43,7 @@ public MillServerLauncher(
    */
   public abstract LaunchedServer initServer(Path daemonDir, Locks locks) throws Exception;
 
-  public Result run(Path daemonDir, String javaHome, Consumer<String> log) throws Exception {
+  public int run(Path daemonDir, String javaHome, Consumer<String> log) throws Exception {
     Files.createDirectories(daemonDir);
 
     var initData = new ClientInitData(
@@ -76,8 +76,6 @@ public Result run(Path daemonDir, String javaHome, Consumer<String> log) throws
         true /*openSocket*/)) {
       log.accept("runWithConnection: " + launched);
       var result = runWithConnection(
-          "MillServerLauncher[" + launched.socket.getLocalSocketAddress() + " -> "
-              + launched.socket.getRemoteSocketAddress() + "]",
           launched.socket,
           streams,
           false,
@@ -93,19 +91,24 @@ public Result run(Path daemonDir, String javaHome, Consumer<String> log) throws
           () -> {
             log.accept("running client logic");
             forceTestFailure(daemonDir, log);
-            return 0;
-          });
-      log.accept("runWithConnection exit code: " + result.exitCode);
-      return new Result(result.exitCode, daemonDir);
+          },
+          "MillServerLauncher[" + launched.socket.getLocalSocketAddress() + " -> "
+              + launched.socket.getRemoteSocketAddress() + "]");
+      log.accept("runWithConnection exit code: " + result);
+      return result;
     }
   }
 
-  private void forceTestFailure(Path daemonDir, Consumer<String> log) throws Exception {
+  private void forceTestFailure(Path daemonDir, Consumer<String> log) {
     if (forceFailureForTestingMillisDelay > 0) {
       log.accept(
           "Force failure for testing in " + forceFailureForTestingMillisDelay + "ms: " + daemonDir);
-      Thread.sleep(forceFailureForTestingMillisDelay);
-      throw new Exception("Force failure for testing: " + daemonDir);
+      try {
+        Thread.sleep(forceFailureForTestingMillisDelay);
+      } catch (InterruptedException e) {
+        throw new RuntimeException(e);
+      }
+      throw new RuntimeException("Force failure for testing: " + daemonDir);
     } else {
       log.accept("No force failure for testing: " + daemonDir);
     }

runner/launcher/src/mill/launcher/MillLauncherMain.java

@@ -48,12 +48,9 @@ public static void main(String[] args) throws Exception {
     // Ensure that if we're running in BSP mode we don't start a daemon.
     //
     // This is needed because when Metals/Idea closes, they only kill the BSP client and the BSP
-    // server lurks around
-    // waiting for the next client to connect.
-    //
+    // server lurks around waiting for the next client to connect.
     // This is unintuitive from the user's perspective and wastes resources, as most people expect
-    // everything related
-    // to the BSP server to be killed when closing the editor.
+    // everything related to the BSP server to be killed when closing the editor.
     if (bspMode) runNoDaemon = true;
 
     var outMode = bspMode ? OutFolderMode.BSP : OutFolderMode.REGULAR;
@@ -116,12 +113,13 @@ public LaunchedServer initServer(Path daemonDir, Locks locks) throws Exception {
               }
             };
 
-        var daemonDir0 = Paths.get(outDir, OutFiles.millDaemon);
+        var daemonDir = Paths.get(outDir, OutFiles.millDaemon);
         String javaHome = MillProcessLauncher.javaHome(outMode);
 
-        var exitCode = launcher.run(daemonDir0, javaHome, log).exitCode;
+        MillProcessLauncher.prepareMillRunFolder(daemonDir);
+        var exitCode = launcher.run(daemonDir, javaHome, log);
         if (exitCode == ClientUtil.ExitServerCodeWhenVersionMismatch()) {
-          exitCode = launcher.run(daemonDir0, javaHome, log).exitCode;
+          exitCode = launcher.run(daemonDir, javaHome, log);
         }
         System.exit(exitCode);
       } catch (Exception e) {

runner/launcher/src/mill/launcher/MillProcessLauncher.java

@@ -26,6 +26,8 @@ static int launchMillNoDaemon(
     final Path processDir =
         Paths.get(".").resolve(outFor(outMode)).resolve(millNoDaemon).resolve(sig);
 
+    MillProcessLauncher.prepareMillRunFolder(processDir);
+
     final List<String> l = new ArrayList<>();
     l.addAll(millLaunchJvmCommand(outMode, runnerClasspath));
     Map<String, String> propsMap = ClientUtil.getUserSetProperties();
@@ -77,7 +79,7 @@ static Process configureRunMillProcess(ProcessBuilder builder, Path daemonDir) t
 
     Path sandbox = daemonDir.resolve(DaemonFiles.sandbox);
     Files.createDirectories(sandbox);
-    MillProcessLauncher.prepareMillRunFolder(daemonDir);
+
     builder.environment().put(EnvVars.MILL_WORKSPACE_ROOT, new File("").getCanonicalPath());
     if (System.getenv(EnvVars.MILL_EXECUTABLE_PATH) == null)
       builder.environment().put(EnvVars.MILL_EXECUTABLE_PATH, getExecutablePath());

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

@@ -99,6 +99,7 @@ object ClientServerTests extends TestSuite {
       val in = new ByteArrayInputStream(s"hello$ENDL".getBytes())
       val out = new ByteArrayOutputStream()
       val err = new ByteArrayOutputStream()
+      val daemonDir = outDir / "server-0"
       val result = new MillServerLauncher(
         ServerLauncher.Streams(in, out, err),
         env.asJava,
@@ -121,14 +122,14 @@ object ClientServerTests extends TestSuite {
           LaunchedServer.NewThread(t, () => { /* do nothing */ })
         }
       }.run(
-        (outDir / "server-0").relativeTo(os.pwd).toNIO,
+        daemonDir.relativeTo(os.pwd).toNIO,
         "",
         msg => println(s"MillServerLauncher: $msg")
       )
 
       ClientResult(
-        result.exitCode,
-        os.Path(result.daemonDir, os.pwd),
+        result,
+        daemonDir,
         outDir,
         out.toString,
         err.toString