Fix race condition in DoubleLocked#release (#5153)

This PR makes some uses of `DoubleLock` safer, and fixes a race
condition with it.

`DoubleLock` is basically used as a wrapper around a file lock (second
lock), using it alongside a memory lock (first lock), that acts as a
mutex for it. That way, the same JVM process doesn't try to acquire the
file lock twice at the same time in a given JVM, which
`java.nio.channels.FileChannel` doesn't allow (it throws an
`OverlappingFileLockException` in that case).

This PR makes sure `MillMain` and `MillDaemonMain` use the same
`Lock.memory()` instance as a guard when trying to acquire a lock for
the output directory. This doesn't solve the issue I was running into
(see below), but seems to be safer nonetheless.

It also tries to dispose of locks in `DoubleLock` in case calls to
`Lock#{lock,tryLock}` throw.

Lastly, it releases the two locks in `DoubleLocked` in the reverse order
that they were acquired. This fixes the test case below. Doing so fixes
a race condition, where
- a first thread acquires the lock
- a second one tries to acquire it, and waits for it
- the first thread releases the lock

During the third step, releasing the memory lock before the file lock
makes the second thread acquire the memory lock, and then possibly try
to acquire the file lock, before the first thread is done releasing the
file lock. The JVM doesn't allow this, and the second thread gets an
`OverlappingFileLockException`.

See the discussion in #5152 too

This fixes
```text
$ ./mill 'integration.feature[output-directory].packaged.daemon.testForked' "mill.integration.OutputDirectoryLockTests.basic"
```
for me, locally.

Author: alexarchambault

runner/client/src/mill/client/lock/DoubleLock.java

@@ -12,22 +12,44 @@ public DoubleLock(Lock lock1, Lock lock2) throws Exception {
 
   @Override
   public Locked lock() throws Exception {
-    return new DoubleLocked(lock1.lock(), lock2.lock());
+    Locked l1 = null;
+    Locked l2 = null;
+    Locked result = null;
+    try {
+      l1 = lock1.lock();
+      l2 = lock2.lock();
+      result = new DoubleLocked(l1, l2);
+    } finally {
+      if (result == null) {
+        if (l2 != null) l2.release();
+        if (l1 != null) l1.release();
+      }
+    }
+
+    return result;
   }
 
   @Override
   public TryLocked tryLock() throws Exception {
-    TryLocked l1 = lock1.tryLock();
-    TryLocked l2 = lock2.tryLock();
-    if (l1.isLocked() && l2.isLocked()) {
-      return new DoubleTryLocked(l1, l2);
-    } else {
-      // Unlock the locks in the opposite order in which we originally took them
-      l2.release();
-      l1.release();
-
-      return new DoubleTryLocked(null, null);
+    TryLocked l1 = null;
+    TryLocked l2 = null;
+    TryLocked result = null;
+    try {
+      l1 = lock1.tryLock();
+      if (l1.isLocked()) {
+        l2 = lock2.tryLock();
+        if (l2.isLocked()) result = new DoubleTryLocked(l1, l2);
+      }
+    } finally {
+      if (result == null) {
+        if (l2 != null) l2.release();
+        if (l1 != null) l1.release();
+      }
     }
+
+    if (result == null) result = new DoubleTryLocked(null, null);
+
+    return result;
   }
 
   @Override

runner/client/src/mill/client/lock/DoubleLocked.java

@@ -12,7 +12,7 @@ public DoubleLocked(Locked lock1, Locked lock2) {
 
   @Override
   public void release() throws Exception {
-    this.lock1.release();
     this.lock2.release();
+    this.lock1.release();
   }
 }

runner/daemon/src/mill/daemon/MillDaemonMain.scala

@@ -52,7 +52,7 @@ class MillDaemonMain(
   val out = os.Path(OutFiles.out, mill.define.BuildCtx.workspaceRoot)
 
   val outLock = new DoubleLock(
-    Lock.memory(),
+    MillMain.outMemoryLock,
     Lock.file((out / OutFiles.millOutLock).toString)
   )
 

runner/daemon/src/mill/daemon/MillMain.scala

@@ -3,7 +3,7 @@ package mill.daemon
 import mill.api.internal.{BspServerResult, internal}
 import mill.api.{Logger, MillException, Result, SystemStreams}
 import mill.bsp.BSP
-import mill.client.lock.Lock
+import mill.client.lock.{DoubleLock, Lock}
 import mill.constants.{DaemonFiles, OutFiles, Util}
 import mill.define.BuildCtx
 import mill.internal.{Colors, MultiStream, PromptLogger}
@@ -58,6 +58,11 @@ object MillMain {
       }
     )
 
+    val outLock = new DoubleLock(
+      outMemoryLock,
+      Lock.file((out / OutFiles.millOutLock).toString)
+    )
+
     val daemonDir = os.Path(args.head)
     val (result, _) =
       try main0(
@@ -71,13 +76,15 @@ object MillMain {
           initialSystemProperties = sys.props.toMap,
           systemExit = i => sys.exit(i),
           daemonDir = daemonDir,
-          outLock = Lock.file((out / OutFiles.millOutLock).toString)
+          outLock = outLock
         )
       catch handleMillException(initialSystemStreams.err, ())
 
     System.exit(if (result) 0 else 1)
   }
 
+  val outMemoryLock = Lock.memory()
+
   private def withStreams[T](
       bspMode: Boolean,
       streams: SystemStreams