1 files 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.
+                //
+                // This is allegedly filed as rdar://37537852.
                 //
-                // 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
-                    ;
+                // 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();
+        }
     }
 };