Re: [PATCH v4] transport-helper, connect: use clean_on_exit to reap children on abnormal exit
Andrew Au <cshung@gmail.com> writes:
Show 23 quoted lines
> When a long-running service (e.g., a source indexer) runs as PID 1
> inside a container and repeatedly spawns git, git may in turn spawn
> child processes such as git-remote-https or ssh. If git exits abnormally
> (e.g., via exit(128) on a transport error), the normal cleanup paths
> (disconnect_helper, finish_connect) are bypassed, and these children are
> never waited on. The children are reparented to PID 1, which does not
> reap them, so they accumulate as zombies over time.
>
> Set clean_on_exit and wait_after_clean on child_process structs in both
> transport-helper.c and connect.c so that the existing run-command
> cleanup infrastructure handles reaping on any exit path. This avoids
> rolling custom atexit handlers that call finish_command(), which could
> deadlock if the child is blocked waiting for the parent to close a pipe.
>
> The clean_on_exit mechanism sends SIGTERM first, then waits, ensuring
> the child terminates promptly. It also handles signal-based exits, not
> just atexit.
>
> Signed-off-by: Andrew Au <cshung@gmail.com>
> ---
> connect.c | 4 ++++
> transport-helper.c | 2 ++
> 2 files changed, 6 insertions(+)
Show 36 quoted lines
>
> diff --git a/connect.c b/connect.c
> index eef752f14..5039adca7 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -989,6 +989,8 @@ static struct child_process *git_proxy_connect(int fd[2], char *host)
> strvec_push(&proxy->args, port);
> proxy->in = -1;
> proxy->out = -1;
> + proxy->clean_on_exit = 1;
> + proxy->wait_after_clean = 1;
> if (start_command(proxy))
> die(_("cannot start proxy %s"), git_proxy_command);
> fd[0] = proxy->out; /* read from proxy stdout */
> @@ -1447,6 +1449,8 @@ struct child_process *git_connect(int fd[2], const char *url,
> }
> strvec_push(&conn->args, cmd.buf);
>
> + conn->clean_on_exit = 1;
> + conn->wait_after_clean = 1;
> if (start_command(conn))
> die(_("unable to fork"));
>
> diff --git a/transport-helper.c b/transport-helper.c
> index e95267a4a..6633a999b 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -140,6 +140,8 @@ static struct child_process *get_helper(struct transport *transport)
>
> helper->trace2_child_class = helper->args.v[0]; /* "remote-<name>" */
>
> + helper->clean_on_exit = 1;
> + helper->wait_after_clean = 1;
> code = start_command(helper);
> if (code < 0 && errno == ENOENT)
> die(_("unable to find remote helper for '%s'"), data->name);