Fix race condition of timeout thread interrupt to stabilize multi level build tests (#4254)

The basic problem was that `thread.interrupt()` running before
`interrupt = false` meant there was a chance the thread's `if
(interrupt) {` conditional would be `true`, resulting in the timeout
thread timing out the open socket immediately, and the Mill server
process shutting down.

The solution is to move `interrupt = false` to before
`thread.interrupt()`

This should hopefully fix the flakiness in multi-level-build tests,
where the process shutting down would cause the next command to re-spawn
a new process, resulting in all classloaders to be re-spawned, violating
our assertions

Improved the test error checking so next time a similar thing happens,
we get more precise reporting "an unwanted process restart occurred",
rather than just "classloader invalidation was unexpected".

Tested manually with `while ./mill
'integration.invalidation[multi-level-editing].server.test'; do :;
done`. This seems to reproduce the problem on my laptop in a few tens of
minutes without this fix, after this fix I haven't managed to make it
appear

Author: lihaoyi

integration/invalidation/multi-level-editing/src/MultiLevelBuildTests.scala

@@ -15,6 +15,7 @@ import scala.util.matching.Regex
 // in all cases.
 trait MultiLevelBuildTests extends UtestIntegrationTestSuite {
   var savedClassLoaderIds: Seq[Option[Int]] = Nil
+  var savedServerId: String = ""
   val retryCount: Int = if (sys.env.contains("CI")) 2 else 0
   def runAssertSuccess(tester: IntegrationTester, expected: String): Unit = {
     val res = tester.eval("foo.run")
@@ -104,6 +105,19 @@ trait MultiLevelBuildTests extends UtestIntegrationTestSuite {
       tester: IntegrationTester,
       expectedChanged0: java.lang.Boolean*
   ): Unit = {
+
+    // Before checking classloaders, make sure we check to ensure server spawns and
+    // restarts behave as expected:
+    if (clientServerMode) {
+      // Only one server should be running at any point in time
+      val Seq(serverFolder) = os.list(tester.workspacePath / "out" / "mill-server")
+
+      // client-server mode should never restart in these tests and preserve the same process,
+      val currentServerId = os.read(serverFolder / "serverId")
+      assert(currentServerId == savedServerId || savedServerId == "")
+      savedServerId = currentServerId
+    }
+
     val currentClassLoaderIds =
       for ((frame, path) <- loadFrames(tester, expectedChanged0.length))
         yield frame.classLoaderIdentity
@@ -132,7 +146,7 @@ trait MultiLevelBuildTests extends UtestIntegrationTestSuite {
 object MultiLevelBuildTestsValidEdits extends MultiLevelBuildTests {
   val tests: Tests = Tests {
     savedClassLoaderIds = Seq.empty[Option[Int]]
-
+    savedServerId = ""
     test("validEdits") - retry(retryCount) {
       integrationTest { tester =>
         import tester._
@@ -255,6 +269,7 @@ object MultiLevelBuildTestsValidEdits extends MultiLevelBuildTests {
 object MultiLevelBuildTestsParseErrorEdits extends MultiLevelBuildTests {
   val tests: Tests = Tests {
     savedClassLoaderIds = Seq.empty[Option[Int]]
+    savedServerId = ""
     test("parseErrorEdits") - retry(retryCount) {
       integrationTest { tester =>
         import tester._
@@ -328,6 +343,7 @@ object MultiLevelBuildTestsParseErrorEdits extends MultiLevelBuildTests {
 object MultiLevelBuildTestsCompileErrorEdits extends MultiLevelBuildTests {
   val tests: Tests = Tests {
     savedClassLoaderIds = Seq.empty[Option[Int]]
+    savedServerId = ""
     test("compileErrorEdits") - retry(retryCount) {
       integrationTest { tester =>
         import tester._
@@ -416,6 +432,7 @@ object MultiLevelBuildTestsCompileErrorEdits extends MultiLevelBuildTests {
 object MultiLevelBuildTestsRuntimeErrorEdits extends MultiLevelBuildTests {
   val tests: Tests = Tests {
     savedClassLoaderIds = Seq.empty[Option[Int]]
+    savedServerId = ""
     test("runtimeErrorEdits") - retry(retryCount) {
       integrationTest { tester =>
         import tester._

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

@@ -65,6 +65,11 @@ abstract class Server[T](
         ) ()
         serverLog("server loop ended")
       }.getOrElse(throw new Exception("Mill server process already present"))
+    } catch {
+      case e: Throwable =>
+        serverLog("server loop error: " + e)
+        serverLog("server loop stack trace: " + e.getStackTrace.mkString("\n"))
+        throw e
     } finally {
       serverLog("finally exitServer")
       exitServer()
@@ -139,8 +144,8 @@ abstract class Server[T](
       else res
 
     } finally {
-      thread.interrupt()
       interrupt = false
+      thread.interrupt()
     }
   }