[lttng-dev] [PATCH lttng-tools] Fix: mi test: possible race between listing UST events and testapp start

[lttng-dev] [PATCH lttng-tools] Fix: mi test: possible race between listing UST events and testapp start

[Jonathan Rajotte]

Signed-off-by: Jonathan Rajotte <jonathan.r.julien at gmail.com>
---
 tests/regression/tools/mi/test_mi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/regression/tools/mi/test_mi b/tests/regression/tools/mi/test_mi
index fa46b51..7acec18 100755
--- a/tests/regression/tools/mi/test_mi
+++ b/tests/regression/tools/mi/test_mi
@@ -507,6 +507,11 @@ function test_list_ust_event ()
 
 	#Begin testing
 	$TESTAPP_BIN $NR_USEC_WAIT & 2>/dev/null
+	pid=$!
+
+	#Wait for TESTAPP to run
+	while ! kill -0 $pid 2> /dev/null; do :; done
+
 	list_lttng_with_opts "-u -f"
 	$XML_VALIDATE $OUTPUT_DEST
 	ok $? "Mi test: list ust event xsd validation"
-- 
1.9.1

[lttng-dev] [PATCH lttng-tools] Fix: mi test: possible race between listing UST events and testapp start

[Nathan Lynch]

On 10/26/2014 08:21 PM, Jonathan Rajotte wrote:
> Signed-off-by: Jonathan Rajotte <jonathan.r.julien at gmail.com>

More description of the change is needed IMO.

> ---
>  tests/regression/tools/mi/test_mi | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/tests/regression/tools/mi/test_mi b/tests/regression/tools/mi/test_mi
> index fa46b51..7acec18 100755
> --- a/tests/regression/tools/mi/test_mi
> +++ b/tests/regression/tools/mi/test_mi
> @@ -507,6 +507,11 @@ function test_list_ust_event ()
>  
>  	#Begin testing
>  	$TESTAPP_BIN $NR_USEC_WAIT & 2>/dev/null
> +	pid=$!
> +
> +	#Wait for TESTAPP to run
> +	while ! kill -0 $pid 2> /dev/null; do :; done
> +
>  	list_lttng_with_opts "-u -f"

Is this an attempt to give the test program enough time to register
tracepoints?

If so, I agree there is a race, but I don't think this is the right way
to address it.  I would not expect the while ! kill loop to repeat at
all; if you remove the stderr redirection do you see any output, ever?

The exit status from kill $! is not a reliable indication that the shell
has execve'd the program in the background, and even if it were, it
would not be a reliable indication of how far the program has progressed
(e.g. whether it has got far enough along to register tracepoints).

If a test program needs to run in the background, yet needs to reach a
certain state before the rest of the test should proceed, then it should
have a way to indicate to the test harness that it has reached the
desired state.  The shell's rudimentary facilities for managing
concurrency are not sufficient.

[lttng-dev] [PATCH lttng-tools] Fix: mi test: possible race between listing UST events and testapp start

[Jonathan Rajotte]

Hey

On Mon, Oct 27, 2014 at 4:48 PM, Nathan Lynch <Nathan_Lynch at mentor.com>
wrote:

> On 10/26/2014 08:21 PM, Jonathan Rajotte wrote:
> > Signed-off-by: Jonathan Rajotte <jonathan.r.julien at gmail.com>
>
> More description of the change is needed IMO.
>
> > ---
> >  tests/regression/tools/mi/test_mi | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/tests/regression/tools/mi/test_mi
> b/tests/regression/tools/mi/test_mi
> > index fa46b51..7acec18 100755
> > --- a/tests/regression/tools/mi/test_mi
> > +++ b/tests/regression/tools/mi/test_mi
> > @@ -507,6 +507,11 @@ function test_list_ust_event ()
> >
> >       #Begin testing
> >       $TESTAPP_BIN $NR_USEC_WAIT & 2>/dev/null
> > +     pid=$!
> > +
> > +     #Wait for TESTAPP to run
> > +     while ! kill -0 $pid 2> /dev/null; do :; done
> > +
> >       list_lttng_with_opts "-u -f"
>
> Is this an attempt to give the test program enough time to register
> tracepoints?
>

A poor one... if one at all.... for which I needed feedback (missing rfc
tag here) because I could not simply find any simple way. See [1]


> If so, I agree there is a race, but I don't think this is the right way
> to address it.

   I would not expect the while ! kill loop to repeat at

> all;


The presence of ":" assure the execution of the while loop.
A simple test on a process that does not exist yield a lot of printing:

while ! kill -0 500; do :; done

Anyway it does not solve the problem. ;) and I'm not sure you were talking
about that.

if you remove the stderr redirection do you see any output, ever?

The exit status from kill $! is not a reliable indication that the shell
> has execve'd the program in the background, and even if it were, it
> would not be a reliable indication of how far the program has progressed


+1

(e.g. whether it has got far enough along to register tracepoints).
>
>

> If a test program needs to run in the background, yet needs to reach a
> certain state before the rest of the test should proceed, then it should
> have a way to indicate to the test harness that it has reached the
> desired state.  The shell's rudimentary facilities for managing
> concurrency are not sufficient.
>

I'm pushing limits here regarding simplicity: how about a trap and signal
exchange between the testapp... might give it a shot. Or a simple sleep...

Thanks, I'll think about it a little more.

[1]:http://meta.wikimedia.org/wiki/Cunningham%27s_Law
Bonus xkcd: http://xkcd.com/386/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lttng.org/pipermail/lttng-dev/attachments/20141027/838b34b6/attachment-0001.html>

[lttng-dev] [PATCH lttng-tools] Fix: mi test: possible race between listing UST events and testapp start

[David Goulet]

On 26 Oct (21:21:46), Jonathan Rajotte wrote:
> Signed-off-by: Jonathan Rajotte <jonathan.r.julien at gmail.com>
> ---
>  tests/regression/tools/mi/test_mi | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/tests/regression/tools/mi/test_mi b/tests/regression/tools/mi/test_mi
> index fa46b51..7acec18 100755
> --- a/tests/regression/tools/mi/test_mi
> +++ b/tests/regression/tools/mi/test_mi
> @@ -507,6 +507,11 @@ function test_list_ust_event ()
>  
>  	#Begin testing
>  	$TESTAPP_BIN $NR_USEC_WAIT & 2>/dev/null
> +	pid=$!
> +
> +	#Wait for TESTAPP to run
> +	while ! kill -0 $pid 2> /dev/null; do :; done

So of what I understand here, this tests the existance of the PID but
wondering if it really fixes the race. Even if the PID indeed exists,
still it might not have registered to the session daemon.

I think one way to fix that is to have a "lttng list -u | grep $PID"
test and if the output is not empty, the application did registered.
Also, you can use the "create file" option of gen-ust-events. You can
pass a fourth argument to the app, once a tracepoint is hit, it will
create the file you gave with the full path. You can then loop on that.
There are some tests that already uses that, you can check it out.

Finally, that test should actually spawn the application with a long
life time and kill it at the end so we don't race between very long
commands and the application actually quiting gracefully.

Cheers!
David

> +
>  	list_lttng_with_opts "-u -f"
>  	$XML_VALIDATE $OUTPUT_DEST
>  	ok $? "Mi test: list ust event xsd validation"
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 603 bytes
Desc: Digital signature
URL: <http://lists.lttng.org/pipermail/lttng-dev/attachments/20141029/4a0d1c16/attachment.sig>