When git runs as PID 1 inside a container and exits via exit(128) on transport errors, child processes (git-remote-https, ssh, proxy) are never waited on because the normal cleanup paths (disconnect_helper, finish_connect) are bypassed. Since PID 1 has no parent to reap its children, these become zombies that persist for the container's lifetime.

This patch registers atexit handlers in transport-helper.c and connect.c to ensure children are reaped on any exit path, and clears them on the normal cleanup paths to avoid double-waiting.

Blog post with detailed investigation: https://cshung.github.io/posts/zombie-git/

Andrew Au (1):
  transport-helper, connect: add atexit handler to reap children on
    abnormal exit

 connect.c          | 17 +++++++++++++++++
 transport-helper.c | 11 +++++++++++
 2 files changed, 28 insertions(+)

From: Andrew Au <3410332+cshung@users.noreply.github.com>

When git exits via exit(128) on transport errors, child processes (git-remote-https, ssh, proxy) are never waited on because the normal cleanup paths (disconnect_helper, finish_connect) are bypassed. When git is PID 1 in a container, these un-reaped children become zombies.

Register atexit handlers in both transport-helper.c and connect.c to ensure children are reaped on any exit path. Clear the handlers on the normal cleanup paths to avoid double-waiting.

Signed-off-by: Andrew Au <cshung@gmail.com>
---
 connect.c          | 17 +++++++++++++++++
 transport-helper.c | 11 +++++++++++
 2 files changed, 28 insertions(+)

On Mon, Feb 23, 2026, at 17:51, Andrew Au wrote:

This email here,

has to match with the email here. Or probably vice versa since a real email address is better than a GitHub noreply email in this context.

Maybe this was just an automatic don’t-use-my-real-email-address thing that GitHub did?

Yes, this is forced by the GitHub don't use my real email address option. I won't be allowed to push if I used my real address there.

I can change the commit author to use my real email address if that's preferred for upstream.

On Mon, Feb 23, 2026 at 9:15 AM Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com> wrote:

When git exits via exit(128) on transport errors, child processes (git-remote-https, ssh, proxy) are never waited on because the normal cleanup paths (disconnect_helper, finish_connect) are bypassed. When git is PID 1 in a container, these un-reaped children become zombies.

Register atexit handlers in both transport-helper.c and connect.c to ensure children are reaped on any exit path. Clear the handlers on the normal cleanup paths to avoid double-waiting.

Signed-off-by: Andrew Au <cshung@gmail.com>
---
 connect.c          | 17 +++++++++++++++++
 transport-helper.c | 11 +++++++++++
 2 files changed, 28 insertions(+)

Andrew Au <cshung@gmail.com> writes:

Could you tell me more about the real use case behind such a set-up.

These children become zombies, and then what will be done to the container that lost the "git" process, running of which presumably was the primary reason why the container was brought up in the first place? Wouldn't these zombies go away when the container that finished its sole purpose of running "git" gets dismantled?

Thanks.

Thank you for the feedback.

The use case is a long-running service running as PID 1 inside a container. The service continuously spawns git to detect repository changes — it is not a one-shot container where git itself is the primary process. Because the service is meant to stay alive indefinitely, any zombies git leaves behind accumulate over time rather than being cleaned up when the container exits.

In my specific case, I observed over 6,500 zombie processes before identifying this as the root cause. The blog post linked in the cover letter documents the investigation in detail.

The fix ensures git cleans up its own children on abnormal exit paths, which is the right behavior regardless of whether the parent is PID 1 or not.

On Wed, Mar 11, 2026 at 10:58 AM Junio C Hamano <gitster@pobox.com> wrote:

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

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

Andrew Au <cshung@gmail.com> writes:

Wait, the proposed log message said "Git is the pid 1", and now the above is "there is some long-running service that runs Git". Which one is it? Wouldn't that long-running service be the right process that should reap these unwaited children? Or is "git" used as that long-running service somehow?

On 2026-02-23 at 16:51:46, Andrew Au wrote:

Usually people use a tool like tini as PID 1 in containers, which allows that process to handle process reaping while still exec'ing the normal Git or other command. It's the case that _most_ processes are not designed to run properly as process 1 in a container or otherwise, so a specialized init-capable helper program is usually a good idea.

Even with this series to fix some of the process reaping problems, I expect you'll find other cases in Git where we don't always reap processes correctly as well, so an init helper would still be useful.

Thank you for the suggestion. You are right that tini is a common and reasonable approach, and I will look into using it regardless.

That said, I think fixing git to clean up its own children is still worthwhile — processes should not leave zombies on abnormal exit paths whether or not an init helper is present. The two approaches are complementary rather than mutually exclusive.

On Wed, Mar 11, 2026 at 2:15 PM brian m. carlson <sandals@crustytoothpaste.net> wrote:

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>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
Thanks to Jeff King for suggesting the clean_on_exit approach,
which is simpler and avoids potential deadlocks from the atexit
handler in v2. Also thanks to Junio for catching the inaccurate
description of the PID 1 scenario.

 connect.c          | 4 ++++
 transport-helper.c | 2 ++
 2 files changed, 6 insertions(+)

