git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Sixt <j.sixt@viscovery.net>
To: bert Dvornik <dvornik+git@gmail.com>
Cc: git@vger.kernel.org, msysgit@googlegroups.com,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH] start_command: close cmd->err descriptor when fork/spawn fails
Date: Mon, 26 Apr 2010 08:48:41 +0200	[thread overview]
Message-ID: <4BD53749.6040708@viscovery.net> (raw)
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

  parent reply	other threads:[~2010-04-26  6:48 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-26  1:15 [PATCH] start_command: close cmd->err descriptor when fork/spawn fails bert Dvornik
2010-04-26  6:16 ` Johannes Sixt
2010-04-26  6:29 ` Johannes Schindelin
2010-04-26  6:48 ` Johannes Sixt [this message]
2010-04-26 11:47   ` Albert Dvornik
2010-04-26 23:58     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4BD53749.6040708@viscovery.net \
    --to=j.sixt@viscovery.net \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=dvornik+git@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=msysgit@googlegroups.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).