guiwf/lix
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MonitorFdHup: introduce a test

Browse Source 
Well, I was trying to figure out
https://git.lix.systems/lix-project/lix/issues/729 in which this feature
is clearly just broken on macOS, but frustratingly, it seems that it
*does* work, except for the daemon. um........ sure.

Change-Id: Iaa962f045c16fdfa82854151c90a03e5cf0eea47
This commit is contained in:
Jade Lovelace
2025-04-04 16:59:23 -07:00
parent 6d8e4337e2
commit 7aec313597
5 changed files with 144 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lix/libutil/meson.build
+1
View File
@@ -25,6 +25,7 @@ libutil_sources = files(
'hilite.cc',
'json-utils.cc',
'logging.cc',
'monitor-fd.cc',
'mount.cc',
'namespaces.cc',
'position.cc',
lix/libutil/monitor-fd.cc
+85
View File
@@ -0,0 +1,85 @@
#include "monitor-fd.hh"
namespace nix {
void MonitorFdHup::runThread(int watchFd, int terminateFd)
{
setCurrentThreadName("MonitorFdHup");
while (!quit) {
/* Wait indefinitely until a POLLHUP occurs. */
struct pollfd fds[2];
fds[0].fd = watchFd;
// There is a POSIX violation on macOS: you have to listen for
// at least POLLHUP to receive HUP events for a FD. POSIX says
// this is not so, and you should just receive them regardless,
// however, as of our testing on macOS 14.5, the events do not
// get delivered in such a case.
//
// This is allegedly filed as rdar://37537852.
//
// Relevant code, which backs this up:
// https://github.com/apple-oss-distributions/xnu/blob/94d3b452840153a99b38a3a9659680b2a006908e/bsd/kern/sys_generic.c#L1751-L1758
fds[0].events = POLLHUP;
fds[1].fd = terminateFd;
fds[1].events = POLLIN;
auto count = poll(fds, 2, -1);
if (count == -1) {
if (errno == EINTR || errno == EAGAIN) {
// These are best dealt with by just trying again.
continue;
} else {
throw SysError("in MonitorFdHup poll()");
}
}
/* This shouldn't happen, but can on macOS due to a bug.
See rdar://37550628.
This may eventually need a delay or further
coordination with the main thread if spinning proves
too harmful.
*/
if (count == 0) {
continue;
}
if (fds[0].revents & POLLHUP) {
callback();
break;
}
// No reason to actually look at the pipe FD if that's what
// woke us, the only thing that actually matters is the quit
// flag.
if (quit) {
break;
}
// On macOS, it is possible (although not observed on macOS
// 14.5) that in some limited cases on buggy kernel versions,
// all the non-POLLHUP events for the socket get delivered.
// Sleeping avoids pointlessly spinning a thread on those.
//
// N.B. excessive delay on this can cause the daemon connection
// thread to live longer than the client and lead to
// synchronization problems if clients assume that the server
// thread has released its temporary gc roots, etc.
// See https://github.com/NixOS/nix/pull/12714#discussion_r2009265904
usleep(1'000);
}
}
MonitorFdHup::MonitorFdHup(int fd, std::function<void()> callback) : callback(callback)
{
terminatePipe.create();
int terminateFd = terminatePipe.readSide.get();
thread = std::thread([this, fd, terminateFd]() { this->runThread(fd, terminateFd); });
};
MonitorFdHup::~MonitorFdHup()
{
quit = true;
// Poke the thread out of its poll wait
writeFull(terminatePipe.writeSide.get(), "*", false);
if (thread.joinable()) {
thread.join();
}
}
}
lix/libutil/monitor-fd.hh
+8 -77
View File
@@ -1,5 +1,6 @@
#pragma once
///@file
///@file Observes a file descriptor for hang-up events and notifies a
/// callback.
#include <thread>
#include <atomic>
@@ -15,7 +16,7 @@
namespace nix {
/** Observes a file descriptor for hang-up events and notifies a callback. */
class MonitorFdHup
{
private:
@@ -25,84 +26,14 @@ private:
*/
Pipe terminatePipe;
std::atomic_bool quit = false;
std::function<void()> callback;
void runThread(int watchFd, int terminateFd);
public:
MonitorFdHup(int fd)
{
terminatePipe.create();
auto &quit_ = this->quit;
int terminateFd = terminatePipe.readSide.get();
thread = std::thread([fd, terminateFd, &quit_]() {
setCurrentThreadName("MonitorFdHup");
while (!quit_) {
/* Wait indefinitely until a POLLHUP occurs. */
struct pollfd fds[2];
fds[0].fd = fd;
// There is a POSIX violation on macOS: you have to listen for
// at least POLLHUP to receive HUP events for a FD. POSIX says
// this is not so, and you should just receive them regardless,
// however, as of our testing on macOS 14.5, the events do not
// get delivered in such a case.
//
// This is allegedly filed as rdar://37537852.
//
// Relevant code, which backs this up:
// https://github.com/apple-oss-distributions/xnu/blob/94d3b452840153a99b38a3a9659680b2a006908e/bsd/kern/sys_generic.c#L1751-L1758
fds[0].events = POLLHUP;
fds[1].fd = terminateFd;
fds[1].events = POLLIN;
MonitorFdHup(int fd, std::function<void()> callback = nix::triggerInterrupt);
auto count = poll(fds, 2, -1);
if (count == -1) {
if (errno == EINTR || errno == EAGAIN) {
// These are best dealt with by just trying again.
continue;
} else {
throw SysError("in MonitorFdHup poll()");
}
}
/* This shouldn't happen, but can on macOS due to a bug.
See rdar://37550628.
This may eventually need a delay or further
coordination with the main thread if spinning proves
too harmful.
*/
if (count == 0) continue;
if (fds[0].revents & POLLHUP) {
triggerInterrupt();
break;
}
// No reason to actually look at the pipe FD if that's what
// woke us, the only thing that actually matters is the quit
// flag.
if (quit_) {
break;
}
// On macOS, it is possible (although not observed on macOS
// 14.5) that in some limited cases on buggy kernel versions,
// all the non-POLLHUP events for the socket get delivered.
// Sleeping avoids pointlessly spinning a thread on those.
//
// N.B. excessive delay on this can cause the daemon connection
// thread to live longer than the client and lead to
// synchronization problems if clients assume that the server
// thread has released its temporary gc roots, etc.
// See https://github.com/NixOS/nix/pull/12714#discussion_r2009265904
usleep(1'000);
}
});
};
~MonitorFdHup()
{
quit = true;
// Poke the thread out of its poll wait
writeFull(terminatePipe.writeSide.get(), "*", false);
if (thread.joinable()) {
thread.join();
}
}
~MonitorFdHup();
};
tests/unit/libutil/monitor-fd.cc
+49
View File
@@ -0,0 +1,49 @@
#include "lix/libutil/monitor-fd.hh"
#include "lix/libutil/error.hh"
#include "lix/libutil/file-descriptor.hh"
#include <atomic>
#include <future>
#include <gtest/gtest.h>
#include <sys/socket.h>
using namespace std::literals::chrono_literals;
namespace nix {
TEST(MonitorFdHup, works)
{
int socks[2];
int rv = socketpair(AF_UNIX, SOCK_STREAM, 0, socks);
if (rv) throw SysError("socketpair");
auto sock1 = AutoCloseFD{socks[0]};
auto sock2 = AutoCloseFD{socks[1]};
std::promise<void> called;
MonitorFdHup monitor(sock1.get(), [&called]() {
called.set_value();
});
sock2.close();
// 30 seconds should certainly do it.
called.get_future().wait_for(10s);
}
// Ensures that destroying the MonitorFdHup causes it to actually go away.
TEST(MonitorFdHup, destroys_safely)
{
int socks[2];
int rv = socketpair(AF_UNIX, SOCK_STREAM, 0, socks);
if (rv) throw SysError("socketpair");
auto sock1 = AutoCloseFD{socks[0]};
auto sock2 = AutoCloseFD{socks[1]};
{
MonitorFdHup monitor(sock1.get(), []() {
abort();
});
}
}
}
tests/unit/meson.build
+1
View File
@@ -62,6 +62,7 @@ libutil_tests_sources = files(
'libutil/json-utils.cc',
'libutil/logging.cc',
'libutil/lru-cache.cc',
'libutil/monitor-fd.cc',
'libutil/paths-setting.cc',
'libutil/pool.cc',
'libutil/references.cc',