start_command: close cmd->err descriptor when fork/spawn fails

Fix the problem where the cmd->err passed into start_command wasn't
being properly closed when certain types of errors occurr.  (Compare
the affected code with the clean shutdown code later in the function.)

On Windows, this problem would be triggered if mingw_spawnvpe()
failed, which would happen if the command to be executed was malformed
(e.g. a text file that didn't start with a #! line).  If cmd->err was
a pipe, the failure to close it could result in a hang while the other
side was waiting (forever) for either input or pipe close, e.g. while
trying to shove the output into the side band.  On msysGit, this
problem was causing a hang in t5516-fetch-push.

[J6t: With a slight adjustment of the test case, the hang is also
observed on Linux.]

Signed-off-by: bert Dvornik <dvornik+git@gmail.com>
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/run-command.c b/run-command.c
index eb5c57562909c1e505c2a7688bad6dbeea4b96a0..c7793f50fbe0a43495c2b2d36a47c0b5aac37483 100644 (file)
--- a/run-command.c
+++ b/run-command.c
@@ -383,6 +383,8 @@ int start_command(struct child_process *cmd)
                        close(cmd->out);
                if (need_err)
                        close_pair(fderr);
+               else if (cmd->err)
+                       close(cmd->err);
                errno = failed_errno;
                return -1;
        }

diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 2de98e6561607b87bceef66c20ad9055d04878c3..6a37a4d993df3fa4958a719fdfdb82f0dd2de623 100755 (executable)
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -528,7 +528,7 @@ test_expect_success 'push does not update local refs on failure' '
        mk_test heads/master &&
        mk_child child &&
        mkdir testrepo/.git/hooks &&
-       echo exit 1 >testrepo/.git/hooks/pre-receive &&
+       echo "#!/no/frobnication/today" >testrepo/.git/hooks/pre-receive &&
        chmod +x testrepo/.git/hooks/pre-receive &&
        (cd child &&
                git pull .. master