[LTP] [PATCH v2] test_children_cleanup.sh: Fix race condition

[LTP] [PATCH v2] test_children_cleanup.sh: Fix race condition

[Martin Doucha]

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
+		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


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

Re: [LTP] [PATCH v2] test_children_cleanup.sh: Fix race condition

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 1826 bytes --]

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
>
>

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 3020 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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

Re: [LTP] [PATCH v2] test_children_cleanup.sh: Fix race condition

[Cyril Hrubis]

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

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2] test_children_cleanup.sh: Fix race condition

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 1779 bytes --]

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~

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 2745 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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