* [LTP] [PATCH 1/2] tst_test: discard the stderr output
@ 2021-05-18 12:26 Li Wang
2021-05-18 12:26 ` [LTP] [PATCH 2/2] tst_test: using SIGTERM to terminate process Li Wang
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Li Wang @ 2021-05-18 12:26 UTC (permalink / raw)
To: ltp
Signed-off-by: Li Wang <liwang@redhat.com>
---
Notes:
We can merge this before the new release.
testcases/lib/tst_test.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index 28c2052d6..3a5651c01 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -444,13 +444,13 @@ _tst_kill_test()
kill -INT -$pid
tst_sleep 100ms
- while kill -0 $pid 2>&1 > /dev/null && [ $i -gt 0 ]; do
+ while kill -0 $pid >/dev/null 2>&1 && [ $i -gt 0 ]; do
tst_res TINFO "Test is still running, waiting ${i}s"
sleep 1
i=$((i-1))
done
- if kill -0 $pid 2>&1 > /dev/null; then
+ if kill -0 $pid >/dev/null 2>&1; then
tst_res TBROK "Test still running, sending SIGKILL"
kill -KILL -$pid
fi
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [LTP] [PATCH 2/2] tst_test: using SIGTERM to terminate process 2021-05-18 12:26 [LTP] [PATCH 1/2] tst_test: discard the stderr output Li Wang @ 2021-05-18 12:26 ` Li Wang 2021-05-19 6:46 ` Joerg Vehlow 2021-05-31 12:49 ` Cyril Hrubis 2021-05-18 13:50 ` [LTP] [PATCH 1/2] tst_test: discard the stderr output Cyril Hrubis 2021-05-18 14:35 ` Petr Vorel 2 siblings, 2 replies; 15+ messages in thread From: Li Wang @ 2021-05-18 12:26 UTC (permalink / raw) To: ltp We'd better avoid using SIGINT for process terminating becuasue, it has different behavoir on kind of shell. From Joerg Vehlow's test: - bash does not seem to care about SIGINT delivery to background processes, but can be blocked using trap - zsh ignores SIGINT for background processes by default, but can be allowed using trap - dash and busybox sh ignore the signal to background processes, and this cannot be changed with trap This patch cover the below situations: 1. SIGINT (Ctrl^C) for terminating the main process and do cleanup correctly before a timeout 2. Test finish normally and retrieves the _tst_timeout_process in the background via SIGTERM(sending by _tst_cleanup_timer) 3. Test timed out occurs and _tst_kill_test sending SIGTERM to terminating all process, and the main process do cleanup work 4. Test timed out occurs but still have process alive after _tst_kill_test sending SIGTERM, then sending SIGKILL to the whole group 5. Test terminated by SIGTERM unexpectly (e.g. system shutdown or process manager) and do cleanup work as well Co-authored-by: Joerg Vehlow <lkml@jv-coder.de> Signed-off-by: Li Wang <liwang@redhat.com> --- lib/newlib_tests/shell/test_timeout.sh | 2 +- lib/newlib_tests/shell/timeout03.sh | 1 + testcases/lib/tst_test.sh | 9 +++++---- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/newlib_tests/shell/test_timeout.sh b/lib/newlib_tests/shell/test_timeout.sh index b05680cb1..9f31afa32 100755 --- a/lib/newlib_tests/shell/test_timeout.sh +++ b/lib/newlib_tests/shell/test_timeout.sh @@ -28,7 +28,7 @@ timeout02.sh| -10|0| |2 timeout02.sh| -0.1|0| |0 timeout02.sh| -1.1|0| |2 timeout02.sh|-10.1|0| |2 -timeout03.sh| |0|12|137| | | |Test kill if test does not terminate by SIGINT +timeout03.sh| |0|12|137| | | |Test kill if test does not terminate by SIGTERM timeout04.sh| |0| | 2|0|0|1|Verify that timeout is enforced timeout02.sh| 2|1| 2| |1|0|0|Test termination of timeout process " diff --git a/lib/newlib_tests/shell/timeout03.sh b/lib/newlib_tests/shell/timeout03.sh index cd548d9a2..124e96a84 100755 --- a/lib/newlib_tests/shell/timeout03.sh +++ b/lib/newlib_tests/shell/timeout03.sh @@ -30,6 +30,7 @@ TST_TIMEOUT=1 do_test() { + trap "tst_res TINFO 'Sorry, timeout03 is still alive'" TERM tst_res TINFO "testing killing test after TST_TIMEOUT" sleep 2 diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh index 3a5651c01..66ffde4eb 100644 --- a/testcases/lib/tst_test.sh +++ b/testcases/lib/tst_test.sh @@ -21,7 +21,8 @@ export TST_LIB_LOADED=1 . tst_security.sh # default trap function -trap "tst_brk TBROK 'test interrupted or timed out'" INT +trap "tst_brk TBROK 'test interrupted'" INT +trap "unset _tst_setup_timer_pid; tst_brk TBROK 'test terminated'" TERM _tst_do_exit() { @@ -439,9 +440,9 @@ _tst_kill_test() { local i=10 - trap '' INT - tst_res TBROK "Test timeouted, sending SIGINT! If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1" - kill -INT -$pid + trap '' TERM + tst_res TBROK "Test timed out, sending SIGTERM! If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1" + kill -TERM -$pid tst_sleep 100ms while kill -0 $pid >/dev/null 2>&1 && [ $i -gt 0 ]; do -- 2.31.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [LTP] [PATCH 2/2] tst_test: using SIGTERM to terminate process 2021-05-18 12:26 ` [LTP] [PATCH 2/2] tst_test: using SIGTERM to terminate process Li Wang @ 2021-05-19 6:46 ` Joerg Vehlow 2021-05-19 8:52 ` Li Wang 2021-05-31 12:35 ` Cyril Hrubis 2021-05-31 12:49 ` Cyril Hrubis 1 sibling, 2 replies; 15+ messages in thread From: Joerg Vehlow @ 2021-05-19 6:46 UTC (permalink / raw) To: ltp Hi Li, On 5/18/2021 2:26 PM, Li Wang wrote: > Co-authored-by: Joerg Vehlow <lkml@jv-coder.de> Please use Joerg Vehlow <joerg.vehlow@aox-tech.de> here, thanks. > Signed-off-by: Li Wang <liwang@redhat.com> > --- > lib/newlib_tests/shell/test_timeout.sh | 2 +- > lib/newlib_tests/shell/timeout03.sh | 1 + > testcases/lib/tst_test.sh | 9 +++++---- > 3 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/lib/newlib_tests/shell/test_timeout.sh b/lib/newlib_tests/shell/test_timeout.sh > index b05680cb1..9f31afa32 100755 > --- a/lib/newlib_tests/shell/test_timeout.sh > +++ b/lib/newlib_tests/shell/test_timeout.sh > @@ -28,7 +28,7 @@ timeout02.sh| -10|0| |2 > timeout02.sh| -0.1|0| |0 > timeout02.sh| -1.1|0| |2 > timeout02.sh|-10.1|0| |2 > -timeout03.sh| |0|12|137| | | |Test kill if test does not terminate by SIGINT > +timeout03.sh| |0|12|137| | | |Test kill if test does not terminate by SIGTERM Whoa what did I write there, that's hard to read :) While you're at it, can you please change this to "Test is killed, if it does not terminate after SIGTERM" > timeout04.sh| |0| | 2|0|0|1|Verify that timeout is enforced > timeout02.sh| 2|1| 2| |1|0|0|Test termination of timeout process > " > diff --git a/lib/newlib_tests/shell/timeout03.sh b/lib/newlib_tests/shell/timeout03.sh At the top of this file, there is a comment with the expected output, that should be corrected > index cd548d9a2..124e96a84 100755 > --- a/lib/newlib_tests/shell/timeout03.sh > +++ b/lib/newlib_tests/shell/timeout03.sh > @@ -30,6 +30,7 @@ TST_TIMEOUT=1 > > do_test() > { > + trap "tst_res TINFO 'Sorry, timeout03 is still alive'" TERM We don't need this. > tst_res TINFO "testing killing test after TST_TIMEOUT" > > sleep 2 > diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh > index 3a5651c01..66ffde4eb 100644 > --- a/testcases/lib/tst_test.sh > +++ b/testcases/lib/tst_test.sh > @@ -21,7 +21,8 @@ export TST_LIB_LOADED=1 > . tst_security.sh > > # default trap function > -trap "tst_brk TBROK 'test interrupted or timed out'" INT > +trap "tst_brk TBROK 'test interrupted'" INT > +trap "unset _tst_setup_timer_pid; tst_brk TBROK 'test terminated'" TERM > > _tst_do_exit() > { > @@ -439,9 +440,9 @@ _tst_kill_test() > { > local i=10 > > - trap '' INT > - tst_res TBROK "Test timeouted, sending SIGINT! If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1" > - kill -INT -$pid > + trap '' TERM > + tst_res TBROK "Test timed out, sending SIGTERM! If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1" > + kill -TERM -$pid This works fine. +Reviewed-by: Joerg Vehlow <joerg.vehlow@aox-tech.de> after my remarks are resolved. But one more strange thing here. I wonder why this even works. $pid is used in _tst_kill_test and defined in _tst_setup_timer as a local variable. It looks like it is inherited through the call chain and since it is copied to the background process, it cannot be manipulated by the tests. Still I would vote for changing this at some point, because it is highly confusing. J?rg ^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH 2/2] tst_test: using SIGTERM to terminate process 2021-05-19 6:46 ` Joerg Vehlow @ 2021-05-19 8:52 ` Li Wang 2021-05-31 12:35 ` Cyril Hrubis 1 sibling, 0 replies; 15+ messages in thread From: Li Wang @ 2021-05-19 8:52 UTC (permalink / raw) To: ltp Hi Joerg, > This works fine. > +Reviewed-by: Joerg Vehlow <joerg.vehlow@aox-tech.de> > after my remarks are resolved. Thanks for the above comments, I will fix them all in patch V2. > But one more strange thing here. > I wonder why this even works. $pid is used in _tst_kill_test and defined > in _tst_setup_timer as a local variable. > It looks like it is inherited through the call chain and since it is > copied to the background process, it cannot be manipulated by the tests. > Still I would vote for changing this at some point, because it is highly > confusing. +1 Agree, I look at it a while and seems a local variable can only be visible in the sub-function/call-chain of SHELL. But I vote to fix the problem in a separate patch, it'd be appreciated if you help make the patch. -- Regards, Li Wang ^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH 2/2] tst_test: using SIGTERM to terminate process 2021-05-19 6:46 ` Joerg Vehlow 2021-05-19 8:52 ` Li Wang @ 2021-05-31 12:35 ` Cyril Hrubis 2021-05-31 13:16 ` Joerg Vehlow 1 sibling, 1 reply; 15+ messages in thread From: Cyril Hrubis @ 2021-05-31 12:35 UTC (permalink / raw) To: ltp Hi! > But one more strange thing here. > I wonder why this even works. $pid is used in _tst_kill_test and defined > in _tst_setup_timer as a local variable. > It looks like it is inherited through the call chain and since it is > copied to the background process, it cannot be manipulated by the tests. > Still I would vote for changing this at some point, because it is highly > confusing. That's actually a correct and well defined behavior, if you call a function g from function f the function g has access to the variables local to f. And yes it's confusing, but the alternative is having another global variable which I do not think is much better than this. -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH 2/2] tst_test: using SIGTERM to terminate process 2021-05-31 12:35 ` Cyril Hrubis @ 2021-05-31 13:16 ` Joerg Vehlow 2021-05-31 12:54 ` Cyril Hrubis 0 siblings, 1 reply; 15+ messages in thread From: Joerg Vehlow @ 2021-05-31 13:16 UTC (permalink / raw) To: ltp Hi Cyril, On 5/31/2021 2:35 PM, Cyril Hrubis wrote: > Hi! >> But one more strange thing here. >> I wonder why this even works. $pid is used in _tst_kill_test and defined >> in _tst_setup_timer as a local variable. >> It looks like it is inherited through the call chain and since it is >> copied to the background process, it cannot be manipulated by the tests. >> Still I would vote for changing this at some point, because it is highly >> confusing. > That's actually a correct and well defined behavior, if you call a > function g from function f the function g has access to the variables > local to f. > > And yes it's confusing, but the alternative is having another global > variable which I do not think is much better than this. Hm shell code has more strange behavior than I would have ever expected... But at least Li and myself did not know that and even while you know about this "feature", you think it is strange. I would rather like to be explicit and use a global variable (what harm is it really?) instead of confusing the next one looking at this piece of code.... J?rg ^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH 2/2] tst_test: using SIGTERM to terminate process 2021-05-31 13:16 ` Joerg Vehlow @ 2021-05-31 12:54 ` Cyril Hrubis 2021-06-01 4:23 ` Joerg Vehlow 0 siblings, 1 reply; 15+ messages in thread From: Cyril Hrubis @ 2021-05-31 12:54 UTC (permalink / raw) To: ltp Hi! > >> But one more strange thing here. > >> I wonder why this even works. $pid is used in _tst_kill_test and defined > >> in _tst_setup_timer as a local variable. > >> It looks like it is inherited through the call chain and since it is > >> copied to the background process, it cannot be manipulated by the tests. > >> Still I would vote for changing this at some point, because it is highly > >> confusing. > > That's actually a correct and well defined behavior, if you call a > > function g from function f the function g has access to the variables > > local to f. > > > > And yes it's confusing, but the alternative is having another global > > variable which I do not think is much better than this. > Hm shell code has more strange behavior than I would have ever expected... Strongly agree here, to be honest if there was as better scripting language that would be widely avaiable I would have switched long time ago... > But at least Li and myself did not know that and even while you know > about this "feature", you think it is strange. > I would rather like to be explicit and use a global variable (what harm > is it really?) instead of confusing the next one looking at this piece > of code.... If you really think global variable would be better, then go ahead and send a patch, I do not have a strong feelings about this particular detail. -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH 2/2] tst_test: using SIGTERM to terminate process 2021-05-31 12:54 ` Cyril Hrubis @ 2021-06-01 4:23 ` Joerg Vehlow 0 siblings, 0 replies; 15+ messages in thread From: Joerg Vehlow @ 2021-06-01 4:23 UTC (permalink / raw) To: ltp Hi Cyril, On 5/31/2021 2:54 PM, Cyril Hrubis wrote: >>>> But one more strange thing here. >>>> I wonder why this even works. $pid is used in _tst_kill_test and defined >>>> in _tst_setup_timer as a local variable. >>>> It looks like it is inherited through the call chain and since it is >>>> copied to the background process, it cannot be manipulated by the tests. >>>> Still I would vote for changing this at some point, because it is highly >>>> confusing. >>> That's actually a correct and well defined behavior, if you call a >>> function g from function f the function g has access to the variables >>> local to f. >>> >>> And yes it's confusing, but the alternative is having another global >>> variable which I do not think is much better than this. >> Hm shell code has more strange behavior than I would have ever expected... > Strongly agree here, to be honest if there was as better scripting > language that would be widely avaiable I would have switched long time > ago... > >> But at least Li and myself did not know that and even while you know >> about this "feature", you think it is strange. >> I would rather like to be explicit and use a global variable (what harm >> is it really?) instead of confusing the next one looking at this piece >> of code.... > If you really think global variable would be better, then go ahead and > send a patch, I do not have a strong feelings about this particular > detail. I think there is another solution, to make this at least more explicit without using a new global variable. Passing the pid as a parameter to _tst_timeout_process and _tst_kill_test. Although it is not required, it makes it a lot less implicit and a lot better simpler to read. J?rg ^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH 2/2] tst_test: using SIGTERM to terminate process 2021-05-18 12:26 ` [LTP] [PATCH 2/2] tst_test: using SIGTERM to terminate process Li Wang 2021-05-19 6:46 ` Joerg Vehlow @ 2021-05-31 12:49 ` Cyril Hrubis 2021-05-31 13:20 ` Joerg Vehlow 1 sibling, 1 reply; 15+ messages in thread From: Cyril Hrubis @ 2021-05-31 12:49 UTC (permalink / raw) To: ltp Hi! > lib/newlib_tests/shell/test_timeout.sh | 2 +- > lib/newlib_tests/shell/timeout03.sh | 1 + > testcases/lib/tst_test.sh | 9 +++++---- > 3 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/lib/newlib_tests/shell/test_timeout.sh b/lib/newlib_tests/shell/test_timeout.sh > index b05680cb1..9f31afa32 100755 > --- a/lib/newlib_tests/shell/test_timeout.sh > +++ b/lib/newlib_tests/shell/test_timeout.sh > @@ -28,7 +28,7 @@ timeout02.sh| -10|0| |2 > timeout02.sh| -0.1|0| |0 > timeout02.sh| -1.1|0| |2 > timeout02.sh|-10.1|0| |2 > -timeout03.sh| |0|12|137| | | |Test kill if test does not terminate by SIGINT > +timeout03.sh| |0|12|137| | | |Test kill if test does not terminate by SIGTERM > timeout04.sh| |0| | 2|0|0|1|Verify that timeout is enforced > timeout02.sh| 2|1| 2| |1|0|0|Test termination of timeout process > " > diff --git a/lib/newlib_tests/shell/timeout03.sh b/lib/newlib_tests/shell/timeout03.sh > index cd548d9a2..124e96a84 100755 > --- a/lib/newlib_tests/shell/timeout03.sh > +++ b/lib/newlib_tests/shell/timeout03.sh > @@ -30,6 +30,7 @@ TST_TIMEOUT=1 > > do_test() > { > + trap "tst_res TINFO 'Sorry, timeout03 is still alive'" TERM > tst_res TINFO "testing killing test after TST_TIMEOUT" > > sleep 2 > diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh > index 3a5651c01..66ffde4eb 100644 > --- a/testcases/lib/tst_test.sh > +++ b/testcases/lib/tst_test.sh > @@ -21,7 +21,8 @@ export TST_LIB_LOADED=1 > . tst_security.sh > > # default trap function > -trap "tst_brk TBROK 'test interrupted or timed out'" INT > +trap "tst_brk TBROK 'test interrupted'" INT > +trap "unset _tst_setup_timer_pid; tst_brk TBROK 'test terminated'" TERM I've been looking at this for a while and I think that we should unset the _tst_setup_timer_pid at the end of the _tst_timeout_process() instead, right? Otherwise we will leave the timeout process sleeping if someone sends SIGTERM to the test process from the outside, or do I miss something? -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH 2/2] tst_test: using SIGTERM to terminate process 2021-05-31 12:49 ` Cyril Hrubis @ 2021-05-31 13:20 ` Joerg Vehlow 2021-05-31 12:57 ` Cyril Hrubis 0 siblings, 1 reply; 15+ messages in thread From: Joerg Vehlow @ 2021-05-31 13:20 UTC (permalink / raw) To: ltp Hi, On 5/31/2021 2:49 PM, Cyril Hrubis wrote: > Hi! >> lib/newlib_tests/shell/test_timeout.sh | 2 +- >> lib/newlib_tests/shell/timeout03.sh | 1 + >> testcases/lib/tst_test.sh | 9 +++++---- >> 3 files changed, 7 insertions(+), 5 deletions(-) >> >> diff --git a/lib/newlib_tests/shell/test_timeout.sh b/lib/newlib_tests/shell/test_timeout.sh >> index b05680cb1..9f31afa32 100755 >> --- a/lib/newlib_tests/shell/test_timeout.sh >> +++ b/lib/newlib_tests/shell/test_timeout.sh >> @@ -28,7 +28,7 @@ timeout02.sh| -10|0| |2 >> timeout02.sh| -0.1|0| |0 >> timeout02.sh| -1.1|0| |2 >> timeout02.sh|-10.1|0| |2 >> -timeout03.sh| |0|12|137| | | |Test kill if test does not terminate by SIGINT >> +timeout03.sh| |0|12|137| | | |Test kill if test does not terminate by SIGTERM >> timeout04.sh| |0| | 2|0|0|1|Verify that timeout is enforced >> timeout02.sh| 2|1| 2| |1|0|0|Test termination of timeout process >> " >> diff --git a/lib/newlib_tests/shell/timeout03.sh b/lib/newlib_tests/shell/timeout03.sh >> index cd548d9a2..124e96a84 100755 >> --- a/lib/newlib_tests/shell/timeout03.sh >> +++ b/lib/newlib_tests/shell/timeout03.sh >> @@ -30,6 +30,7 @@ TST_TIMEOUT=1 >> >> do_test() >> { >> + trap "tst_res TINFO 'Sorry, timeout03 is still alive'" TERM >> tst_res TINFO "testing killing test after TST_TIMEOUT" >> >> sleep 2 >> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh >> index 3a5651c01..66ffde4eb 100644 >> --- a/testcases/lib/tst_test.sh >> +++ b/testcases/lib/tst_test.sh >> @@ -21,7 +21,8 @@ export TST_LIB_LOADED=1 >> . tst_security.sh >> >> # default trap function >> -trap "tst_brk TBROK 'test interrupted or timed out'" INT >> +trap "tst_brk TBROK 'test interrupted'" INT >> +trap "unset _tst_setup_timer_pid; tst_brk TBROK 'test terminated'" TERM > I've been looking at this for a while and I think that we should unset > the _tst_setup_timer_pid at the end of the _tst_timeout_process() > instead, right? That won't work or am I missing something? ;) We are in a subshell in _tst_timeout_process, it cannot manipulate the environment of the parent or is my mental modal of how shell works wrong again?. > Otherwise we will leave the timeout process sleeping if someone sends > SIGTERM to the test process from the outside, or do I miss something? Yes, that is one of the reasons why I initially suggested a different signal for timeout. That would cleanly separate the logic. But on the other hand when will SIGTERM be sent from the outside? This is probably only happening on shutdown? In that case it is not really a problem. If someone wants to terminate gracefully using SIGTERM, he could send it to the process group. That would kill the timeout process as well. J?rg ^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH 2/2] tst_test: using SIGTERM to terminate process 2021-05-31 13:20 ` Joerg Vehlow @ 2021-05-31 12:57 ` Cyril Hrubis 2021-06-01 2:08 ` Li Wang 0 siblings, 1 reply; 15+ messages in thread From: Cyril Hrubis @ 2021-05-31 12:57 UTC (permalink / raw) To: ltp Hi! > >> # default trap function > >> -trap "tst_brk TBROK 'test interrupted or timed out'" INT > >> +trap "tst_brk TBROK 'test interrupted'" INT > >> +trap "unset _tst_setup_timer_pid; tst_brk TBROK 'test terminated'" TERM > > I've been looking at this for a while and I think that we should unset > > the _tst_setup_timer_pid at the end of the _tst_timeout_process() > > instead, right? > That won't work or am I missing something? ;) We are in a subshell in > _tst_timeout_process, it cannot manipulate the environment of the parent > or is my mental modal of how shell works wrong again?. Nah, you are right this time, I missed that it runs on a background. > > Otherwise we will leave the timeout process sleeping if someone sends > > SIGTERM to the test process from the outside, or do I miss something? > Yes, that is one of the reasons why I initially suggested a different > signal for timeout. That would cleanly separate the logic. > But on the other hand when will SIGTERM be sent from the outside? This > is probably only happening on shutdown? In that case it is not really a > problem. > If someone wants to terminate gracefully using SIGTERM, he could send it > to the process group. That would kill the timeout process as well. Okay. Let's go with this version, it's simple enough and anything that would behave better would only overcomplicate the code again. For the patch: Reviewed-by: Cyril Hrubis <chrubis@suse.cz> -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH 2/2] tst_test: using SIGTERM to terminate process 2021-05-31 12:57 ` Cyril Hrubis @ 2021-06-01 2:08 ` Li Wang 2021-06-01 4:03 ` Petr Vorel 0 siblings, 1 reply; 15+ messages in thread From: Li Wang @ 2021-06-01 2:08 UTC (permalink / raw) To: ltp > Okay. Let's go with this version, it's simple enough and anything that > would behave better would only overcomplicate the code again. > > For the patch: > > Reviewed-by: Cyril Hrubis <chrubis@suse.cz> Thanks for the review, I helped merge the patch V2. (tiny updates in v2: 'add comments + remove trap' from timeout03) -- Regards, Li Wang ^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH 2/2] tst_test: using SIGTERM to terminate process 2021-06-01 2:08 ` Li Wang @ 2021-06-01 4:03 ` Petr Vorel 0 siblings, 0 replies; 15+ messages in thread From: Petr Vorel @ 2021-06-01 4:03 UTC (permalink / raw) To: ltp Hi Li, all, > > Okay. Let's go with this version, it's simple enough and anything that > > would behave better would only overcomplicate the code again. > > For the patch: > > Reviewed-by: Cyril Hrubis <chrubis@suse.cz> > Thanks for the review, I helped merge the patch V2. > (tiny updates in v2: 'add comments + remove trap' from timeout03) Thanks a lot for finishing this, sorry for late reply. Kind regards, Petr ^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH 1/2] tst_test: discard the stderr output 2021-05-18 12:26 [LTP] [PATCH 1/2] tst_test: discard the stderr output Li Wang 2021-05-18 12:26 ` [LTP] [PATCH 2/2] tst_test: using SIGTERM to terminate process Li Wang @ 2021-05-18 13:50 ` Cyril Hrubis 2021-05-18 14:35 ` Petr Vorel 2 siblings, 0 replies; 15+ messages in thread From: Cyril Hrubis @ 2021-05-18 13:50 UTC (permalink / raw) To: ltp Hi! > We can merge this before the new release. Looks safe enough for the release. Reviewed-by: Cyril Hrubis <chrubis@suse.cz> -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 15+ messages in thread
* [LTP] [PATCH 1/2] tst_test: discard the stderr output 2021-05-18 12:26 [LTP] [PATCH 1/2] tst_test: discard the stderr output Li Wang 2021-05-18 12:26 ` [LTP] [PATCH 2/2] tst_test: using SIGTERM to terminate process Li Wang 2021-05-18 13:50 ` [LTP] [PATCH 1/2] tst_test: discard the stderr output Cyril Hrubis @ 2021-05-18 14:35 ` Petr Vorel 2 siblings, 0 replies; 15+ messages in thread From: Petr Vorel @ 2021-05-18 14:35 UTC (permalink / raw) To: ltp Hi Li, thanks for the fix, merged! Kind regards, Petr ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-06-01 4:23 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-18 12:26 [LTP] [PATCH 1/2] tst_test: discard the stderr output Li Wang 2021-05-18 12:26 ` [LTP] [PATCH 2/2] tst_test: using SIGTERM to terminate process Li Wang 2021-05-19 6:46 ` Joerg Vehlow 2021-05-19 8:52 ` Li Wang 2021-05-31 12:35 ` Cyril Hrubis 2021-05-31 13:16 ` Joerg Vehlow 2021-05-31 12:54 ` Cyril Hrubis 2021-06-01 4:23 ` Joerg Vehlow 2021-05-31 12:49 ` Cyril Hrubis 2021-05-31 13:20 ` Joerg Vehlow 2021-05-31 12:57 ` Cyril Hrubis 2021-06-01 2:08 ` Li Wang 2021-06-01 4:03 ` Petr Vorel 2021-05-18 13:50 ` [LTP] [PATCH 1/2] tst_test: discard the stderr output Cyril Hrubis 2021-05-18 14:35 ` Petr Vorel
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.