From: Phillip Wood <phillip.wood123@gmail.com>
Date: Thu, 26 Jun 2025 13:14:42 GMT
Subject: Re: [PATCH v3 4/4] daemon: explicitly allow EINTR during poll()
Message-ID: <dba9ae0d-1e43-4345-a7ec-b57a07d45a07@gmail.com>
In-Reply-To: <851d663be0bda9ecb0e267ab0f85cc1f14cff10e.1750927989.git.gitgitgadget@gmail.com>

On 26/06/2025 09:53, Carlo Marcelo Arenas Belón via GitGitGadget wrote:
> 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 on those interruptions
> make sure to explicitly disable SA_RESTART around this function.

Given "git daemon" seems to have been designed with the assumption that 
long running system calls can fail with EINTR I don't know why this 
series is trying to use SA_RESTART in the first place.

Thanks

Phillip

> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>   daemon.c | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
> 
> diff --git a/daemon.c b/daemon.c
> index 155b2e180167..7e29c03e313f 100644
> --- a/daemon.c
> +++ b/daemon.c
> @@ -1116,6 +1116,25 @@ 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)
> +		psa->sa_flags |= SA_RESTART;
> +	else
> +		psa->sa_flags &= ~SA_RESTART;
> +	sigaction(SIGCHLD, psa, NULL);
> +}
> +
> +#else
> +
> +static void set_sa_restart(struct sigaction *psa UNUSED, int enable UNUSED)
> +{
> +}
> +
> +#endif
> +
>   static int service_loop(struct socketlist *socklist)
>   {
>   	struct sigaction sa;
> @@ -1136,6 +1155,7 @@ static int service_loop(struct socketlist *socklist)
>   	for (;;) {
>   		check_dead_children();
>   
> +		set_sa_restart(&sa, 0);
>   		if (poll(pfd, socklist->nr, -1) < 0) {
>   			if (errno != EINTR) {
>   				logerror("Poll failed, resuming: %s",
> @@ -1144,6 +1164,7 @@ static int service_loop(struct socketlist *socklist)
>   			}
>   			continue;
>   		}
> +		set_sa_restart(&sa, 1);
>   
>   		for (size_t i = 0; i < socklist->nr; i++) {
>   			if (pfd[i].revents & POLLIN) {