Skip to content

Commit 9f03fac

Browse files
committed
Revert "transport-helper, connect: use clean_on_exit to reap children on abnormal exit"
This reverts commit dd3693e. The goal of that commit was to avoid zombie child processes hanging around when the parent git process is killed. But it doesn't quite work when the child command is run by the shell: 1. If there is a shell, then we kill and wait for the shell, not the process spawned by the shell. And so the child process, even if it eventually exits, will hang around as a zombie forever. And this is true of most (all?) shells: bash, dash, etc. So we are not really accomplishing our goal in the first place. 2. Not all shells will exit immediately upon receiving a signal. In particular, mksh will wait for its children to exit (but not actually propagate the signal to them!) leaving us with a potential deadlock: git is wait()ing on mksh, which is wait()ing on a child process, but that child process is waiting on git to produce more input (or EOF) over a pipe. You can see several examples of this deadlock in the test suite, for example by running: make SHELL_PATH=/bin/mksh cd t ./t5702-protocol-v2.sh Because this is a regression for mksh users, and because we did not achieve our goal even with other shells, let's revert the commit for now. If there is a more clever way of doing the same thing, we can consider applying it separately on top (or do nothing and just accept the zombies and rely on PID 1 to reap them). Reported-by: Jan Palus <jpalus@fastmail.com>
1 parent 94f0577 commit 9f03fac

2 files changed

Lines changed: 0 additions & 6 deletions

File tree

connect.c

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,8 +1054,6 @@ static struct child_process *git_proxy_connect(int fd[2], char *host)
10541054
strvec_push(&proxy->args, port);
10551055
proxy->in = -1;
10561056
proxy->out = -1;
1057-
proxy->clean_on_exit = 1;
1058-
proxy->wait_after_clean = 1;
10591057
if (start_command(proxy))
10601058
die(_("cannot start proxy %s"), git_proxy_command);
10611059
fd[0] = proxy->out; /* read from proxy stdout */
@@ -1517,8 +1515,6 @@ struct child_process *git_connect(int fd[2], const char *url,
15171515
}
15181516
strvec_push(&conn->args, cmd.buf);
15191517

1520-
conn->clean_on_exit = 1;
1521-
conn->wait_after_clean = 1;
15221518
if (start_command(conn))
15231519
die(_("unable to fork"));
15241520

transport-helper.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,6 @@ static struct child_process *get_helper(struct transport *transport)
154154

155155
helper->trace2_child_class = helper->args.v[0]; /* "remote-<name>" */
156156

157-
helper->clean_on_exit = 1;
158-
helper->wait_after_clean = 1;
159157
code = start_command(helper);
160158
if (code < 0 && errno == ENOENT)
161159
die(_("unable to find remote helper for '%s'"), data->name);

0 commit comments

Comments
 (0)