[6/7] tst_test.sh: Run cleanup also after test timeout

Also timeout requires to run a test cleanup (e.g. zram01.sh).
Thus send first SIGINT, but keep also SIGKILL for safety reasons
(after 5 sec to give some time to the cleanup function and
_tst_check_security_modules()).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Originally posted in 
https://patchwork.ozlabs.org/project/ltp/patch/20210202101942.31328-1-pvorel@suse.cz/

* renamed function
* use signal names instead of numbers in kill parameters

 testcases/lib/tst_test.sh | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

On Tue, Mar 2, 2021 at 6:02 AM Petr Vorel <pvorel@suse.cz> wrote:

> Also timeout requires to run a test cleanup (e.g. zram01.sh).
> Thus send first SIGINT, but keep also SIGKILL for safety reasons
> (after 5 sec to give some time to the cleanup function and
> _tst_check_security_modules()).
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Originally posted in
>
> https://patchwork.ozlabs.org/project/ltp/patch/20210202101942.31328-1-pvorel@suse.cz/
>
> * renamed function
> * use signal names instead of numbers in kill parameters
>
>  testcases/lib/tst_test.sh | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index 58056e28b..097f672a1 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -21,7 +21,7 @@ export TST_LIB_LOADED=1
>  . tst_security.sh
>
>  # default trap function
> -trap "tst_brk TBROK 'test interrupted'" INT
> +trap "tst_brk TBROK 'test interrupted or timed out'" INT
>
>  _tst_cleanup_timer()
>  {
> @@ -442,6 +442,14 @@ _tst_multiply_timeout()
>         return 0
>  }
>
> +_tst_run_timer()
>

Hmm, this name is not good than before, or rename to _tst_kill_timer_pid(),
_tst_stop_timer()?



> +{
> +       tst_res TBROK "test killed, timeout! If you are running on slow
> machine, try exporting LTP_TIMEOUT_MUL > 1"
> +       kill -INT -$pid
> +       sleep 5
> +       kill -KILL -$pid
> +}
> +
>  _tst_setup_timer()
>  {
>         TST_TIMEOUT=${TST_TIMEOUT:-300}
> @@ -465,8 +473,7 @@ _tst_setup_timer()
>         tst_res TINFO "timeout per run is ${h}h ${m}m ${s}s"
>
>         _tst_cleanup_timer
> -
> -       sleep $sec && tst_res TBROK "test killed, timeout! If you are
> running on slow machine, try exporting LTP_TIMEOUT_MUL > 1" && kill -9
> -$pid &
> +       sleep $sec && _tst_run_timer &
>
>         _tst_setup_timer_pid=$!
>  }
> --
> 2.30.1
>
>

Hi Li,

...
> >  _tst_cleanup_timer()
> >  {
> > @@ -442,6 +442,14 @@ _tst_multiply_timeout()
> >         return 0
> >  }

> > +_tst_run_timer()


> Hmm, this name is not good than before, or rename to _tst_kill_timer_pid(),
> _tst_stop_timer()?

Good point. I slightly prefer _tst_stop_timer, but no hard feeling about it.


Kind regards,
Petr

> > +{
> > +       tst_res TBROK "test killed, timeout! If you are running on slow
> > machine, try exporting LTP_TIMEOUT_MUL > 1"
> > +       kill -INT -$pid
> > +       sleep 5
> > +       kill -KILL -$pid
> > +}
> > +

Hi!
> > >  _tst_cleanup_timer()
> > >  {
> > > @@ -442,6 +442,14 @@ _tst_multiply_timeout()
> > >         return 0
> > >  }
> 
> > > +_tst_run_timer()
> 
> 
> > Hmm, this name is not good than before, or rename to _tst_kill_timer_pid(),
> > _tst_stop_timer()?
> 
> Good point. I slightly prefer _tst_stop_timer, but no hard feeling about it.

Or _tst_kill_test()?

> > > +{
> > > +       tst_res TBROK "test killed, timeout! If you are running on slow
> > > machine, try exporting LTP_TIMEOUT_MUL > 1"
> > > +       kill -INT -$pid
> > > +       sleep 5
> > > +       kill -KILL -$pid

Maybe we should change the messages to reflect what is happening and
maybe we should check if the test is still running before sending
SIGKILL with kill -0 $pid?

	tst_res TBROK "Test timeouted, sending SIGINT, ...."
	kill -INT -$pid

	sleep 5

	if kill -0 $pid 2>&1 > /dev/null; then
		tst_res TBROK "Test still running, sending SIGKILL"
		kill -KILL -$pid
	fi

We can also bussy loop wait for the process to terminate, e.g. loop 10
times with sleep 1 in the body and break the loop if kill -0 $pid
returns failure.

> > > +}
> > > +

Hi Cyril, Li,

> > > > +_tst_run_timer()

> > > Hmm, this name is not good than before, or rename to _tst_kill_timer_pid(),
> > > _tst_stop_timer()?

> > Good point. I slightly prefer _tst_stop_timer, but no hard feeling about it.

> Or _tst_kill_test()?
+1

> > > > +{
> > > > +       tst_res TBROK "test killed, timeout! If you are running on slow
> > > > machine, try exporting LTP_TIMEOUT_MUL > 1"
> > > > +       kill -INT -$pid
> > > > +       sleep 5
> > > > +       kill -KILL -$pid

> Maybe we should change the messages to reflect what is happening and
> maybe we should check if the test is still running before sending
> SIGKILL with kill -0 $pid?

> 	tst_res TBROK "Test timeouted, sending SIGINT, ...."
> 	kill -INT -$pid

> 	sleep 5

> 	if kill -0 $pid 2>&1 > /dev/null; then
> 		tst_res TBROK "Test still running, sending SIGKILL"
> 		kill -KILL -$pid
> 	fi

> We can also bussy loop wait for the process to terminate, e.g. loop 10
> times with sleep 1 in the body and break the loop if kill -0 $pid
> returns failure.
Busy loop wait 10 times + final -KILL make sense to me. I'm going to merge first
five commits and send v2 this + the last commit.

Kind regards,
Petr