Re: [PATCH v2] transport-helper, connect: add atexit handler to reap children on abnormal exit

From: Jeff King <peff@peff.net>
Date: Mar 11, 2026, 18:42 UTC
Message-ID: <20260311184206.GA1911377@coredump.intra.peff.net>
In-Reply-To: <20260311142021.3464789-1-cshung@gmail.com>

On Wed, Mar 11, 2026 at 02:20:21PM +0000, Andrew Au wrote:

Show 11 quoted lines

> +/*
> + * Ensure the connection child (ssh, proxy, or local git) is reaped on
> + * any exit path, mirroring the transport-helper.c atexit pattern.
> + */
> +static struct child_process *conn_to_reap;
> +
> +static void cleanup_conn_on_exit(void)
> +{
> +	if (conn_to_reap)
> +		finish_command(conn_to_reap);
> +}

This waits for the command to exit. Are we sure it will always do so, and it won't sometimes be waiting on us to do something (like close a pipe that is feeding it)? If not, then we can get deadlocks.

I think you actually want to kill(), then wait. There is already support for this in run-command.[ch]. You just need to set the clean_on_exit flag of the child_process struct.

I actually wonder if clean_on_exit should become the default behavior. It should be rare for our subprocesses to outlive us. Commit afe19ff7b5 (run-command: optionally kill children on exit, 2012-01-07) mentions the pager, but I don't think that was true even back then (we wait around for the pager to finish). There are a few cases where we spawn daemon programs, which would need to be marked as survivable. I think mostly we have not looked into it because somebody would have to look at each run_command() callsite.

Anyway, that is a bit of a tangent. I think it would be safe to mark the spots in this patch as clean_on_exit.

-Peff