| Message ID | 20220215102053.1790-1-mdoucha@suse.cz |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] test_children_cleanup.sh: Fix race condition | expand |
On Tue, Feb 15, 2022 at 6:20 PM Martin Doucha <mdoucha@suse.cz> wrote: > Processes can stay alive for a short while even after receiving SIGKILL. > Give the child in subprocess cleanup libtest up to 5 seconds to fully exit > or change state to zombie before reporting that it was left behind. > > Signed-off-by: Martin Doucha <mdoucha@suse.cz> > Reviewed-by: Li Wang <liwang@redhat.com> > --- > > Changes since v1: Report success even if the child gets stuck in zombie > state > > lib/newlib_tests/test_children_cleanup.sh | 23 +++++++++++++++++------ > 1 file changed, 17 insertions(+), 6 deletions(-) > > diff --git a/lib/newlib_tests/test_children_cleanup.sh > b/lib/newlib_tests/test_children_cleanup.sh > index 4b4e8b2f0..23408c1bc 100755 > --- a/lib/newlib_tests/test_children_cleanup.sh > +++ b/lib/newlib_tests/test_children_cleanup.sh > @@ -10,10 +10,21 @@ rm "$TMPFILE" > if [ "x$CHILD_PID" = "x" ]; then > echo "TFAIL: Child process was not created" > exit 1 > -elif ! kill -s 0 $CHILD_PID &>/dev/null; then > - echo "TPASS: Child process was cleaned up" > - exit 0 > -else > - echo "TFAIL: Child process was left behind" > - exit 1 > fi > + > +# The child process can stay alive for a short while even after receiving > +# SIGKILL, especially if the system is under heavy load. Wait up to 5 > seconds > +# for it to fully exit. > +for i in `seq 6`; do > + CHILD_STATE=`sed -ne 's/^State:\s*\([A-Z]\).*$/\1/p' > "/proc/$CHILD_PID/status" 2>/dev/null` > + > + if [ ! -e "/proc/$CHILD_PID" ] || [ "x$CHILD_STATE" = "xZ" ]; then > + echo "TPASS: Child process was cleaned up" > + exit 0 > + fi > + > + sleep 1 > +done > + > +echo "TFAIL: Child process was left behind" > +exit 1 > -- > 2.34.1 > >
Hi! > Processes can stay alive for a short while even after receiving SIGKILL. > Give the child in subprocess cleanup libtest up to 5 seconds to fully exit > or change state to zombie before reporting that it was left behind. > > Signed-off-by: Martin Doucha <mdoucha@suse.cz> > --- > > Changes since v1: Report success even if the child gets stuck in zombie state > > lib/newlib_tests/test_children_cleanup.sh | 23 +++++++++++++++++------ > 1 file changed, 17 insertions(+), 6 deletions(-) > > diff --git a/lib/newlib_tests/test_children_cleanup.sh b/lib/newlib_tests/test_children_cleanup.sh > index 4b4e8b2f0..23408c1bc 100755 > --- a/lib/newlib_tests/test_children_cleanup.sh > +++ b/lib/newlib_tests/test_children_cleanup.sh > @@ -10,10 +10,21 @@ rm "$TMPFILE" > if [ "x$CHILD_PID" = "x" ]; then > echo "TFAIL: Child process was not created" > exit 1 > -elif ! kill -s 0 $CHILD_PID &>/dev/null; then > - echo "TPASS: Child process was cleaned up" > - exit 0 > -else > - echo "TFAIL: Child process was left behind" > - exit 1 > fi > + > +# The child process can stay alive for a short while even after receiving > +# SIGKILL, especially if the system is under heavy load. Wait up to 5 seconds > +# for it to fully exit. > +for i in `seq 6`; do > + CHILD_STATE=`sed -ne 's/^State:\s*\([A-Z]\).*$/\1/p' "/proc/$CHILD_PID/status" 2>/dev/null` > + > + if [ ! -e "/proc/$CHILD_PID" ] || [ "x$CHILD_STATE" = "xZ" ]; then As long as we have the variable inside the quotes there is no point in adding the 'x'. Other than that: Reviewed-by: Cyril Hrubis <chrubis@suse.cz> > + echo "TPASS: Child process was cleaned up" > + exit 0 > + fi > + > + sleep 1 > +done > + > +echo "TFAIL: Child process was left behind" > +exit 1
On Tue, Feb 15, 2022 at 8:18 PM Cyril Hrubis <chrubis@suse.cz> wrote: > Hi! > > Processes can stay alive for a short while even after receiving SIGKILL. > > Give the child in subprocess cleanup libtest up to 5 seconds to fully > exit > > or change state to zombie before reporting that it was left behind. > > > > Signed-off-by: Martin Doucha <mdoucha@suse.cz> > > --- > > > > Changes since v1: Report success even if the child gets stuck in zombie > state > > > > lib/newlib_tests/test_children_cleanup.sh | 23 +++++++++++++++++------ > > 1 file changed, 17 insertions(+), 6 deletions(-) > > > > diff --git a/lib/newlib_tests/test_children_cleanup.sh > b/lib/newlib_tests/test_children_cleanup.sh > > index 4b4e8b2f0..23408c1bc 100755 > > --- a/lib/newlib_tests/test_children_cleanup.sh > > +++ b/lib/newlib_tests/test_children_cleanup.sh > > @@ -10,10 +10,21 @@ rm "$TMPFILE" > > if [ "x$CHILD_PID" = "x" ]; then > > echo "TFAIL: Child process was not created" > > exit 1 > > -elif ! kill -s 0 $CHILD_PID &>/dev/null; then > > - echo "TPASS: Child process was cleaned up" > > - exit 0 > > -else > > - echo "TFAIL: Child process was left behind" > > - exit 1 > > fi > > + > > +# The child process can stay alive for a short while even after > receiving > > +# SIGKILL, especially if the system is under heavy load. Wait up to 5 > seconds > > +# for it to fully exit. > > +for i in `seq 6`; do > > + CHILD_STATE=`sed -ne 's/^State:\s*\([A-Z]\).*$/\1/p' > "/proc/$CHILD_PID/status" 2>/dev/null` > > + > > + if [ ! -e "/proc/$CHILD_PID" ] || [ "x$CHILD_STATE" = "xZ" ]; then > > As long as we have the variable inside the quotes there is no point in > adding the 'x'. > Pushed with a note added and this fix. Thanks~
diff --git a/lib/newlib_tests/test_children_cleanup.sh b/lib/newlib_tests/test_children_cleanup.sh index 4b4e8b2f0..23408c1bc 100755 --- a/lib/newlib_tests/test_children_cleanup.sh +++ b/lib/newlib_tests/test_children_cleanup.sh @@ -10,10 +10,21 @@ rm "$TMPFILE" if [ "x$CHILD_PID" = "x" ]; then echo "TFAIL: Child process was not created" exit 1 -elif ! kill -s 0 $CHILD_PID &>/dev/null; then - echo "TPASS: Child process was cleaned up" - exit 0 -else - echo "TFAIL: Child process was left behind" - exit 1 fi + +# The child process can stay alive for a short while even after receiving +# SIGKILL, especially if the system is under heavy load. Wait up to 5 seconds +# for it to fully exit. +for i in `seq 6`; do + CHILD_STATE=`sed -ne 's/^State:\s*\([A-Z]\).*$/\1/p' "/proc/$CHILD_PID/status" 2>/dev/null` + + if [ ! -e "/proc/$CHILD_PID" ] || [ "x$CHILD_STATE" = "xZ" ]; then + echo "TPASS: Child process was cleaned up" + exit 0 + fi + + sleep 1 +done + +echo "TFAIL: Child process was left behind" +exit 1
Processes can stay alive for a short while even after receiving SIGKILL. Give the child in subprocess cleanup libtest up to 5 seconds to fully exit or change state to zombie before reporting that it was left behind. Signed-off-by: Martin Doucha <mdoucha@suse.cz> --- Changes since v1: Report success even if the child gets stuck in zombie state lib/newlib_tests/test_children_cleanup.sh | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-)