Re: [v3 PATCH 17/17] eval: Add vfork support

Re: [v3 PATCH 17/17] eval: Add vfork support

[Harald van Dijk]

On 18/05/2018 19:39, Herbert Xu wrote:
> This patch adds basic vfork support for the case of a simple command.
>...  
> @@ -879,17 +892,30 @@ forkchild(struct job *jp, union node *n, int mode)
>   		}
>   	}
>   	if (!oldlvl && iflag) {
> -		setsignal(SIGINT);
> -		setsignal(SIGQUIT);
> +		if (mode != FORK_BG) {
> +			setsignal(SIGINT);
> +			setsignal(SIGQUIT);
> +		}
>   		setsignal(SIGTERM);
>   	}
> +
> +	if (lvforked)
> +		return;
> +
>   	for (jp = curjob; jp; jp = jp->prev_job)
>   		freejob(jp);
>   }

This leaves SIGQUIT ignored in background jobs in interactive shells.

   ENV= dash -ic 'dash -c "kill -QUIT \$\$; echo huh" & wait'

As of dash 0.5.11, this prints "huh". Before, the subprocess process 
killed itself before it could print anything. Other shells do not leave 
SIGQUIT ignored.

(In a few other shells, this also prints "huh", but in those other 
shells, that is because the inner shell chooses to ignore SIGQUIT, not 
because the outer shell leaves it ignored.)

Cheers,
Harald van Dijk

[PATCH] jobs: Always reset SIGINT/SIGQUIT handlers

[Herbert Xu]

On Fri, Jan 08, 2021 at 08:55:41PM +0000, Harald van Dijk wrote:
> On 18/05/2018 19:39, Herbert Xu wrote:
> > This patch adds basic vfork support for the case of a simple command.
> > ...  @@ -879,17 +892,30 @@ forkchild(struct job *jp, union node *n, int
> > mode)
> >   		}
> >   	}
> >   	if (!oldlvl && iflag) {
> > -		setsignal(SIGINT);
> > -		setsignal(SIGQUIT);
> > +		if (mode != FORK_BG) {
> > +			setsignal(SIGINT);
> > +			setsignal(SIGQUIT);
> > +		}
> >   		setsignal(SIGTERM);
> >   	}
> > +
> > +	if (lvforked)
> > +		return;
> > +
> >   	for (jp = curjob; jp; jp = jp->prev_job)
> >   		freejob(jp);
> >   }
> 
> This leaves SIGQUIT ignored in background jobs in interactive shells.
> 
>   ENV= dash -ic 'dash -c "kill -QUIT \$\$; echo huh" & wait'
> 
> As of dash 0.5.11, this prints "huh". Before, the subprocess process killed
> itself before it could print anything. Other shells do not leave SIGQUIT
> ignored.
> 
> (In a few other shells, this also prints "huh", but in those other shells,
> that is because the inner shell chooses to ignore SIGQUIT, not because the
> outer shell leaves it ignored.)

Thanks for catching this.  I have no idea how that got in there
and it makes no sense whatsoever.  This patch removes the if
conditional.

Fixes: e94a964e7dd0 ("eval: Add vfork support")
Reported-by: Harald van Dijk <harald@gigawatt.nl>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/src/jobs.c b/src/jobs.c
index 516786f..08c4f13 100644
--- a/src/jobs.c
+++ b/src/jobs.c
@@ -892,10 +892,8 @@ static void forkchild(struct job *jp, union node *n, int mode)
 		}
 	}
 	if (!oldlvl && iflag) {
-		if (mode != FORK_BG) {
-			setsignal(SIGINT);
-			setsignal(SIGQUIT);
-		}
+		setsignal(SIGINT);
+		setsignal(SIGQUIT);
 		setsignal(SIGTERM);
 	}
 
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt