[PATCH v4 0/2] daemon: explicitly allow EINTR during poll()

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

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@gmail.com>
      
       ## 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@gmail.com>
      
     @@ 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