From 69e2ee5b25752ba5fd8644cef56fb9d627ca4a64 Mon Sep 17 00:00:00 2001
From: Jade Lovelace <lix@jade.fyi>
Date: Sat, 13 Jul 2024 00:27:09 +0200
Subject: [PATCH] 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
---
 src/libutil/monitor-fd.hh | 45 +++++++++++++++++----------------------
 1 file changed, 20 insertions(+), 25 deletions(-)

diff --git a/src/libutil/monitor-fd.hh b/src/libutil/monitor-fd.hh
index 233b73796..ea6e89017 100644
--- a/src/libutil/monitor-fd.hh
+++ b/src/libutil/monitor-fd.hh
@@ -38,26 +38,17 @@ public:
                 /* Wait indefinitely until a POLLHUP occurs. */
                 struct pollfd fds[2];
                 fds[0].fd = fd;
-                /* 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.
-                // https://github.com/apple-oss-distributions/xnu/blob/94d3b452840153a99b38a3a9659680b2a006908e/bsd/kern/sys_generic.c#L1751-L1758
+                // 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.
                 //
-                // So, this needs actual testing and we need to figure out if
-                // this is actually bogus.
-                fds[0].events =
-                    #ifdef __APPLE__
-                    POLLRDNORM
-                    #else
-                    0
-                    #endif
-                    ;
+                // 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;
 
@@ -82,14 +73,16 @@ public:
                     triggerInterrupt();
                     break;
                 }
-                // No reason to actually look at the pipe FD, we just need it
-                // to be able to get woken.
+                // 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;
                 }
-                /* 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);
             }
         });
@@ -100,7 +93,9 @@ public:
         quit = true;
         // Poke the thread out of its poll wait
         writeFull(terminatePipe.writeSide.get(), "*", false);
-        thread.join();
+        if (thread.joinable()) {
+            thread.join();
+        }
     }
 };