summaryrefslogtreecommitdiffstats
path: root/source/d/make/b552b05251980f693c729e251f93f5225b400714.patch
blob: 6f44ae3f2a587fb36eb9ef5d69e91444324d91a3 (plain) (blame)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
From b552b05251980f693c729e251f93f5225b400714 Mon Sep 17 00:00:00 2001
From: Paul Smith <psmith@gnu.org>
Date: Sat, 3 Jun 2017 16:20:51 -0400
Subject: [SV 51159] Use a non-blocking read with pselect to avoid hangs.

* posixos.c (set_blocking): Set blocking on a file descriptor.
(jobserver_setup): Set non-blocking on the jobserver read side.
(jobserver_parse_auth): Ditto.
(jobserver_acquire_all): Set blocking to avoid a busy-wait loop.
(jobserver_acquire): If the non-blocking read() returns without
taking a token then try again.
---
 posixos.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 71 insertions(+), 26 deletions(-)

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