From: rsbecker@nexbridge.com Date: Wed, 08 Apr 2026 15:52:34 GMT Subject: RE: Help needed on 2.54.0-rc0 t5301.13 looping. Message-ID: <012101dcc76f$bae10a80$30a31f80$@nexbridge.com> In-Reply-To: <871pgp3byq.fsf@collabora.com> On April 8, 2026 7:53 AM, Adrian Ratiu wrote: >To: Jeff King ; rsbecker@nexbridge.com >Cc: Junio C Hamano ; git@vger.kernel.org >Subject: Re: Help needed on 2.54.0-rc0 t5301.13 looping. > >On Wed, 08 Apr 2026, Jeff King 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. This fix, however, causes an unreported infinite loop in t5571: expecting success of 5571.11 'sigpipe does not cause pre-push hook failure': test_hook --clobber pre-push <<-\EOF && exit 0 EOF git push parent1 "refs/heads/b/*:refs/heads/b/*" Total 0 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0)