From: rsbecker@nexbridge.com Date: Wed, 08 Apr 2026 15:55:01 GMT Subject: RE: Help needed on 2.54.0-rc0 t5301.13 looping. Message-ID: <012b01dcc770$126425d0$372c7170$@nexbridge.com> In-Reply-To: <871pgp3byq.fsf@collabora.com> On April 8, 2026 11:53 AM, I wrote (accidentally): >To: 'Adrian Ratiu' ; 'Jeff King' >Cc: 'Junio C Hamano' ; 'git@vger.kernel.org' > >Subject: RE: Help needed on 2.54.0-rc0 t5301.13 looping. > >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) I was not patient enough. This continued eventually and passed.