From: Adrian Ratiu Date: Wed, 08 Apr 2026 16:26:59 GMT Subject: Re: Help needed on 2.54.0-rc0 t5301.13 looping. Message-ID: <87y0ix1kq4.fsf@collabora.com> In-Reply-To: On Wed, 08 Apr 2026, Junio C Hamano wrote: > Jeff King writes: > >> I think the root of the issue is that we should not be trying to >> propagate SIGPIPE to the child in this case at all. Our handler is >> pushed there only because it's part of sigchain_push_common(), which is >> sensible: in general if we are dying to SIGPIPE we want to do our >> cleanup. It's just funny in this case with the ordering of our SIG_IGN, >> because now that SIG_IGN isn't on top of the stack anymore. >> >> I.e., I think we want to reorder like this: >> >> diff --git a/run-command.c b/run-command.c >> index 32c290ee6a..8a95f7ff1e 100644 >> --- a/run-command.c >> +++ b/run-command.c >> @@ -1895,14 +1895,19 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) >> "max:%"PRIuMAX, >> (uintmax_t)opts->processes); >> >> + pp_init(&pp, opts, &pp_sig); >> + >> /* >> * Child tasks might receive input via stdin, terminating early (or not), so >> * ignore the default SIGPIPE which gets handled by each feed_pipe_fn which >> * actually writes the data to children stdin fds. >> + * >> + * This _must_ come after pp_init(), because it installs its own >> + * SIGPIPE handler (to cleanup children), and we want to supersede >> + * that. >> */ >> sigchain_push(SIGPIPE, SIG_IGN); >> >> - pp_init(&pp, opts, &pp_sig); >> while (1) { >> for (i = 0; >> i < spawn_cap && !pp.shutdown && >> >> Does that make your problem go away? >> >> 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. > > Oh, I am very much impressed by this analysis. > > As -rc1 has already been tagged (but not pushed out yet), we would > probably want to apply a fix before -rc2, I suppose. Yes, that is fine. All my local tests also look good with Peff's patch (including the parallel series). @Peff Please let me know if you wish me to send a patch or if you wish to send it yourself, since this investigation is your work & effort. :)