Re: Help needed on 2.54.0-rc0 t5301.13 looping.
- From
Adrian Ratiu <adrian.ratiu@collabora.com>
- Date
- Apr 8, 2026, 11:53 UTC
- Message-ID
- <871pgp3byq.fsf@collabora.com>
- In-Reply-To
- <20260408054347.GA2284358@coredump.intra.peff.net>
On Wed, 08 Apr 2026, Jeff King <peff@peff.net> wrote:
Show 66 quoted lines
> 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.