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

> Jeff King <peff@peff.net> 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.