RE: Help needed on 2.54.0-rc0 t5301.13 looping.
On April 8, 2026 11:53 AM, I wrote (accidentally):
>To: 'Adrian Ratiu' <adrian.ratiu@collabora.com>; 'Jeff King'
Show 52 quoted lines
>Cc: 'Junio C Hamano' <gitster@pobox.com>; 'git@vger.kernel.org'
><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 <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.
>>
>>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
Show 47 quoted lines
>>> + # 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
Show 6 quoted lines
> 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) <and hung>
I was not patient enough. This continued eventually and passed.