Revert "transport-helper, connect: use clean_on_exit to reap children on abnormal...

This reverts commit dd3693eb0859274d62feac8047e1d486b3beaf31.

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>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/connect.c b/connect.c
index fcd35c5539a76e4f694f8392615de262c5b40c6d..a02583a102724120689139fd7795ff965e5c2263 100644 (file)
--- a/connect.c
+++ b/connect.c
@@ -1054,8 +1054,6 @@ 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 */
@@ -1517,8 +1515,6 @@ 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 570d7c6439569a96136b6151ff28b7f5afa2aa7b..4d95d84f9e4d05db5117016bcdacadf3a4fe46b2 100644 (file)
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -154,8 +154,6 @@ 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);