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.
This commit is contained in:
Moritz Angermann 2023-04-05 17:05:17 +08:00
parent ef432b2b15
commit 0e18254aa8
No known key found for this signature in database
GPG key ID: A98C646D142C675F
2 changed files with 19 additions and 7 deletions

View file

@ -1457,7 +1457,7 @@ void LocalDerivationGoal::startDaemon()
(struct sockaddr *) &remoteAddr, &remoteAddrLen); (struct sockaddr *) &remoteAddr, &remoteAddrLen);
if (!remote) { if (!remote) {
if (errno == EINTR || errno == EAGAIN) continue; if (errno == EINTR || errno == EAGAIN) continue;
if (errno == EINVAL) break; if (errno == EINVAL || errno == ECONNABORTED) break;
throw SysError("accepting connection"); throw SysError("accepting connection");
} }
@ -1487,8 +1487,22 @@ void LocalDerivationGoal::startDaemon()
void LocalDerivationGoal::stopDaemon() void LocalDerivationGoal::stopDaemon()
{ {
if (daemonSocket && shutdown(daemonSocket.get(), SHUT_RDWR) == -1) if (daemonSocket && shutdown(daemonSocket.get(), SHUT_RDWR) == -1) {
throw SysError("shutting down daemon socket"); // 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()) if (daemonThread.joinable())
daemonThread.join(); daemonThread.join();
@ -1499,7 +1513,8 @@ void LocalDerivationGoal::stopDaemon()
thread.join(); thread.join();
daemonWorkerThreads.clear(); daemonWorkerThreads.clear();
daemonSocket = -1; // release the socket.
daemonSocket.close();
} }

View file

@ -3,9 +3,6 @@ source common.sh
sed -i 's/experimental-features .*/& recursive-nix/' "$NIX_CONF_DIR"/nix.conf sed -i 's/experimental-features .*/& recursive-nix/' "$NIX_CONF_DIR"/nix.conf
restartDaemon restartDaemon
# FIXME
if [[ $(uname) != Linux ]]; then skipTest "Not running Linux"; fi
clearStore clearStore
rm -f $TEST_ROOT/result rm -f $TEST_ROOT/result