[PATCH v1] strace -D is killed by process group kill.

[Dmitry V. Levin]

Hi,

On Fri, Oct 04, 2019 at 11:29:07AM +0200, Fanda Uchytil wrote:
> Hi,
> 
> I stumble upon unexpected behavior: if strace is used with option '-D'
> (tracer as a detached grandchild) and process (leader) kills whole
> process group, it will kill strace too.
> 
> It can be easily reproduced by `timeout` from "coreutils":
> 
>   # timeout -s KILL 2 strace -D -o ./strace-inside.log /bin/sleep 10 &
> 
> Here we can see, that `strace` didn't finished its output (because it
> was killed):
> 
>   # tail -n 1 ./strace-inside.log
>   nanosleep({tv_sec=10, tv_nsec=0}, 
> 
> If `timeout` is not run in '--foreground' mode, it changes process group
> and after "timeout" it sends two kills:
> 
>   setpgid(0, 0)                           = 0
>   kill(37337, SIGKILL)                    = 0
>   kill(0, SIGKILL)                        = ?
> 
> The first kill is for the `sleep` and the second one is for the process
> group (which is `strace` part of). PIDs and their relations are:
> 
>   timeout  pid=30595   [ppid=476   bash   ]     pgrp=30595
>   sleep    pid=37337   [ppid=30595 timeout]     pgrp=30595
>   strace   pid=30603   [ppid=1     systemd]     pgrp=30595
> 
> Here is "strace log" of `strace` inside `timeout`:
> 
>   strace: Process 30603 attached
>   wait4(-1,  <unfinished ...>)            = ?
>   +++ killed by SIGKILL +++
> 
> I think that detached `strace` should not be killed like that -- it
> should not be part of former grandparents' "job pipeline".
> 
> Solution for that could be creating a new process group for strace
> (= his own pgid). I attached proof-of-concept patch, but please look at
> it (I didn't see any side effects, but that doesn't mean there are
> none).
> 
> -- 
> Fanda Uchytil
> 
> ---
>  strace.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/strace.c b/strace.c
> index b52a3db3..aeb23351 100644
> --- a/strace.c
> +++ b/strace.c
> @@ -1131,6 +1131,8 @@ startup_attach(void)
>  			_exit(0); /* paranoia */
>  		}
>  		/* grandchild */
> +		/* Create new process group, so we'll not be killed by kill(0, ...) */
> +		setpgid(0, 0);
>  		/* We will be the tracer process. Remember our new pid: */
>  		strace_tracer_pid = getpid();
>  	}

I think this approach makes sense, thanks!


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20191004/01729375/attachment.bin>