From 0e18254aa81b9315c13d6ae736cb38666d19f122 Mon Sep 17 00:00:00 2001
From: Moritz Angermann <moritz.angermann@gmail.com>
Date: Wed, 5 Apr 2023 17:05:17 +0800
Subject: [PATCH] Fix shutdown behavior and resource management for
 recursive-nix on macOS

Previously, we relied on the `shutdown()` function to terminate `accept()`
calls on a listening socket. However, this approach did not work on macOS as
the waiting `accept()` call is not considered a connected socket, resulting in
an `ENOTCONN` error. Instead, we now close the listening socket to terminate
the `accept()` call.

Additionally, we fixed a resource management issue where we set the
`daemonSocket` variable to -1, triggering resource cleanup and causing the
`stopDaemon` function to be called twice. This resulted in errors as the socket
was already closed by the time the second `stopDaemon` call was made. Instead of
setting `daemonSocket` to -1, we now release the socket using the `release()`
method on a unique pointer. This properly transfers ownership and allows for
correct resource cleanup.

These changes ensure proper behavior and resource management for the
recursive-nix feature on macOS.
---
 src/libstore/build/local-derivation-goal.cc | 23 +++++++++++++++++----
 tests/recursive.sh                          |  3 ---
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc
index 21cd6e7ee..4b978f2a4 100644
--- a/src/libstore/build/local-derivation-goal.cc
+++ b/src/libstore/build/local-derivation-goal.cc
@@ -1457,7 +1457,7 @@ void LocalDerivationGoal::startDaemon()
                 (struct sockaddr *) &remoteAddr, &remoteAddrLen);
             if (!remote) {
                 if (errno == EINTR || errno == EAGAIN) continue;
-                if (errno == EINVAL) break;
+                if (errno == EINVAL || errno == ECONNABORTED) break;
                 throw SysError("accepting connection");
             }
 
@@ -1487,8 +1487,22 @@ void LocalDerivationGoal::startDaemon()
 
 void LocalDerivationGoal::stopDaemon()
 {
-    if (daemonSocket && shutdown(daemonSocket.get(), SHUT_RDWR) == -1)
-        throw SysError("shutting down daemon socket");
+    if (daemonSocket && shutdown(daemonSocket.get(), SHUT_RDWR) == -1) {
+        // According to the POSIX standard, the 'shutdown' function should
+        // return an ENOTCONN error when attempting to shut down a socket that
+        // hasn't been connected yet. This situation occurs when the 'accept'
+        // function is called on a socket without any accepted connections,
+        // leaving the socket unconnected. While Linux doesn't seem to produce
+        // an error for sockets that have only been accepted, more
+        // POSIX-compliant operating systems like OpenBSD, macOS, and others do
+        // return the ENOTCONN error. Therefore, we handle this error here to
+        // avoid raising an exception for compliant behaviour.
+        if (errno == ENOTCONN) {
+            daemonSocket.close();
+        } else {
+            throw SysError("shutting down daemon socket");
+        }
+    }
 
     if (daemonThread.joinable())
         daemonThread.join();
@@ -1499,7 +1513,8 @@ void LocalDerivationGoal::stopDaemon()
         thread.join();
     daemonWorkerThreads.clear();
 
-    daemonSocket = -1;
+    // release the socket.
+    daemonSocket.close();
 }
 
 
diff --git a/tests/recursive.sh b/tests/recursive.sh
index 638f06f85..b661422ed 100644
--- a/tests/recursive.sh
+++ b/tests/recursive.sh
@@ -3,9 +3,6 @@ source common.sh
 sed -i 's/experimental-features .*/& recursive-nix/' "$NIX_CONF_DIR"/nix.conf
 restartDaemon
 
-# FIXME
-if [[ $(uname) != Linux ]]; then skipTest "Not running Linux"; fi
-
 clearStore
 
 rm -f $TEST_ROOT/result