Skip to content

Commit

Permalink
WIP fix runBackground with -i/--no-server (#4259)
Browse files Browse the repository at this point in the history
This might have been the cause of a lot of flakiness that seems to have
gone away with #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 com-lihaoyi/os-lib#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`
  • Loading branch information
lihaoyi authored Jan 7, 2025
1 parent 1adb97d commit 42b3353
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 1 deletion.
6 changes: 6 additions & 0 deletions integration/invalidation/run-background/resources/build.mill
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package build
import mill._, javalib._

object foo extends JavaModule{
def runBackgroundLogToConsole: Boolean = false
}
Original file line number Diff line number Diff line change
@@ -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!");
}
}
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
}
}
3 changes: 2 additions & 1 deletion main/util/src/mill/util/Jvm.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
}

Expand Down

0 comments on commit 42b3353

Please sign in to comment.