f4cb0e
From b552b05251980f693c729e251f93f5225b400714 Mon Sep 17 00:00:00 2001
f4cb0e
From: Paul Smith <psmith@gnu.org>
f4cb0e
Date: Sat, 3 Jun 2017 16:20:51 -0400
f4cb0e
Subject: [SV 51159] Use a non-blocking read with pselect to avoid hangs.
f4cb0e
f4cb0e
* posixos.c (set_blocking): Set blocking on a file descriptor.
f4cb0e
(jobserver_setup): Set non-blocking on the jobserver read side.
f4cb0e
(jobserver_parse_auth): Ditto.
f4cb0e
(jobserver_acquire_all): Set blocking to avoid a busy-wait loop.
f4cb0e
(jobserver_acquire): If the non-blocking read() returns without
f4cb0e
taking a token then try again.
f4cb0e
f4cb0e
diff --git a/posixos.c b/posixos.c
f4cb0e
index e642d7f..dbafa51 100644
f4cb0e
--- a/posixos.c
f4cb0e
+++ b/posixos.c
f4cb0e
@@ -62,6 +62,24 @@ make_job_rfd (void)
f4cb0e
 #endif
f4cb0e
 }
f4cb0e
 
f4cb0e
+static void
f4cb0e
+set_blocking (int fd, int blocking)
f4cb0e
+{
f4cb0e
+  // If we're not using pselect() don't change the blocking
f4cb0e
+#ifdef HAVE_PSELECT
f4cb0e
+  int flags;
f4cb0e
+  EINTRLOOP (flags, fcntl (fd, F_GETFL));
f4cb0e
+  if (flags >= 0)
f4cb0e
+    {
f4cb0e
+      int r;
f4cb0e
+      flags = blocking ? (flags & ~O_NONBLOCK) : (flags | O_NONBLOCK);
f4cb0e
+      EINTRLOOP (r, fcntl (fd, F_SETFL, flags));
f4cb0e
+      if (r < 0)
f4cb0e
+        pfatal_with_name ("fcntl(O_NONBLOCK)");
f4cb0e
+    }
f4cb0e
+#endif
f4cb0e
+}
f4cb0e
+
f4cb0e
 unsigned int
f4cb0e
 jobserver_setup (int slots)
f4cb0e
 {
f4cb0e
@@ -86,6 +104,9 @@ jobserver_setup (int slots)
f4cb0e
         pfatal_with_name (_("init jobserver pipe"));
f4cb0e
     }
f4cb0e
 
f4cb0e
+  /* When using pselect() we want the read to be non-blocking.  */
f4cb0e
+  set_blocking (job_fds[0], 0);
f4cb0e
+
f4cb0e
   return 1;
f4cb0e
 }
f4cb0e
 
f4cb0e
@@ -121,6 +142,9 @@ jobserver_parse_auth (const char *auth)
f4cb0e
       return 0;
f4cb0e
     }
f4cb0e
 
f4cb0e
+  /* When using pselect() we want the read to be non-blocking.  */
f4cb0e
+  set_blocking (job_fds[0], 0);
f4cb0e
+
f4cb0e
   return 1;
f4cb0e
 }
f4cb0e
 
