Git development
 help / color / mirror / Atom feed
From: Adrian Ratiu <adrian.ratiu@collabora.com>
To: Jeff King <peff@peff.net>, rsbecker@nexbridge.com
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: Help needed on 2.54.0-rc0 t5301.13 looping.
Date: Wed, 08 Apr 2026 14:53:17 +0300	[thread overview]
Message-ID: <871pgp3byq.fsf@collabora.com> (raw)
In-Reply-To: <20260408054347.GA2284358@coredump.intra.peff.net>

On Wed, 08 Apr 2026, Jeff King <peff@peff.net> wrote:
> On Wed, Apr 08, 2026 at 01:20:31AM -0400, Jeff King wrote:
>
>> I suspect we could construct a related case that does fail on Linux
>> without the patch above. Imagine we actually have two hooks running in
>> parallel. The first one is fast and does not read its input, and the
>> second one is slow. We'll get SIGPIPE writing to the first one, and then
>> kill _both_ children. But that's wrong! There is no reason to kill the
>> second hook, as our intent was to ignore SIGPIPE.
>
> This would require running hooks in parallel, which isn't implemented
> yet for v2.54.0. But if I build on top of the ar/parallel-hooks topic,
> then this test:
>
> diff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh
> index 44ec875aef..97257763d3 100755
> --- a/t/t5401-update-hooks.sh
> +++ b/t/t5401-update-hooks.sh
> @@ -139,4 +139,43 @@ test_expect_success 'pre-receive hook that forgets to read its input' '
>  	git push ./victim.git "+refs/heads/*:refs/heads/*"
>  '
>  
> +test_expect_success 'hooks in parallel that do not read input' '
> +	# Add this to our $PATH to avoid having to write the whole trash
> +	# directory into our config options, which would require quoting.
> +	mkdir bin &&
> +	PATH=$PWD/bin:$PATH &&
> +
> +	write_script bin/hook-fast <<-\EOF &&
> +	# This hook does not read its input, so the parent process
> +	# may see SIGPIPE if it is not ignored. It should happen
> +	# relatively quickly.
> +	exit 0
> +	EOF
> +
> +	write_script bin/hook-slow <<-\EOF &&
> +	# This hook is slow, so we expect it to still be running
> +	# when the other hook has exited (and the parent has a pipe error
> +	# writing to it).
> +	#
> +	# So we want to be slow enough that we expect this to happen, but not
> +	# so slow that the test takes forever. 1 second is probably enough
> +	# in practice (and if it is occasionally not on a loaded system, we
> +	# will err on the side of having the test pass).
> +	sleep 1
> +	exit 0
> +	EOF
> +
> +
> +	git init --bare parallel.git &&
> +	git -C parallel.git config hook.fast.command "hook-fast" &&
> +	git -C parallel.git config hook.fast.event pre-receive &&
> +	git -C parallel.git config hook.fast.parallel true &&
> +	git -C parallel.git config hook.slow.command "hook-slow" &&
> +	git -C parallel.git config hook.slow.event pre-receive &&
> +	git -C parallel.git config hook.slow.parallel true &&
> +	git -C parallel.git config hook.jobs 2 &&
> +
> +	git push ./parallel.git "+refs/heads/*:refs/heads/*"
> +'
> +
>  test_done
>
> fails reliably. And applying the patch I suggested earlier fixes it.
>
> So I think it's probably a good idea regardless, though I'm still
> curious to see if it solves Randall's non-parallel case on NonStop.

Thanks Peff for the in-depth analysis, fix and test.
It is very much appreciated. I missed this case.

I agree with your assesement: this must be fixed regardless if it also
fixes Randall's case or not (might be a separate root cause).

I would proceed like this (obviously crediting you for the fix & test):

If it fixes Randall's case:
   send a standalone bug-fix patch, then integrate the test into the
   parallel series.
else
   integrate both the fix and the test into the parallel series.

@Randall please let us know if the fix proposed by Peff in the other
response works for you.

  reply	other threads:[~2026-04-08 11:53 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-07 23:37 Help needed on 2.54.0-rc0 t5301.13 looping rsbecker
2026-04-08  5:20 ` Jeff King
2026-04-08  5:43   ` Jeff King
2026-04-08 11:53     ` Adrian Ratiu [this message]
2026-04-08 15:44       ` rsbecker
2026-04-08 15:52       ` rsbecker
2026-04-08 15:55       ` rsbecker
2026-04-08 16:53       ` Junio C Hamano
2026-04-08 16:58         ` rsbecker
2026-04-08 17:01         ` Adrian Ratiu
2026-04-08 17:30           ` [PATCH] t5401: test SIGPIPE with parallel hooks Jeff King
2026-04-08 15:50   ` Help needed on 2.54.0-rc0 t5301.13 looping Junio C Hamano
2026-04-08 16:26     ` Adrian Ratiu
2026-04-08 17:20       ` [PATCH] run_processes_parallel(): fix order of sigpipe handling Jeff King
2026-04-08 17:59         ` Junio C Hamano
2026-04-08 20:54           ` Junio C Hamano
2026-04-08 23:42             ` Jeff King
2026-04-09 13:40               ` 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=871pgp3byq.fsf@collabora.com \
    --to=adrian.ratiu@collabora.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=rsbecker@nexbridge.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