Re: [PATCH] run-command: fix segfault when cleaning forked async process
- From
- Brandon Williams <bmwill@google.com>
- Date
- Mar 17, 2017, 23:38 UTC
- Message-ID
- <20170317233806.GG63813@google.com>
- In-Reply-To
- <20170317232004.xqs5f2q2swa3b5bq@sigill.intra.peff.net>
On 03/17, Jeff King wrote:
Show 44 quoted lines
> 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