This series addresses and ambiguity that is at least visible in OpenBSD, where zombie proceses would only be cleared after a new connection is received. The underlying problem is that when this code was originally introduced, SA_RESTART was not widely implemented, and the signal() call usually implemented SysV like semantics, at least until it started being reimplemented by calling sigaction() internally. Changes since v3 * Remove patches 1 and 4 as suggested by Phillip, and setup the signal without SA_RESTART instead. 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 (2): compat/mingw: allow sigaction(SIGCHLD) daemon: use sigaction() to install child_handler() compat/mingw-posix.h | 1 + compat/mingw.c | 4 +++- daemon.c | 12 +++++++----- 3 files changed, 11 insertions(+), 6 deletions(-) base-commit: cb3b40381e1d5ee32dde96521ad7cfd68eb308a6 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2002%2Fcarenas%2Fsiginterrupt-v4 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2002/carenas/siginterrupt-v4 Pull-Request: https://github.com/git/git/pull/2002 Range-diff vs v3: 1: ae1ca6bb2b2 < -: ----------- compat/posix.h: track SA_RESTART fallback 2: 3f63479119f ! 1: f21e8ff5c9d compat/mingw: allow sigaction(SIGCHLD) @@ Commit message A future change will start using sigaction to setup a SIGCHLD signal handler. - The current code uses signal() which returns SIG_ERR (but doesn't + The current code uses signal(), which returns SIG_ERR (but doesn't seem to set errno) so instruct sigaction() to do the same. + A new SA flag will be needed, so copy the one from Cygwinr; note that + the sigacgtion() implementation that is provided won't use it, so + its value is otherwise irrelevant. + Signed-off-by: Carlo Marcelo Arenas Belón <carenas@xxxxxxxxx> ## compat/mingw-posix.h ## @@ compat/mingw-posix.h: struct sigaction { - sig_handler_t sa_handler; unsigned sa_flags; }; + #define SA_RESTART 0 +#define SA_NOCLDSTOP 1 struct itimerval { 3: c66bda461f4 ! 2: 81c41b43e60 daemon: use sigaction() to install child_handler() @@ Metadata ## Commit message ## daemon: use sigaction() to install child_handler() - In a future change, the flags used for processing SIGCHLD will need to - be updated, which is only possible by using sigaction(). + Replace signal() with an equivalent invocation of sigaction(), but + make sure to NOT set SA_RESTART so the original code that expects + to be interrupted when children complete still works as designed. - 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. + This change has the added benefit of using BSD signal 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: static int service_loop(struct socketlist *socklist) - signal(SIGCHLD, child_handler); + sigemptyset(&sa.sa_mask); -+ sa.sa_flags = SA_NOCLDSTOP | SA_RESTART; ++ sa.sa_flags = SA_NOCLDSTOP; + sa.sa_handler = child_handler; + sigaction(SIGCHLD, &sa, NULL); 4: 851d663be0b < -: ----------- daemon: explicitly allow EINTR during poll() -- gitgitgadget
