"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
Re: gotd: Don't use WNOHANG when waiting for child processes
To:
Josiah Frentsos <jfrent@tilde.team>
Cc:
gameoftrees@openbsd.org
Date:
Sat, 04 Mar 2023 11:58:46 +0100

Download raw body.

Thread
On 2023/03/03 09:35:59 -0500, Josiah Frentsos <jfrent@tilde.team> wrote:
> Fixes this:
> 
>     2023-03-03T13:51:15.155Z silicon gotd[7715]: child PID 0 terminated; signal 12
>     2023-03-03T13:51:15.186Z silicon last message repeated 419 times

I don't think the diff is wrong per-se, wait_for_child is just a
wrapper around waitpid and we're interested in one child only anyway,
so I *think* WNOHANG is not much important.  I'm curious why it's
needed though.

waitpid(WNOHANG) returns 0 when "there are no stopped or exited
children".  Since we call it always after kill_proc() maybe it's a
timing issue: we call waitpid(WNOHANG) before the signal is handled
and the other process exits?

> diff 6221bdf771a3f2638d2aca0a58162261424f2ca6 2a511b1a88651c9b58f9390624dc29312b627b5e
> commit - 6221bdf771a3f2638d2aca0a58162261424f2ca6
> commit + 2a511b1a88651c9b58f9390624dc29312b627b5e
> blob - 86f2b6013f0c53e31f8adbc89cd954b062c2ee0b
> blob + c74da7c7b48e76509e16e1ed0428b5e9b6d53251
> --- gotd/gotd.c
> +++ gotd/gotd.c
> @@ -276,15 +276,15 @@ wait_for_child(pid_t child_pid)
>  	    (long)child_pid);
>  
>  	do {
> -		pid = waitpid(child_pid, &status, WNOHANG);
> +		pid = waitpid(child_pid, &status, 0);
>  		if (pid == -1) {
> -			if (errno != EINTR && errno != ECHILD)
> -				fatal("wait");
> +			if (errno != EINTR)
> +				fatal("waitpid");
>  		} else if (WIFSIGNALED(status)) {
>  			log_warnx("child PID %ld terminated; signal %d",
>  			    (long)pid, WTERMSIG(status));
>  		}
> -	} while (pid != -1 || (pid == -1 && errno == EINTR));
> +	} while (pid == -1 && errno == EINTR);
>  }
>  
>  static void