Re: [i-g-t,1/2] lib/igt_core: Check if helper proc was alive before kill()

[Daniel Charles <daniel.charles@intel.com>]

On 10/7/2026 7:16 AM, Gustavo Sousa wrote:
> The function igt_stop_helper() does a sanity check by verifying if the
> helper process died "too early", however it does that by checking if
> it was terminated due to not handling the signal used in the kill()
> call.
>
> That logic has a flaw though: it the helper process gracefully handles
> the signal sent by kill() and exits normally, igt_stop_helper() will
> mistakenly take that as dying too early, because WIFSIGNALED(status)
> will be false.
>
> That's the case with the pw-reserve process: in an upcoming change we
> will change pipewire_reserve_wait() to call it with execlp().  Because
> it handles SIGTERM and exits normally, a warning and assertion failure
> would happen with the current logic.
>
> Fix that by checking if the process was alive before calling kill().
>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
>   lib/igt_core.c | 20 +++++++++-----------
>   1 file changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 8ee9432923e6..d9949ea131f8 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -2652,13 +2652,6 @@ int igt_wait_helper(struct igt_helper_process *proc)
>   	return status;
>   }
>   
> -static bool helper_was_alive(struct igt_helper_process *proc,
> -			     int status)
> -{
> -	return (WIFSIGNALED(status) &&
> -		WTERMSIG(status) == (proc->use_SIGKILL ? SIGKILL : SIGTERM));
> -}
> -
>   /**
>    * igt_stop_helper:
>    * @proc: #igt_helper_process structure
> @@ -2670,17 +2663,22 @@ static bool helper_was_alive(struct igt_helper_process *proc,
>   void igt_stop_helper(struct igt_helper_process *proc)
>   {
>   	int status;
> +	int ret;
>   
>   	if (!proc->running) /* never even started */
>   		return;
>   
> +	ret = waitpid(proc->pid, &status, WNOHANG);
put waitpid in a do/while loop to catch any EINT return that may need a 
retry.
> +
> +	if (ret == proc->pid)
> +		igt_debug("Helper died too early with status=%d\n", status);
> +
>   	/* failure here means the pid is already dead and so waiting is safe */
>   	kill(proc->pid, proc->use_SIGKILL ? SIGKILL : SIGTERM);
>   
> -	status = igt_wait_helper(proc);
> -	if (!helper_was_alive(proc, status))
> -		igt_debug("Helper died too early with status=%d\n", status);
> -	assert(helper_was_alive(proc, status));
> +	assert(ret == 0);
This assert can be moved before kill call to make sure the correct 
process is killed.

-- Daniel.

> +
> +	igt_wait_helper(proc);
>   }
>   
>   static void children_exit_handler(int sig)