Skip to content

Commit fc012c2

Browse files
bert Dvornikgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Johannes Sixt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 60890cc commit fc012c2

File tree

2 files changed

+3
-1
lines changed

2 files changed

+3
-1
lines changed

run-command.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,8 @@ int start_command(struct child_process *cmd)
383383
close(cmd->out);
384384
if (need_err)
385385
close_pair(fderr);
386+
else if (cmd->err)
387+
close(cmd->err);
386388
errno = failed_errno;
387389
return -1;
388390
}

t/t5516-fetch-push.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,7 @@ test_expect_success 'push does not update local refs on failure' '
528528
mk_test heads/master &&
529529
mk_child child &&
530530
mkdir testrepo/.git/hooks &&
531-
echo exit 1 >testrepo/.git/hooks/pre-receive &&
531+
echo "#!/no/frobnication/today" >testrepo/.git/hooks/pre-receive &&
532532
chmod +x testrepo/.git/hooks/pre-receive &&
533533
(cd child &&
534534
git pull .. master

0 commit comments

Comments
 (0)