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

[PATCH 2/5 maint] start_command: close cmd->err descriptor when fork/spawn fails

From
Johannes Sixt <j6t@kdbg.org>
Date
May 20, 2010, 18:57 UTC
Message-ID
<753dbe1c95461e736185f4a1b06e20937dab9db7.1274380838.git.j6t@kdbg.org>
In-Reply-To
<cover.1274380838.git.j6t@kdbg.org>
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>
---
 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.64.ga1799.dirty
Previous: Johannes SixtNext: Johannes Sixt
Message 3 of 7 in “Various fixes for Windows”
  1. 0/5 Various fixes for WindowsJohannes Sixt, May 20, 2010
  2. 1/5 Fix "Out of memory? mmap failed" for files larger than 4GB on WindowsJohannes Sixt, May 20, 2010
  3. 2/5 start_command: close cmd->err descriptor when fork/spawn failsJohannes Sixt, May 20, 2010
  4. 3/5 Fix checkout of large files to network shares on Windows XPJohannes Sixt, May 20, 2010
  5. 4/5 mingw: use _commit to implement fsyncJohannes Sixt, May 20, 2010
  6. 5/5 Recent MinGW has a C99 implementation of snprintf functionsJohannes Sixt, May 20, 2010
  7. Junio C HamanoMay 20, 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.