Re: [PATCH] jobs: Block signals during tcsetpgrp

[Harald van Dijk <harald@gigawatt.nl>]

On 06/01/2021 04:45, Herbert Xu wrote:
> This patch implements the blocking of SIGTTOU (and everything else)
> while we call tcsetpgrp.
> 
> Reported-by: Steffen Nurpmeso <steffen@sdaoden.eu>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> diff --git a/src/jobs.c b/src/jobs.c
> index 516786f..809f37c 100644
> --- a/src/jobs.c
> +++ b/src/jobs.c
> @@ -1512,7 +1512,13 @@ showpipe(struct job *jp, struct output *out)
>   STATIC void
>   xtcsetpgrp(int fd, pid_t pgrp)
>   {
> -	if (tcsetpgrp(fd, pgrp))
> +	int err;
> +
> +	sigblockall(NULL);
> +	err = tcsetpgrp(fd, pgrp);
> +	sigclearmask();
> +
> +	if (err)
>   		sh_error("Cannot set tty process group (%s)", strerror(errno));
>   }
>   #endif

While this is a step in the right direction, Jilles has already replied 
with an explanation of why this is not enough: if the terminal is in 
TOSTOP mode, it's not just tcsetpgrp() that needs to be handled, it's 
any write as well that may occur while the shell is not in the 
foreground process group. While it may be working according to design 
for messages written when the shell is not supposed to be in the 
foreground process group, it is another story when the shell is both 
responsible for taking itself out of the foreground process group and 
for writing a message. This is made worse by the fact that there is no 
synchronisation with child processes on errors, so even forcibly 
restoring the foreground process group may not be enough: unfortunate 
scheduling may result in a child process immediately setting the 
foreground process group to the child process after the parent process 
attempted to restore it to itself. I have not yet seen a good solution 
for this.

Cheers,
Harald van Dijk