git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 096/104] start_command: close cmd->err descriptor when fork/spawn fails

From
Sam Vilain <sam.vilain@catalyst.net.nz>
Date
May 26, 2010, 06:01 UTC
Message-ID
<1274853674-18521-96-git-send-email-sam.vilain@catalyst.net.nz>
In-Reply-To
<1274853674-18521-1-git-send-email-sam.vilain@catalyst.net.nz>
From: bert Dvornik <dvornik+git@gmail.com>

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>
---
 run-command.c         |    2 ++
 t/t5516-fetch-push.sh |    2 +-
 2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/run-command.c b/run-command.c
index eb5c575..c7793f5 100644
--- a/run-command.c
+++ b/run-command.c
@@ -383,6 +383,8 @@ fail_pipe:
 			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 2de98e6..6a37a4d 100755
--- 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
-- 
1.7.1.rc2.333.gb2668
Previous: Sam VilainNext: Sam Vilain
Message 14 of 23 in “tests: chmod +x t5150”
  1. 075/104 tests: chmod +x t5150Sam Vilain, May 26, 2010
  2. 076/104 t7604-merge-custom-message: shift expected output creationSam Vilain, May 26, 2010
  3. 082/104 fmt-merge-msg: add function to append shortlog onlySam Vilain, May 26, 2010
  4. 084/104 autocrlf: Make it work also for un-normalized repositoriesSam Vilain, May 26, 2010
  5. 086/104 gitweb: Use @diff_opts while using format-patchSam Vilain, May 26, 2010
  6. 087/104 hash_object: correction for zero length fileSam Vilain, May 26, 2010
  7. 088/104 for-each-ref: Field with abbreviated objectnameSam Vilain, May 26, 2010
  8. 090/104 Documentation: rebase -i ignores options passed to "git am"Sam Vilain, May 26, 2010
  9. 091/104 Documentation: fix minor inconsistencySam Vilain, May 26, 2010
  10. 092/104 Documentation/gitdiffcore: fix order in pickaxe descriptionSam Vilain, May 26, 2010
  11. 093/104 post-receive-email: document command-line modeSam Vilain, May 26, 2010
  12. 094/104 diff: fix coloring of extended diff headersSam Vilain, May 26, 2010
  13. 095/104 Fix "Out of memory? mmap failed" for files larger than 4GB on WindowsSam Vilain, May 26, 2010
  14. 096/104 start_command: close cmd->err descriptor when fork/spawn failsSam Vilain, May 26, 2010
  15. 097/104 Fix checkout of large files to network shares on Windows XPSam Vilain, May 26, 2010
  16. 098/104 mingw: use _commit to implement fsyncSam Vilain, May 26, 2010
  17. 099/104 Recent MinGW has a C99 implementation of snprintf functionsSam Vilain, May 26, 2010
  18. 100/104 Complete prototype of git_config_from_parameters()Sam Vilain, May 26, 2010
  19. 101/104 test get_git_work_tree() return value for NULLSam Vilain, May 26, 2010
  20. 102/104 t7502-commit: fix spellingSam Vilain, May 26, 2010
  21. 103/104 show-branch: use DEFAULT_ABBREV instead of 7Sam Vilain, May 26, 2010
  22. 104/104 Documentation/SubmittingPatches: clarify GMail section and SMTPSam Vilain, May 26, 2010
  23. Sverre RabbelierMay 26, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.