git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Help needed on 2.54.0-rc0 t5301.13 looping.

From
Junio C Hamano <gitster@pobox.com>
Date
Apr 8, 2026, 15:50 UTC
Message-ID
<xmqqpl491mfd.fsf@gitster.g>
In-Reply-To
<20260408052031.GB1324339@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 43 quoted lines
> 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.

Thanks.
Previous: Jeff KingNext: Adrian Ratiu
Message 12 of 18 in “Help needed on 2.54.0-rc0 t5301.13 looping.”
  1. rsbecker@nexbridge.comApr 7, 2026
  2. Jeff KingApr 8, 2026
  3. Jeff KingApr 8, 2026
  4. Adrian RatiuApr 8, 2026
  5. rsbecker@nexbridge.comApr 8, 2026
  6. rsbecker@nexbridge.comApr 8, 2026
  7. rsbecker@nexbridge.comApr 8, 2026
  8. Junio C HamanoApr 8, 2026
  9. rsbecker@nexbridge.comApr 8, 2026
  10. Adrian RatiuApr 8, 2026
  11. t5401: test SIGPIPE with parallel hooksJeff King, Apr 8, 2026
  12. Junio C HamanoApr 8, 2026
  13. Adrian RatiuApr 8, 2026
  14. run_processes_parallel(): fix order of sigpipe handlingJeff King, Apr 8, 2026
  15. Junio C HamanoApr 8, 2026
  16. Junio C HamanoApr 8, 2026
  17. Jeff KingApr 8, 2026
  18. Junio C HamanoApr 9, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.