1
0
Fork 0
mirror of https://github.com/NixOS/nix synced 2025-06-25 10:41:16 +02:00

daemon: remove workaround for macOS kernel bug that seems fixed

This was filed as https://github.com/nixos/nix/issues/7584, but as far
as I can tell, the previous solution of POLLHUP works just fine on macOS
14. I've also tested on an ancient machine with macOS 10.15.7, which
also has POLLHUP work correctly.

It's possible this might regress some older versions of macOS that have
a kernel bug, but I went looking through the history on the sources and
didn't find anything that looked terribly convincingly like a bug fix
between 2020 and today. If such a broken version exists, it seems pretty
reasonable to suggest simply updating the OS.

Change-Id: I178a038baa000f927ea2cbc4587d69d8ab786843

Based off of commit 69e2ee5b25752ba5fd8644cef56fb9d627ca4a64. Ericson2314 added
additional other information.
This commit is contained in:
Jade Lovelace 2024-07-13 00:27:09 +02:00 committed by John Ericson
parent 1c636284a3
commit 9b3352c3c8

View file

@ -26,22 +26,38 @@ public:
notifyPipe.create(); notifyPipe.create();
thread = std::thread([this, fd]() { thread = std::thread([this, fd]() {
while (true) { while (true) {
/* Polling for no specific events (i.e. just waiting // There is a POSIX violation on macOS: you have to listen for
for an error/hangup) doesn't work on macOS // at least POLLHUP to receive HUP events for a FD. POSIX says
anymore. So wait for read events and ignore // this is not so, and you should just receive them regardless.
them. */ // However, as of our testing on macOS 14.5, the events do not
// FIXME(jade): we have looked at the XNU kernel code and as // get delivered if in the all-bits-unset case, but do get
// far as we can tell, the above is bogus. It should be the // delivered if `POLLHUP` is set.
// case that the previous version of this and the current //
// version are identical: waiting for POLLHUP and POLLRDNORM in // This bug filed as rdar://37537852
// the kernel *should* be identical. // (https://openradar.appspot.com/37537852).
//
// macOS's own man page
// (https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/poll.2.html)
// additionally says that `POLLHUP` is ignored as an input. It
// seems the likely order of events here was
//
// 1. macOS did not follow the POSIX spec
//
// 2. Somebody ninja-fixed this other spec violation to make
// sure `POLLHUP` was not forgotten about, even though they
// "fixed" this issue in a spec-non-compliant way. Whatever,
// we'll use the fix.
//
// Relevant code, current version, which shows the :
// https://github.com/apple-oss-distributions/xnu/blob/94d3b452840153a99b38a3a9659680b2a006908e/bsd/kern/sys_generic.c#L1751-L1758 // https://github.com/apple-oss-distributions/xnu/blob/94d3b452840153a99b38a3a9659680b2a006908e/bsd/kern/sys_generic.c#L1751-L1758
// //
// So, this needs actual testing and we need to figure out if // The `POLLHUP` detection was added in
// this is actually bogus. // https://github.com/apple-oss-distributions/xnu/commit/e13b1fa57645afc8a7b2e7d868fe9845c6b08c40#diff-a5aa0b0e7f4d866ca417f60702689fc797e9cdfe33b601b05ccf43086c35d395R1468
// That means added in 2007 or earlier. Should be good enough
// for us.
short hangup_events = short hangup_events =
#ifdef __APPLE__ #ifdef __APPLE__
POLLRDNORM POLLHUP
#else #else
0 0
#endif #endif
@ -82,9 +98,10 @@ public:
if (fds[1].revents & POLLHUP) { if (fds[1].revents & POLLHUP) {
break; break;
} }
/* This will only happen on macOS. We sleep a bit to // On macOS, it is possible (although not observed on macOS
avoid waking up too often if the client is sending // 14.5) that in some limited cases on buggy kernel versions,
input. */ // all the non-POLLHUP events for the socket get delivered.
// Sleeping avoids pointlessly spinning a thread on those.
sleep(1); sleep(1);
} }
}); });