Re: [PATCH] run-command: fix segfault when cleaning forked async process

[Brandon Williams <bmwill@google.com>]

On 03/17, Jeff King wrote:
> Callers of the run-command API may mark a child as
> "clean_on_exit"; it gets added to a list and killed when the
> main process dies.  Since commit 46df6906f
> (execv_dashed_external: wait for child on signal death,
> 2017-01-06), we respect an extra "wait_after_clean" flag,
> which we expect to find in the child_process struct.
> 
> When Git is built with NO_PTHREADS, we start "struct
> async" processes by forking rather than spawning a thread.
> The resulting processes get added to the cleanup list but
> they don't have a child_process struct, and the cleanup
> function ends up dereferencing NULL.
> 
> We should notice this case and assume that the processes do
> not need to be waited for (i.e., the same behavior they had
> before 46df6906f).
> 
> Reported-by: Brandon Williams <bmwill@google.com>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> This is a regression in v2.12.0, though there is no hurry to get it into
> v2.12.1 unless your grep patches go in, too. Without them you can't
> actually build with NO_PTHREADS anyway.
> 
> However, applied directly on top of 46df6906f (which predates the build
> breakage), you can easily see the test failures with NO_PTHREADS and
> that this fixes them.
> 
>  run-command.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/run-command.c b/run-command.c
> index 5227f78ae..574b81d3e 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -48,7 +48,7 @@ static void cleanup_children(int sig, int in_signal)
>  
>  		kill(p->pid, sig);
>  
> -		if (p->process->wait_after_clean) {
> +		if (p->process && p->process->wait_after_clean) {
>  			p->next = children_to_wait_for;
>  			children_to_wait_for = p;
>  		} else {

Looks good to me! Thanks for tracking that down so quickly.  I'm glad it
was a quick fix.

-- 
Brandon Williams