Re: [PATCH] Fix race and deadlock when sending pack

From: Daniel Barkalow <barkalow@iabervon.org>
Date: Dec 19, 2005, 06:49 UTC
Message-ID: <Pine.LNX.4.64.0512190130450.25300@iabervon.org>
In-Reply-To: <43A628F6.1060807@serice.net>

On Sun, 18 Dec 2005, Paul Serice wrote:

Show 30 quoted lines

> Fix race and deadlock when sending pack.
> 
> The best way to reproduce the problem is to locally clone your
> repository.  When you perform a push, git-send-pack will directly set
> up pipes connected to stdin and stdout of git-receive-pack.  You
> should then set up hook/post-update or hook/update to try to write
> lots of text to stdout.  (You want to use the local protocol because
> ssh is robust enough to mask the worst behavior.)
> 
> The first problem is that git-send-pack closes git-receive-pack's
> stdout (which is inherited by the hooks) immediately after sending the
> pack.  This almost always causes the hooks to receive SIGPIPE when
> they try to write to stdout.
> 
> After fixing the SIGPIPE problem, you then run into a deadlock because
> git-send-pack is blocked trying to reap git-receive-pack and
> git-receive-pack (or one of its hooks) is blocked waiting for
> git-send-pack to read its output.
> 
> I've also added an example a one-liner to both hooks demonstrating how
> to redirect all subsequent output to stderr.  Because
> git-receive-pack's stderr is not redirected, it has always been safe
> to write to stderr.  Thus, all current status related output appears
> on stderr.  This can lead to confusing ordering of messages if only
> the hooks are using stdout.  The patch has the one-liner commented
> out, but perhaps it should be enabled by default.
> 
> In addition, this commit reverts the work-around provided by
> 128aed684d0b3099092b7597c8644599b45b7503 which redirected both stdout
> and stderr for the hooks to /dev/null.

Actually, that was stdin and stdout. If, for some reason, a hook looked at stdin, it could get surprising results. I don't think that it's actually a good idea to have output to stdout from hooks go to git-send-pack's stdout, since we may want to have git-send-pack report some sort of information of its own to stdout, which would then get confused with output from hooks. I think /dev/null, a log file, and stderr are the reasonable choices for what happens to output (and input pretty much has to be /dev/null).

	-Daniel
*This .sig left intentionally blank*