f4cb0e
@@ -169,7 +193,10 @@ jobserver_acquire_all (void)
f4cb0e
 {
f4cb0e
   unsigned int tokens = 0;
f4cb0e
 
f4cb0e
-  /* Close the write side, so the read() won't hang.  */
f4cb0e
+  /* Use blocking reads to wait for all outstanding jobs.  */
f4cb0e
+  set_blocking (job_fds[0], 1);
f4cb0e
+
f4cb0e
+  /* Close the write side, so the read() won't hang forever.  */
f4cb0e
   close (job_fds[1]);
f4cb0e
   job_fds[1] = -1;
f4cb0e
 
f4cb0e
@@ -236,18 +263,12 @@ jobserver_pre_acquire (void)
f4cb0e
 unsigned int
f4cb0e
 jobserver_acquire (int timeout)
f4cb0e
 {
f4cb0e
-  sigset_t empty;
f4cb0e
-  fd_set readfds;
f4cb0e
   struct timespec spec;
f4cb0e
   struct timespec *specp = NULL;
f4cb0e
-  int r;
f4cb0e
-  char intake;
f4cb0e
+  sigset_t empty;
f4cb0e
 
f4cb0e
   sigemptyset (&empty);
f4cb0e
 
f4cb0e
-  FD_ZERO (&readfds);
f4cb0e
-  FD_SET (job_fds[0], &readfds);
f4cb0e
-
f4cb0e
   if (timeout)
f4cb0e
     {
f4cb0e
       /* Alarm after one second (is this too granular?)  */
f4cb0e
@@ -256,28 +277,52 @@ jobserver_acquire (int timeout)
f4cb0e
       specp = &spe;;
f4cb0e
     }
f4cb0e
 
f4cb0e
-  r = pselect (job_fds[0]+1, &readfds, NULL, NULL, specp, &empty);
f4cb0e
-
f4cb0e
-  if (r == -1)
f4cb0e
+  while (1)
f4cb0e
     {
f4cb0e
-      /* Better be SIGCHLD.  */
f4cb0e
-      if (errno != EINTR)
f4cb0e
-        pfatal_with_name (_("pselect jobs pipe"));
f4cb0e
-      return 0;
f4cb0e
-    }
f4cb0e
+      fd_set readfds;
f4cb0e
+      int r;
f4cb0e
+      char intake;
f4cb0e
 
f4cb0e
-  if (r == 0)
f4cb0e
-    /* Timeout.  */
f4cb0e
-    return 0;
f4cb0e
+      FD_ZERO (&readfds);
f4cb0e
+      FD_SET (job_fds[0], &readfds);
f4cb0e
 
f4cb0e
-  /* The read FD is ready: read it!  */
f4cb0e
-  EINTRLOOP (r, read (job_fds[0], &intake, 1));
f4cb0e
-  if (r < 0)
f4cb0e
-    pfatal_with_name (_("read jobs pipe"));
f4cb0e
+      r = pselect (job_fds[0]+1, &readfds, NULL, NULL, specp, &empty);
f4cb0e
+      if (r < 0)
f4cb0e
+        switch (errno)
f4cb0e
+          {
f4cb0e
+          case EINTR:
f4cb0e
+            /* SIGCHLD will show up as an EINTR.  */
f4cb0e
+            return 0;
f4cb0e
+
f4cb0e
+          case EBADF:
f4cb0e
+            /* Someone closed the jobs pipe.
f4cb0e
+               That shouldn't happen but if it does we're done.  */
f4cb0e
+              O (fatal, NILF, _("job server shut down"));
f4cb0e
 
f4cb0e
-  /* What does it mean if read() returns 0?  It shouldn't happen because only
f4cb0e
-     the master make can reap all the tokens and close the write side...??  */
f4cb0e
-  return r > 0;
f4cb0e
+          default:
f4cb0e
+            pfatal_with_name (_("pselect jobs pipe"));
f4cb0e
+          }
f4cb0e
+
f4cb0e
+      if (r == 0)
f4cb0e
+        /* Timeout.  */
f4cb0e
+        return 0;
f4cb0e
+
f4cb0e
+      /* The read FD is ready: read it!  This is non-blocking.  */
f4cb0e
+      EINTRLOOP (r, read (job_fds[0], &intake, 1));
f4cb0e
+
f4cb0e
+      if (r < 0)
f4cb0e
+        {
f4cb0e
+          /* Someone sniped our token!  Try again.  */
f4cb0e
+          if (errno == EAGAIN)
f4cb0e
+            continue;
f4cb0e
+
f4cb0e
+          pfatal_with_name (_("read jobs pipe"));
f4cb0e
+        }
f4cb0e
+
f4cb0e
+      /* read() should never return 0: only the master make can reap all the
f4cb0e
+         tokens and close the write side...??  */
f4cb0e
+      return r > 0;
f4cb0e
+    }
f4cb0e
 }
f4cb0e
 
f4cb0e
 #else