Re: [LTP] [PATCH v2] lib: shell: Fix timeout process races

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> > > This 'pid' is the parent shell process id, so it obviously that
> > > tst_timeout_kill
> > > process would get signal SIGTERM as well.
> > >
> > > I'm thinking maybe we should let tst_timeout_kill itself ignore SIGTERM
> > > otherwise we have no chance to perform the following double-check code?
> >
> > I guess that signal(SIGTERM, SIG_IGN) a the start of the main() should
> > fix it.
> >
> 
> It works, but better put it behind of sleep(timeout).
> 
> Because we still need to guarantee tst_timeout_kill can be
> stopped by _tst_cleanup_timer before timeout happening.

Right, of course we want the timeout killer to be killable before the
timeout.

> --- a/testcases/lib/tst_timeout_kill.c
> +++ b/testcases/lib/tst_timeout_kill.c
> @@ -44,6 +44,8 @@ int main(int argc, char *argv[])
>         if (timeout)
>                 sleep(timeout);
> 
> +       signal(SIGTERM, SIG_IGN);
> +
>         print_msg("Test timed out, sending SIGTERM!");
>         print_msg("If you are running on slow machine, try exporting
> LTP_TIMEOUT_MUL > 1");
> 
> @@ -57,12 +59,12 @@ int main(int argc, char *argv[])
> 
>         i = 10;
> 
> -       while (!kill(-pid, 0) && i-- > 0) {
> +       while (!kill(pid, 0) && i-- > 0) {
>                 print_msg("Test is still running...");
>                 sleep(1);
>         }
> 
> -       if (!kill(-pid, 0)) {
> +       if (!kill(pid, 0)) {
>                 print_msg("Test is still running, sending SIGKILL");
>                 ret = kill(-pid, SIGKILL);
>                 if (ret) {

Hmm, and of course this does not work since the timeout kill process is
around. Maybe we should make this process a separate process group from
the start, what about calling setpgid(0, 0) instead of setting up the
signal handler? That way we can send the signals to the whole process
group and make sure everything has been cleaned up.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp