From 42b33539c4bbc3b278302a215cfff9a28981cc0d Mon Sep 17 00:00:00 2001
From: Li Haoyi <haoyi.sg@gmail.com>
Date: Tue, 7 Jan 2025 10:14:09 +0800
Subject: [PATCH] WIP fix `runBackground` with `-i`/`--no-server`  (#4259)

This might have been the cause of a lot of flakiness that seems to have
gone away with https://github.com/com-lihaoyi/mill/pull/4254, as the
server exiting caused the `runBackground` calls to exit causing the http
servers to exit and fail to pick up requests.

Might have been caused by https://github.com/com-lihaoyi/os-lib/pull/324
which made `destroyOnExit` the default for spawned subprocesses. This PR
explicitly disables `destroyOnExit` for the subprocesses where
`background = true`

Covered by a new `integration.invalidation` test that runs under both
`server` and `fork`, that previously failed when run under `fork`
---
 .../run-background/resources/build.mill       |  6 ++
 .../resources/foo/src/foo/Foo.java            | 20 +++++++
 .../src/RunBackgroundTests.scala              | 58 +++++++++++++++++++
 main/util/src/mill/util/Jvm.scala             |  3 +-
 4 files changed, 86 insertions(+), 1 deletion(-)
 create mode 100644 integration/invalidation/run-background/resources/build.mill
 create mode 100644 integration/invalidation/run-background/resources/foo/src/foo/Foo.java
 create mode 100644 integration/invalidation/run-background/src/RunBackgroundTests.scala

diff --git a/integration/invalidation/run-background/resources/build.mill b/integration/invalidation/run-background/resources/build.mill
new file mode 100644
index 00000000000..bcda4c33106
--- /dev/null
+++ b/integration/invalidation/run-background/resources/build.mill
@@ -0,0 +1,6 @@
+package build
+import mill._, javalib._
+
+object foo extends JavaModule{
+  def runBackgroundLogToConsole: Boolean = false
+}
\ No newline at end of file
diff --git a/integration/invalidation/run-background/resources/foo/src/foo/Foo.java b/integration/invalidation/run-background/resources/foo/src/foo/Foo.java
new file mode 100644
index 00000000000..4b6e49b0d35
--- /dev/null
+++ b/integration/invalidation/run-background/resources/foo/src/foo/Foo.java
@@ -0,0 +1,20 @@
+package foo;
+import java.io.RandomAccessFile;
+import java.nio.channels.FileChannel;
+import java.nio.file.*;
+public class Foo {
+    public static void main(String[] args) throws Exception{
+        System.out.println("Hello World A!");
+        RandomAccessFile raf = new RandomAccessFile(args[0], "rw");
+        System.out.println("Hello World B!");
+        FileChannel chan = raf.getChannel();
+        System.out.println("Hello World C!");
+        chan.lock();
+        System.out.println("Hello World D!");
+        while(true){
+            if (!Files.exists(Paths.get(args[1]))) Thread.sleep(1);
+            else break;
+        }
+        System.out.println("Hello World E!");
+    }
+}
diff --git a/integration/invalidation/run-background/src/RunBackgroundTests.scala b/integration/invalidation/run-background/src/RunBackgroundTests.scala
new file mode 100644
index 00000000000..090e3158592
--- /dev/null
+++ b/integration/invalidation/run-background/src/RunBackgroundTests.scala
@@ -0,0 +1,58 @@
+package mill.integration
+
+import mill.testkit.UtestIntegrationTestSuite
+import java.io.RandomAccessFile
+import utest.asserts.{RetryInterval, RetryMax}
+import scala.concurrent.duration._
+import utest._
+
+// Make sure that `runBackground` subprocesses properly outlive the execution
+// that created them, and outlive the Mill process whether terminated naturally
+// at the end of `--no-server` or terminated explicitly via `shutdown`
+object RunBackgroundTests extends UtestIntegrationTestSuite {
+  implicit val retryMax: RetryMax = RetryMax(5000.millis)
+  implicit val retryInterval: RetryInterval = RetryInterval(50.millis)
+
+  def probeLockAvailable(lock: os.Path): Boolean = {
+    val raf = new RandomAccessFile(lock.toIO, "rw");
+    val chan = raf.getChannel();
+    chan.tryLock() match {
+      case null => false
+      case locked =>
+        locked.release()
+        true
+    }
+  }
+
+  val tests: Tests = Tests {
+    test("simple") - integrationTest { tester =>
+      import tester._
+      val lock = os.temp()
+      val stop = os.temp()
+      os.remove(stop)
+      eval(("foo.runBackground", lock, stop))
+      eventually { !probeLockAvailable(lock) }
+      if (tester.clientServerMode) eval("shutdown")
+      continually { !probeLockAvailable(lock) }
+      os.write(stop, "")
+      eventually { probeLockAvailable(lock) }
+    }
+    test("clean") - integrationTest { tester =>
+      if (!mill.main.client.Util.isWindows) {
+        import tester._
+        val lock = os.temp()
+        val stop = os.temp()
+        os.remove(stop)
+        eval(("foo.runBackground", lock, stop))
+        eventually {
+          !probeLockAvailable(lock)
+        }
+
+        eval(("clean", "foo.runBackground"))
+        eventually {
+          probeLockAvailable(lock)
+        }
+      }
+    }
+  }
+}
diff --git a/main/util/src/mill/util/Jvm.scala b/main/util/src/mill/util/Jvm.scala
index df656a7cebd..64098054d8b 100644
--- a/main/util/src/mill/util/Jvm.scala
+++ b/main/util/src/mill/util/Jvm.scala
@@ -395,7 +395,8 @@ object Jvm extends CoursierSupport {
       env = envArgs,
       stdin = if (backgroundOutputs.isEmpty) os.Inherit else "",
       stdout = backgroundOutputs.map(_._1).getOrElse(os.Inherit),
-      stderr = backgroundOutputs.map(_._2).getOrElse(os.Inherit)
+      stderr = backgroundOutputs.map(_._2).getOrElse(os.Inherit),
+      destroyOnExit = backgroundOutputs.isEmpty
     )
   }