tjh.dev
1From b552b05251980f693c729e251f93f5225b400714 Mon Sep 17 00:00:00 2001
2From: Paul Smith <psmith@gnu.org>
3Date: Sat, 3 Jun 2017 16:20:51 -0400
4Subject: [SV 51159] Use a non-blocking read with pselect to avoid hangs.
5
6* posixos.c (set_blocking): Set blocking on a file descriptor.
7(jobserver_setup): Set non-blocking on the jobserver read side.
8(jobserver_parse_auth): Ditto.
9(jobserver_acquire_all): Set blocking to avoid a busy-wait loop.
10(jobserver_acquire): If the non-blocking read() returns without
11taking a token then try again.
12---
13 posixos.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++-----------------
14 1 file changed, 71 insertions(+), 26 deletions(-)
15
16diff --git posixos.c posixos.c
17index e642d7f..dbafa51 100644
18--- posixos.c
19+++ posixos.c
20@@ -62,6 +62,24 @@ make_job_rfd (void)
21 #endif
22 }
23
24+static void
25+set_blocking (int fd, int blocking)
26+{
27+ // If we're not using pselect() don't change the blocking
28+#ifdef HAVE_PSELECT
29+ int flags;
30+ EINTRLOOP (flags, fcntl (fd, F_GETFL));
31+ if (flags >= 0)
32+ {
33+ int r;
34+ flags = blocking ? (flags & ~O_NONBLOCK) : (flags | O_NONBLOCK);
35+ EINTRLOOP (r, fcntl (fd, F_SETFL, flags));
36+ if (r < 0)
37+ pfatal_with_name ("fcntl(O_NONBLOCK)");
38+ }
39+#endif
40+}
41+
42 unsigned int
43 jobserver_setup (int slots)
44 {
45@@ -86,6 +104,9 @@ jobserver_setup (int slots)
46 pfatal_with_name (_("init jobserver pipe"));
47 }
48
49+ /* When using pselect() we want the read to be non-blocking. */
50+ set_blocking (job_fds[0], 0);
51+
52 return 1;
53 }
54
55@@ -121,6 +142,9 @@ jobserver_parse_auth (const char *auth)
56 return 0;
57 }
58
59+ /* When using pselect() we want the read to be non-blocking. */
60+ set_blocking (job_fds[0], 0);
61+
62 return 1;
63 }
64
65@@ -169,7 +193,10 @@ jobserver_acquire_all (void)
66 {
67 unsigned int tokens = 0;
68
69- /* Close the write side, so the read() won't hang. */
70+ /* Use blocking reads to wait for all outstanding jobs. */
71+ set_blocking (job_fds[0], 1);
72+
73+ /* Close the write side, so the read() won't hang forever. */
74 close (job_fds[1]);
75 job_fds[1] = -1;
76
77@@ -236,18 +263,12 @@ jobserver_pre_acquire (void)
78 unsigned int
79 jobserver_acquire (int timeout)
80 {
81- sigset_t empty;
82- fd_set readfds;
83 struct timespec spec;
84 struct timespec *specp = NULL;
85- int r;
86- char intake;
87+ sigset_t empty;
88
89 sigemptyset (&empty);
90
91- FD_ZERO (&readfds);
92- FD_SET (job_fds[0], &readfds);
93-
94 if (timeout)
95 {
96 /* Alarm after one second (is this too granular?) */
97@@ -256,28 +277,52 @@ jobserver_acquire (int timeout)
98 specp = &spec;
99 }
100
101- r = pselect (job_fds[0]+1, &readfds, NULL, NULL, specp, &empty);
102-
103- if (r == -1)
104+ while (1)
105 {
106- /* Better be SIGCHLD. */
107- if (errno != EINTR)
108- pfatal_with_name (_("pselect jobs pipe"));
109- return 0;
110- }
111+ fd_set readfds;
112+ int r;
113+ char intake;
114
115- if (r == 0)
116- /* Timeout. */
117- return 0;
118+ FD_ZERO (&readfds);
119+ FD_SET (job_fds[0], &readfds);
120
121- /* The read FD is ready: read it! */
122- EINTRLOOP (r, read (job_fds[0], &intake, 1));
123- if (r < 0)
124- pfatal_with_name (_("read jobs pipe"));
125+ r = pselect (job_fds[0]+1, &readfds, NULL, NULL, specp, &empty);
126+ if (r < 0)
127+ switch (errno)
128+ {
129+ case EINTR:
130+ /* SIGCHLD will show up as an EINTR. */
131+ return 0;
132+
133+ case EBADF:
134+ /* Someone closed the jobs pipe.
135+ That shouldn't happen but if it does we're done. */
136+ O (fatal, NILF, _("job server shut down"));
137
138- /* What does it mean if read() returns 0? It shouldn't happen because only
139- the master make can reap all the tokens and close the write side...?? */
140- return r > 0;
141+ default:
142+ pfatal_with_name (_("pselect jobs pipe"));
143+ }
144+
145+ if (r == 0)
146+ /* Timeout. */
147+ return 0;
148+
149+ /* The read FD is ready: read it! This is non-blocking. */
150+ EINTRLOOP (r, read (job_fds[0], &intake, 1));
151+
152+ if (r < 0)
153+ {
154+ /* Someone sniped our token! Try again. */
155+ if (errno == EAGAIN)
156+ continue;
157+
158+ pfatal_with_name (_("read jobs pipe"));
159+ }
160+
161+ /* read() should never return 0: only the master make can reap all the
162+ tokens and close the write side...?? */
163+ return r > 0;
164+ }
165 }
166
167 #else
168--
169cgit v1.0-41-gc330
170