[PATCH 3/3] daemon: explicitly allow EINTR during poll()

["Carlo Marcelo Arenas Belón via GitGitGadget" <gitgitgadget@gmail.com>]

From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= <carenas@gmail.com>

If the setup for the SIGCHLD signal handler sets SA_RESTART, poll()
might not return with -1 and set errno to EINTR when a signal is
received.

Since the logic to reap zombie childs relies om those interruptions
make sure to explicitly disable SA_RESTART around this function.

Add a Makefile flag for portability to systems that don't have the
functionality to change those flags or where it is not needed.

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 Makefile         |  6 ++++++
 config.mak.uname |  4 ++++
 configure.ac     |  5 +++++
 daemon.c         | 26 ++++++++++++++++++++++----
 meson.build      |  1 +
 5 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 70d1543b6b86..d489f94dc6f0 100644
--- a/Makefile
+++ b/Makefile
@@ -144,6 +144,9 @@ include shared.mak
 # Define NO_PREAD if you have a problem with pread() system call (e.g.
 # cygwin1.dll before v1.5.22).
 #
+# Define NO_SIGINTERRUPT if you don't have siginterrupt() or SA_RESTART
+# or if your signal(SIGCHLD) implementation doesn't set SA_RESTART.
+#
 # Define NO_SETITIMER if you don't have setitimer()
 #
 # Define NO_STRUCT_ITIMERVAL if you don't have struct itimerval
@@ -1902,6 +1905,9 @@ ifdef NO_PREAD
 	COMPAT_CFLAGS += -DNO_PREAD
 	COMPAT_OBJS += compat/pread.o
 endif
+ifdef NO_SIGINTERRUPT
+	COMPAT_CFLAGS += -DNO_SIGINTERRUPT
+endif
 ifdef NO_FAST_WORKING_DIRECTORY
 	BASIC_CFLAGS += -DNO_FAST_WORKING_DIRECTORY
 endif
diff --git a/config.mak.uname b/config.mak.uname
index 52160ef5cb07..e824b45a4020 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -486,6 +486,7 @@ ifeq ($(uname_S),Windows)
 	NO_STRTOUMAX = YesPlease
 	NO_MKDTEMP = YesPlease
 	NO_INTTYPES_H = YesPlease
+	NO_SIGINTERRUPT = YesPlease
 	CSPRNG_METHOD = rtlgenrandom
 	# VS2015 with UCRT claims that snprintf and friends are C99 compliant,
 	# so we don't need this:
@@ -661,6 +662,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
 	NO_PREAD = YesPlease
 	NO_MMAP = YesPlease
 	NO_POLL = YesPlease
+	NO_SIGINTERRUPT = UnfortunatelyYes
 	NO_INTPTR_T = UnfortunatelyYes
 	CSPRNG_METHOD = openssl
 	SANE_TOOL_PATH = /usr/coreutils/bin:/usr/local/bin
@@ -697,6 +699,7 @@ ifeq ($(uname_S),MINGW)
 	NEEDS_LIBICONV = YesPlease
 	NO_STRTOUMAX = YesPlease
 	NO_MKDTEMP = YesPlease
+	NO_SIGINTERRUPT = YesPlease
 	NO_SVN_TESTS = YesPlease
 
 	# The builtin FSMonitor requires Named Pipes and Threads on Windows.
@@ -791,4 +794,5 @@ ifeq ($(uname_S),QNX)
 	NO_PTHREADS = YesPlease
 	NO_STRCASESTR = YesPlease
 	NO_STRLCPY = YesPlease
+	NO_SIGINTERRUPT = UnfortunatelyYes
 endif
diff --git a/configure.ac b/configure.ac
index f6caab919a3e..2abb2a32cd1e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1192,6 +1192,11 @@ GIT_CHECK_FUNC(getdelim,
 [HAVE_GETDELIM=])
 GIT_CONF_SUBST([HAVE_GETDELIM])
 #
+# Define NO_SIGINTERRUPT if you don't have siginterrupt.
+GIT_CHECK_FUNC(siginterrupt,
+[NO_SIGINTERRUPT=],
+[NO_SIGINTERRUPT=YesPlease])
+GIT_CONF_SUBST([NO_SIGINTERRUPT])
 #
 # Define NO_MMAP if you want to avoid mmap.
 #
diff --git a/daemon.c b/daemon.c
index d870ad2f63c1..542e63822391 100644
--- a/daemon.c
+++ b/daemon.c
@@ -912,12 +912,16 @@ static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen)
 		add_child(&cld, addr, addrlen);
 }
 
-static void child_handler(int signo UNUSED)
+static void child_handler(int signo)
 {
 	/*
-	 * Otherwise empty handler because systemcalls will get interrupted
-	 * upon signal receipt
+	 * Empty handler because systemcalls should get interrupted
+	 * upon signal receipt.
 	 */
+#ifdef NO_SIGINTERRUPT
+	/* SysV needs the handler to be rearmed */
+	signal(signo, child_handler);
+#endif
 }
 
 static int set_reuse_addr(int sockfd)
@@ -1118,8 +1122,10 @@ static void socksetup(struct string_list *listen_addr, int listen_port, struct s
 
 static int service_loop(struct socketlist *socklist)
 {
-	struct pollfd *pfd;
+#ifndef NO_SIGINTERRUPT
 	struct sigaction sa;
+#endif
+	struct pollfd *pfd;
 
 	CALLOC_ARRAY(pfd, socklist->nr);
 
@@ -1128,14 +1134,22 @@ static int service_loop(struct socketlist *socklist)
 		pfd[i].events = POLLIN;
 	}
 
+#ifdef NO_SIGINTERRUPT
+	signal(SIGCHLD, child_handler);
+#else
 	sigemptyset(&sa.sa_mask);
 	sa.sa_flags = SA_NOCLDSTOP | SA_RESTART;
 	sa.sa_handler = child_handler;
 	sigaction(SIGCHLD, &sa, NULL);
+#endif
 
 	for (;;) {
 		check_dead_children();
 
+#ifndef NO_SIGINTERRUPT
+		sa.sa_flags &= ~SA_RESTART;
+		sigaction(SIGCHLD, &sa, NULL);
+#endif
 		if (poll(pfd, socklist->nr, -1) < 0) {
 			if (errno != EINTR) {
 				logerror("Poll failed, resuming: %s",
@@ -1144,6 +1158,10 @@ static int service_loop(struct socketlist *socklist)
 			}
 			continue;
 		}
+#ifndef NO_SIGINTERRUPT
+		sa.sa_flags |= SA_RESTART;
+		sigaction(SIGCHLD, &sa, NULL);
+#endif
 
 		for (size_t i = 0; i < socklist->nr; i++) {
 			if (pfd[i].revents & POLLIN) {
diff --git a/meson.build b/meson.build
index 7fea4a34d684..54942db151e8 100644
--- a/meson.build
+++ b/meson.build
@@ -1361,6 +1361,7 @@ checkfuncs = {
   'setenv' : ['setenv.c'],
   'mkdtemp' : ['mkdtemp.c'],
   'initgroups' : [],
+  'siginterrupt' : [],
   'strtoumax' : ['strtoumax.c', 'strtoimax.c'],
   'pread' : ['pread.c'],
 }
-- 
gitgitgadget