On Thu, Mar 12, 2026 at 07:55:59PM +0000, Andrew Au wrote:

Ah, right, I forgot about wait_after_clean when I suggested clean_on_exit. Yes, you definitely want both here.

I don't know if we have established a pattern in the project for AI-assisted work (and whether it is worth marking at all, and if so, if co-author is the right way to do so). But if we are going to, usually the trailers would all be together, with no blank line in between.

The changes here are all a trivial implementation of the idea. So if the idea is good (and I think it is), then the patch looks correct to me.

-Peff

On Thu, Mar 12, 2026 at 04:40:24PM -0400, Jeff King wrote:

Trailers aside, we definitely don't want it in the mail's cc headers, as that address bounces (unsurprisingly).

-Peff

Andrew Au <cshung@gmail.com> writes:

Do not throw a blank line inside your trailer block. In addition, we do not want somebody who cannot stand beind the patch and certify DCO listed there. See also SubmittingPatches::[ai].

This version looks quite simple and clean, taking advantage of existing machinery to clean these processes up.

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(+)

Andrew Au <cshung@gmail.com> writes:

Thanks, queued.

On Thu, Mar 12, 2026 at 03:04:17PM -0700, Junio C Hamano wrote:

Curiously this patch seems to cause a failure in one of the CI leak jobs, but I don't think it's the culprit. See below for a fix and explanation.

I don't know if you want to apply it separately (since it's really a totally different topic) or on top (since it is only the application of Andrew's patch which lets us find the problem).

-- >8 --
Subject: [PATCH] transport: plug leaks in transport_color_config()

We retrieve config values with repo_config_get_string(), which will allocate a new copy of the string for us. But we don't hold on to those strings, since they are just fed to git_config_colorbool() and color_parse(). But nor do we free them, which means they leak.

We can fix this by using the "_tmp" form of repo_config_get_string(), which just hands us a pointer directly to the internal storage. This is OK for our purposes, since we don't need it to last for longer than our parsing calls.

Two interesting side notes here:

  1. Many types already have a repo_config_get_X() variant that handles
     this for us (e.g., repo_config_get_bool()). But neither colorbools
     nor colors themselves have such helpers. We might think about
     adding them, but converting all callers is a larger task, and out
     of scope for this fix.

  2. As far as I can tell, this leak has been there since 960786e761
     (push: colorize errors, 2018-04-21), but wasn't detected by LSan in
     our test suite. It started triggering when we applied dd3693eb08
     (transport-helper, connect: use clean_on_exit to reap children on
     abnormal exit, 2026-03-12) which is mostly unrelated.

     Even weirder, it seems to trigger only with clang (and not gcc),
     and only with GIT_TEST_DEFAULT_REF_FORMAT=reftable. So I think this
     is another odd case where the pointers happened to be hanging
     around in stack memory, but changing the pattern of function calls
     in nearby code was enough for them to be incidentally overwritten.

Signed-off-by: Jeff King <peff@peff.net>
---
 transport.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Jeff King <peff@peff.net> writes:

That certainly is an interesting #leftoverbits project.

The patch looks good.  Will queue.  Thanks.

Jeff King <peff@peff.net> writes:

Regardless of where it goes, we need to change a bit more, it seems?

    CC transport.o
transport.c: In function 'transport_color_config':
transport.c:57:62: error: passing argument 3 of 'repo_config_get_string_tmp' from incompatible pointer type [-Wincompatible-pointer-types]
   57 |         if (!repo_config_get_string_tmp(the_repository, key, &value))
      |                                                              ^~~~~~
      |                                                              |
      |                                                              char **
In file included from transport.c:5:
config.h:644:62: note: expected 'const char **' but argument is of type 'char **'
  644 |                                const char *key, const char **dest);
      |                                                 ~~~~~~~~~~~~~^~~~
transport.c:64:74: error: passing argument 3 of 'repo_config_get_string_tmp' from incompatible pointer type [-Wincompatible-pointer-types]
   64 |                 if (!repo_config_get_string_tmp(the_repository, keys[i], &value)) {
      |                                                                          ^~~~~~
      |                                                                          |
      |                                                                          char **
config.h:644:62: note: expected 'const char **' but argument is of type 'char **'
  644 |                                const char *key, const char **dest);
      |                                                 ~~~~~~~~~~~~~^~~~
gmake: *** [Makefile:2815: transport.o] Error 1

I'll squash an obvious patch in.

 transport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On Mon, Mar 16, 2026 at 01:31:08PM -0700, Junio C Hamano wrote:

Huh. Obviously yes, but how did I manage to bungle this so badly?

I _think_ what happened is that I compiled the topic without -Werror, because of all of the -Wdiscarded-qualifier errors that happen on 'master' with a recent glibc.

My integration cycle would have caught it, since I have another topic with fixes for the discarded-qualifier issue. But I hadn't run one yet. ;)

None of that is important for you, but just wondering if there was something more subtle going on, or if I just screwed up something as simple as typing 'make'.

Yep, that looks good. Thanks for fixing.

-Peff

Jeff King <peff@peff.net> writes:

Thanks. It is probably a good thing that you are using toolchain ahead of the version I use. We catch different kind of errors that way, even though occasionally we see a gotcha like this one.