diff --git a/src/libutil/unix/monitor-fd.hh b/src/libutil/unix/monitor-fd.hh index d6ec47f49..334506146 100644 --- a/src/libutil/unix/monitor-fd.hh +++ b/src/libutil/unix/monitor-fd.hh @@ -26,22 +26,38 @@ public: notifyPipe.create(); thread = std::thread([this, fd]() { while (true) { - /* Polling for no specific events (i.e. just waiting - for an error/hangup) doesn't work on macOS - anymore. So wait for read events and ignore - them. */ - // FIXME(jade): we have looked at the XNU kernel code and as - // far as we can tell, the above is bogus. It should be the - // case that the previous version of this and the current - // version are identical: waiting for POLLHUP and POLLRDNORM in - // the kernel *should* be identical. + // 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 if in the all-bits-unset case, but do get + // delivered if `POLLHUP` is set. + // + // This bug filed as rdar://37537852 + // (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 // - // So, this needs actual testing and we need to figure out if - // this is actually bogus. + // The `POLLHUP` detection was added in + // 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 = #ifdef __APPLE__ - POLLRDNORM + POLLHUP #else 0 #endif @@ -82,9 +98,10 @@ public: if (fds[1].revents & POLLHUP) { break; } - /* This will only happen on macOS. We sleep a bit to - avoid waking up too often if the client is sending - input. */ + // 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. sleep(1); } });