Changes since v2
* Add a new patch 2 that modifies windows' sigaction so there is no more
need for a fallback
* Hopefully no more silly mistakes and a variable that finally makes sense
Changes since v1
* Almost all references to siginterrupt has been removed and a better named
variable used instead
* Changes had been abstracted to minimize ifdefs and their introduction
staged more naturally
Carlo Marcelo Arenas Belón (4):
compat/posix.h: track SA_RESTART fallback
compat/mingw: allow sigaction(SIGCHLD)
daemon: use sigaction() to install child_handler()
daemon: explicitly allow EINTR during poll()
Makefile | 5 +++++
compat/mingw-posix.h | 2 +-
compat/mingw.c | 4 +++-
compat/posix.h | 8 ++++++++
config.mak.uname | 7 ++++---
configure.ac | 16 ++++++++++++++++
daemon.c | 33 ++++++++++++++++++++++++++++-----
meson.build | 4 ++++
8 files changed, 69 insertions(+), 10 deletions(-)
base-commit: cb3b40381e1d5ee32dde96521ad7cfd68eb308a6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2002%2Fcarenas%2Fsiginterrupt-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2002/carenas/siginterrupt-v3
Pull-Request: https://github.com/git/git/pull/2002
Range-diff vs v2:
1: e82b7425bbc ! 1: ae1ca6bb2b2 compat/posix.h: track SA_RESTART fallback
@@ Makefile: include shared.mak
# when attempting to read from an fopen'ed directory (or even to fopen
# it at all).
#
-+# Define USE_NON_POSIX_SIGNAL if don't have support for SA_RESTART or
-+# prefer to use ANSI C signal() over POSIX sigaction()
++# Define NO_RESTARTABLE_SIGNALS if don't have support for SA_RESTART
+#
# Define OPEN_RETURNS_EINTR if your open() system call may return EINTR
# when a signal is received (as opposed to restarting).
@@ Makefile: ifdef FREAD_READS_DIRECTORIES
COMPAT_CFLAGS += -DFREAD_READS_DIRECTORIES
COMPAT_OBJS += compat/fopen.o
endif
-+ifdef USE_NON_POSIX_SIGNAL
-+ COMPAT_CFLAGS += -DUSE_NON_POSIX_SIGNAL
++ifdef NO_RESTARTABLE_SIGNALS
++ COMPAT_CFLAGS += -DNO_RESTARTABLE_SIGNALS
+endif
ifdef OPEN_RETURNS_EINTR
COMPAT_CFLAGS += -DOPEN_RETURNS_EINTR
@@ compat/posix.h: char *gitdirname(char *);
+ * not on some systems (e.g. NonStop, QNX).
+ */
+#ifndef SA_RESTART
-+# define SA_RESTART 0 /* disabled for sigaction() */
++# define SA_RESTART 0 /* disabled for sigaction() */
+#endif
+
typedef uintmax_t timestamp_t;
@@ config.mak.uname: ifeq ($(uname_S),Windows)
NO_STRTOUMAX = YesPlease
NO_MKDTEMP = YesPlease
NO_INTTYPES_H = YesPlease
-+ USE_NON_POSIX_SIGNAL = YesPlease
++ NO_RESTARTABLE_SIGNALS = YesPlease
CSPRNG_METHOD = rtlgenrandom
# VS2015 with UCRT claims that snprintf and friends are C99 compliant,
# so we don't need this:
@@ config.mak.uname: ifeq ($(uname_S),NONSTOP_KERNEL)
NO_MMAP = YesPlease
NO_POLL = YesPlease
NO_INTPTR_T = UnfortunatelyYes
-+ USE_NON_POSIX_SIGNAL = UnfortunatelyYes
++ NO_RESTARTABLE_SIGNALS = UnfortunatelyYes
CSPRNG_METHOD = openssl
SANE_TOOL_PATH = /usr/coreutils/bin:/usr/local/bin
SHELL_PATH = /usr/coreutils/bin/bash
@@ config.mak.uname: ifeq ($(uname_S),MINGW)
NEEDS_LIBICONV = YesPlease
NO_STRTOUMAX = YesPlease
NO_MKDTEMP = YesPlease
-+ USE_NON_POSIX_SIGNAL = YesPlease
++ NO_RESTARTABLE_SIGNALS = YesPlease
NO_SVN_TESTS = YesPlease
# The builtin FSMonitor requires Named Pipes and Threads on Windows.
@@ config.mak.uname: ifeq ($(uname_S),QNX)
NO_PTHREADS = YesPlease
NO_STRCASESTR = YesPlease
NO_STRLCPY = YesPlease
-+ USE_NON_POSIX_SIGNAL = UnfortunatelyYes
++ NO_RESTARTABLE_SIGNALS = UnfortunatelyYes
endif
## configure.ac ##
@@ configure.ac: fi
GIT_CONF_SUBST([ICONV_OMITS_BOM])
fi
-+# Define USE_NON_POSIX_SIGNAL if don't have support for SA_RESTART or
-+# prefer using ANSI C signal() over POSIX sigaction()
++# Define NO_RESTARTABLE_SIGNALS if don't have support for SA_RESTART
+
+AC_CACHE_CHECK([whether SA_RESTART is supported], [ac_cv_siginterrupt], [
+ AC_COMPILE_IFELSE(
+ [AC_LANG_PROGRAM([#include <signal.h>], [[
-+ #ifdef SA_RESTART
-+ #endif
-+ siginterrupt(SIGCHLD, 1)
-+ ]])],[ac_cv_siginterrupt=yes],[
++ #ifdef SA_RESTART
++ restartable signals supported
++ #endif
++ ]])],[
+ ac_cv_siginterrupt=no
-+ USE_NON_POSIX_SIGNAL=UnfortunatelyYes
-+ ]
++ NO_RESTARTABLE_SIGNALS=UnfortunatelyYes
++ ], [ac_cv_siginterrupt=yes]
+ )
+])
-+GIT_CONF_SUBST([USE_NON_POSIX_SIGNAL])
++GIT_CONF_SUBST([NO_RESTARTABLE_SIGNALS])
+
## Checks for typedefs, structures, and compiler characteristics.
AC_MSG_NOTICE([CHECKS for typedefs, structures, and compiler characteristics])
@@ meson.build: else
endif
+if compiler.get_define('SA_RESTART', prefix: '#include <signal.h>') == ''
-+ libgit_c_args += '-DUSE_NON_POSIX_SIGNAL'
++ libgit_c_args += '-DNO_RESTARTABLE_SIGNALS'
+endif
+
if not compiler.has_header('sys/select.h')
-: ----------- > 2: 3f63479119f compat/mingw: allow sigaction(SIGCHLD)
2: 05d945aa1e5 ! 3: c66bda461f4 daemon: use sigaction() to install child_handler()
@@ Commit message
In a future change, the flags used for processing SIGCHLD will need to
be updated, which is only possible by using sigaction().
- Factor out the call to set the signal handler and use sigaction instead
- of signal for the systems that allow that, which has the added benefit
- of using BSD semantics reliably and therefore not needing the rearming
- call.
+ Replace signal() with an equivalent invocation of sigaction(), which
+ has the added benefit of using BSD semantics reliably and therefore
+ not needing the rearming call in the signal handler.
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@xxxxxxxxx>
## daemon.c ##
@@ daemon.c: 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 MAYBE_UNUSED)
+ static void child_handler(int signo UNUSED)
{
/*
- * Otherwise empty handler because systemcalls will get interrupted
@@ daemon.c: static void handle(int incoming, struct sockaddr *addr, socklen_t addr
+ * upon signal receipt.
*/
- signal(SIGCHLD, child_handler);
-+#ifdef USE_NON_POSIX_SIGNAL
-+ /*
-+ * SysV needs the handler to be rearmed, but this is known
-+ * to trigger infinite recursion crashes at least in AIX.
-+ */
-+ signal(signo, child_handler);
-+#endif
}
static int set_reuse_addr(int sockfd)
@@ daemon.c: static void socksetup(struct string_list *listen_addr, int listen_port, struct s
- }
- }
-+#ifndef USE_NON_POSIX_SIGNAL
-+
-+static void set_signal_handler(struct sigaction *psa)
-+{
-+ sigemptyset(&psa->sa_mask);
-+ psa->sa_flags = SA_NOCLDSTOP | SA_RESTART;
-+ psa->sa_handler = child_handler;
-+ sigaction(SIGCHLD, psa, NULL);
-+}
-+
-+#else
-+
-+static void set_signal_handler(struct sigaction *psa UNUSED)
-+{
-+ signal(SIGCHLD, child_handler);
-+}
-+
static int service_loop(struct socketlist *socklist)
{
+ struct sigaction sa;
@@ daemon.c: static int service_loop(struct socketlist *socklist)
}
- signal(SIGCHLD, child_handler);
-+ set_signal_handler(&sa);
++ sigemptyset(&sa.sa_mask);
++ sa.sa_flags = SA_NOCLDSTOP | SA_RESTART;
++ sa.sa_handler = child_handler;
++ sigaction(SIGCHLD, &sa, NULL);
for (;;) {
check_dead_children();
3: b737e0389df ! 4: 851d663be0b daemon: explicitly allow EINTR during poll()
@@ Commit message
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@xxxxxxxxx>
## daemon.c ##
-@@ daemon.c: static void set_signal_handler(struct sigaction *psa)
- sigaction(SIGCHLD, psa, NULL);
+@@ daemon.c: static void socksetup(struct string_list *listen_addr, int listen_port, struct s
+ }
}
++#ifndef NO_RESTARTABLE_SIGNALS
++
+static void set_sa_restart(struct sigaction *psa, int enable)
+{
+ if (enable)
@@ daemon.c: static void set_signal_handler(struct sigaction *psa)
+ sigaction(SIGCHLD, psa, NULL);
+}
+
- #else
-
- static void set_signal_handler(struct sigaction *psa UNUSED)
-@@ daemon.c: static void set_signal_handler(struct sigaction *psa UNUSED)
- signal(SIGCHLD, child_handler);
- }
-
++#else
++
+static void set_sa_restart(struct sigaction *psa UNUSED, int enable UNUSED)
+{
+}
