Re: [PATCH 1/2] daemon: use timeout for uninterruptible poll

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Apr 13, 2018, 12:36 UTC
Message-ID: <nycvar.QRO.7.76.6.1804131433250.65@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz>
In-Reply-To: <20180412210757.7792-2-kgybels@infogroep.be>

Hi Kim,

On Thu, 12 Apr 2018, Kim Gybels wrote:

> The poll provided in compat/poll.c is not interrupted by receiving
> SIGCHLD. Use a timeout for cleaning up dead children in a timely manner.

Maybe say "When using this poll emulation, use a timeout ..."?

Show 9 quoted lines

> diff --git a/daemon.c b/daemon.c
> index fe833ea7de..6dc95c1b2f 100644
> --- a/daemon.c
> +++ b/daemon.c
> @@ -1147,6 +1147,7 @@ static int service_loop(struct socketlist *socklist)
>  {
>  	struct pollfd *pfd;
>  	int i;
> +	int poll_timeout = -1;

Just reuse the line above:

	int poll_timeout = -1, i;

Show 13 quoted lines

> @@ -1161,8 +1162,13 @@ static int service_loop(struct socketlist *socklist)
>  		int i;
>  
>  		check_dead_children();
> -
> -		if (poll(pfd, socklist->nr, -1) < 0) {
> +#ifdef NO_POLL
> +		poll_timeout = live_children ? 100 : -1;
> +#endif
> +		int ret = poll(pfd, socklist->nr, poll_timeout);
> +		if  (ret == 0) {
> +			continue;
> +		} else if (ret < 0) {

I would find it a bit easier on the eyes if this did not use curlies, and dropped the unnecessary `else` (`continue` will take care of that):

		if (!ret)
			continue;
		if (ret < 0)
			[...]

Thank you for working on this!

Ciao, Dscho