Re: [PATCH] start_command: reset disposition of all signals in child

[Phillip Wood <phillip.wood123@gmail.com>]

On 08/09/2023 18:38, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>> Oh I should have looked more carefully at the existing uses. It looks
>> like it is only my sequencer patch that does
>>
>> 	sigchain_push(SIGINT, SIG_IGN);
>> 	sigchain_push(SIGQUIT, SIG_IGN);
>> 	res = run_command(...);
> 
> Hmph, does it mean this patch would become unnecessary, once you fix
> the above sequence to follow the pattern "to spawn and then ignore"?

Yes, sorry for the confusion. There are a couple of things that I think 
we should address though. Firstly we should change the comment in 
run-command which says execve() resets ignored signals to SIG_DFL to say 
something like

	Preserve the set of ignored signals so that running git via a
	wrapper like nohup works as the user expects

The other thing is that we have some instances where we ignore SIGPIPE 
before calling start_command() which means we're ignoring it in the 
child process as well. For example in gpg-interface.c we have

	sigchain_push(SIGPIPE, SIG_IGN);
	ret = pipe_command(&gpg, sigc->payload, sigc->payload_len, &gpg_stdout, 0,
			   &gpg_stderr, 0);
	sigchain_pop(SIGPIPE);

To fix that one we'd need to change pipe_command() to ignore SIGPIPE 
after calling start_command() or add a flag to struct child_process to 
do to that.

Another example is in upload-pack.c

	/*
	 * If the next rev-list --stdin encounters an unknown commit,
	 * it terminates, which will cause SIGPIPE in the write loop
	 * below.
	 */
	sigchain_push(SIGPIPE, SIG_IGN);

	if (start_command(cmd))
		goto error;

rev-list does not check for errors when writing to stdout unless 
GIT_FLUSH is set in the environment so if parent process exits early 
rev-list will keep going until it thinks it has printed everything.

I think adding a flag to struct child_process to ignore SIGPIPE in the 
parent is probably the best way to avoid problems like this.

Best Wishes

Phillip