From: <rsbecker@nexbridge.com>
To: "'Adrian Ratiu'" <adrian.ratiu@collabora.com>,
"'Jeff King'" <peff@peff.net>
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, 8 Apr 2026 11:52:34 -0400 [thread overview]
Message-ID: <012101dcc76f$bae10a80$30a31f80$@nexbridge.com> (raw)
In-Reply-To: <871pgp3byq.fsf@collabora.com>
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
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)
<and hung>
next prev parent reply other threads:[~2026-04-08 15:52 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
2026-04-08 15:44 ` rsbecker
2026-04-08 15:52 ` rsbecker [this message]
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='012101dcc76f$bae10a80$30a31f80$@nexbridge.com' \
--to=rsbecker@nexbridge.com \
--cc=adrian.ratiu@collabora.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/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