Re: [PATCH] t0008: avoid SIGPIPE race condition on fifo

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> Subject: [PATCH] t0008: avoid SIGPIPE race condition on fifo
>
> To test check-ignore's --stdin feature, we use two fifos to
> send and receive data. We carefully keep a descriptor to its
> input open so that it does not receive EOF between input
> lines. However, we do not do the same for its output. That
> means there is a potential race condition in which
> check-ignore has opened the output pipe once (when we read
> the first line), and then writes the second line before we
> have re-opened the pipe.
>
> In that case, check-ignore gets a SIGPIPE and dies. The
> outer shell then tries to open the output fifo but blocks
> indefinitely, because there is no writer.  We can fix it by
> keeping a descriptor open through the whole procedure.

Ahh, figures.

I wish I were smart enough to figure that out immediately after
seeing the test that does funny things to "in" with "9".

Thanks.

> This should also help if check-ignore dies for any other
> reason (we would already have opened the fifo and would
> therefore not block, but just get EOF on read).
>
> However, we are technically still susceptible to
> check-ignore dying early, before we have opened the fifo.
> This is an unlikely race and shouldn't generally happen in
> practice, though, so we can hopefully ignore it.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  t/t0008-ignores.sh | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
> index a56db80..c29342d 100755
> --- a/t/t0008-ignores.sh
> +++ b/t/t0008-ignores.sh
> @@ -697,13 +697,21 @@ test_expect_success PIPE 'streaming support for --stdin' '
>  	# shell, and then echo to the fd. We make sure to close it at
>  	# the end, so that the subprocess does get EOF and dies
>  	# properly.
> +	#
> +	# Similarly, we must keep "out" open so that check-ignore does
> +	# not ever get SIGPIPE trying to write to us. Not only would that
> +	# produce incorrect results, but then there would be no writer on the
> +	# other end of the pipe, and we would potentially block forever trying
> +	# to open it.
>  	exec 9>in &&
> +	exec 8<out &&
>  	test_when_finished "exec 9>&-" &&
> +	test_when_finished "exec 8<&-" &&
>  	echo >&9 one &&
> -	read response <out &&
> +	read response <&8 &&
>  	echo "$response" | grep "^\.gitignore:1:one	one" &&
>  	echo >&9 two &&
> -	read response <out &&
> +	read response <&8 &&
>  	echo "$response" | grep "^::	two"
>  '