From: Johannes Sixt <j.sixt@viscovery.net>
Date: Mon, 26 Apr 2010 06:48:41 GMT
Subject: Re: [PATCH] start_command: close cmd->err descriptor when fork/spawn fails
Message-ID: <4BD53749.6040708@viscovery.net>
In-Reply-To: <1272244540-5960-1-git-send-email-dvornik+git@gmail.com>

Am 4/26/2010 3:15, schrieb bert Dvornik:
> 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.
> 
> I'm not sure why (or if) this problem hasn't cropped up under Linux.
> The non-Windows code *does* try to check for execve() failures in the
> child, in addition to the fork() failures.

The problem does show up on Linux if you trigger the right error, such as
with this patch:

diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 2de98e6..2515a96 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -528,7 +528,8 @@ test_expect_success 'push does not update local refs
 	mk_test heads/master &&
 	mk_child child &&
 	mkdir testrepo/.git/hooks &&
-	echo exit 1 >testrepo/.git/hooks/pre-receive &&
+	echo "#!/bin/frobnicuty" >testrepo/.git/hooks/pre-receive &&
+	echo exit 1 >>testrepo/.git/hooks/pre-receive &&
 	chmod +x testrepo/.git/hooks/pre-receive &&
 	(cd child &&
 		git pull .. master