Subject: Re: [PATCH v3 3/4] daemon: use sigaction() to install child_handler()

From: Junio C Hamano <[email-masked]>
Date: Thu, 26 Jun 2025 08:33:29 -0700

Phillip Wood <[email-masked]> writes:

> Only because you have chosen to use SA_RESTART here. I think it would
> be better to drop that and instead say something like
>
>
> POSIX leaves several aspects of the behavior of signal() up to the
> implementer. It is unspecified whether long running syscalls are
> restarted after an interrupt is received.  The event loop in "git
> daemon" assumes that poll() will return with EINTR if a child exits
> while it is polling but if poll() is restarted after an interrupt is
> received that does not happen which can lead to a build up of
> uncollected zombie processes.
>
> It is also unspecified whether the handler is reset after an interrupt
> is received. In order to work correctly on operating systems such as
> Solaris where the handler for SIGCLD is reset when a signal is
> received "git daemon" calls signal() from the SIGCLD handler to
> re-establish a handler for that signal. Unfortunately this causes
> infinite recursion on other operating systems such as AIX.
>
> Both of these problems can be addressed by using sigaction() instead
> of signal() which has much stricter semantics and so, by setting the
> appropriate flags, we can rely on poll() being interrupted and the
> SIGCLD handler not being reset. This change means that all long
> running system calls could fail with EINTR not just pall() but rest of
> the event loop code is designed to gracefully handle that.
>
> After this change there is still a race where a child that exits after
> it has been checked in check_dead_children() but before we call poll()
> will not be collected until a new connection is received or a child
> exits while we're polling. We could fix this by using the "self-pipe
> trick" but do not because ....
>
>
> Then we can drop patches 1 and 4.

Thanks for a suggestion.  I really like the "everybody in these code
paths are prepared to receive and handle EINTR just fine, so we do
not have to do the SA_RESTART" observation very much.