Subject: Re: t5401-update-hooks test failure

From: "Shawn O. Pearce" <[email-masked]>
Date: Tue, 9 Feb 2010 09:51:39 -0800

Junio C Hamano <[email-masked]> wrote:
> $ while sh t5401-*.sh -i; do :; done
> ... wait for a while ...
> * FAIL 12: send-pack stderr contains hook messages
> 
>                 grep ^remote: send.err | sed "s/ *\$//" >actual &&
>                 test_cmp - actual <expect
> 
> $ t/(364643e...); cat trash\ directory.t5401-update-hooks/actual 
> remote: STDOUT pre-receive
> remote: STDERR pre-receive
> remote: STDOUT update refs/heads/master
> remote: STDERR update refs/heads/master
> remote: STDOUT update refs/heads/tofail
> remote: STDOUT post-receive
> remote: STDERR post-receive
> remote: STDOUT post-update
> remote: STDERR post-update
> $ t/(364643e...); cat trash\ directory.t5401-update-hooks/expect 
> remote: STDOUT pre-receive
> remote: STDERR pre-receive
> remote: STDOUT update refs/heads/master
> remote: STDERR update refs/heads/master
> remote: STDOUT update refs/heads/tofail
> remote: STDERR update refs/heads/tofail
> remote: STDOUT post-receive
> remote: STDERR post-receive
> remote: STDOUT post-update
> remote: STDERR post-update

A quick visual inspection shows that only the STDERR tofail message
is missing here.  That sounds to me like a race condition in the
recv_sideband decoder.  Or, a race condition in the hook code in
builtin-receive-pack.c.

I doubt its in receive-pack. run_update_hook() directly calls the
copy_to_sideband() function, and that reads until EOF on the hook's
stderr stream before it returns and waits for the hook's exit status.
So we should be pulling everything and dumping it into the sideband.

builtin-send-pack.c clearly isn't stopping early while processing
the stream, since we see later messages from the post-receive and
post-update hooks just fine.

So I think the only code that is in question is the case 2 arm of
recv_sideband().  But to be honest, I can't find any fault with it.

I've been running this test in a while loop for a while now, and
I can't make it trigger this failure.  Maybe its possible that
its the update hook itself, not flushing its stderr buffer before
it terminates?

-- 
Shawn.