[PATCH] test: Remove race between commands run in the same context

[PATCH] test: Remove race between commands run in the same context

[David Gibson]

context_run() has a race condition if two commands are run in close
proximity (generally involving at least one in the background).  Because we
always use the same name for the temporary fifo files, if another command
is issued while the fifos for the first still exist, mkfifo will fail,
typically causing the entire test script to jam.

Create unique names for the temporary fifos to avoid this problem.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 test/lib/context | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/lib/context b/test/lib/context
index d9d8260..09cad89 100644
--- a/test/lib/context
+++ b/test/lib/context
@@ -93,8 +93,8 @@ context_run() {
 	__name="$1"
 	__log="${LOGDIR}/context_${__name}.log"
 	__enter="${STATESETUP}/context_${__name}.enter"
-	__stdout="${STATESETUP}/context_${__name}.stdout"
-	__stderr="${STATESETUP}/context_${__name}.stderr"
+	__stdout="$(mktemp -u "${STATESETUP}/context_${__name}.stdout.XXXXXXXX")"
+	__stderr="$(mktemp -u "${STATESETUP}/context_${__name}.stderr.XXXXXXXX")"
 	shift
 	echo "$*" >> "${__log}"
 	mkfifo "${__stdout}" "${__stderr}"
-- 
@@ -93,8 +93,8 @@ context_run() {
 	__name="$1"
 	__log="${LOGDIR}/context_${__name}.log"
 	__enter="${STATESETUP}/context_${__name}.enter"
-	__stdout="${STATESETUP}/context_${__name}.stdout"
-	__stderr="${STATESETUP}/context_${__name}.stderr"
+	__stdout="$(mktemp -u "${STATESETUP}/context_${__name}.stdout.XXXXXXXX")"
+	__stderr="$(mktemp -u "${STATESETUP}/context_${__name}.stderr.XXXXXXXX")"
 	shift
 	echo "$*" >> "${__log}"
 	mkfifo "${__stdout}" "${__stderr}"
-- 
2.39.2

Re: [PATCH] test: Remove race between commands run in the same context

[Stefano Brivio]

On Wed,  5 Apr 2023 11:56:42 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> context_run() has a race condition if two commands are run in close
> proximity (generally involving at least one in the background).  Because we
> always use the same name for the temporary fifo files, if another command
> is issued while the fifos for the first still exist, mkfifo will fail,
> typically causing the entire test script to jam.
> 
> Create unique names for the temporary fifos to avoid this problem.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  test/lib/context | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/test/lib/context b/test/lib/context
> index d9d8260..09cad89 100644
> --- a/test/lib/context
> +++ b/test/lib/context
> @@ -93,8 +93,8 @@ context_run() {
>  	__name="$1"
>  	__log="${LOGDIR}/context_${__name}.log"
>  	__enter="${STATESETUP}/context_${__name}.enter"
> -	__stdout="${STATESETUP}/context_${__name}.stdout"
> -	__stderr="${STATESETUP}/context_${__name}.stderr"
> +	__stdout="$(mktemp -u "${STATESETUP}/context_${__name}.stdout.XXXXXXXX")"
> +	__stderr="$(mktemp -u "${STATESETUP}/context_${__name}.stderr.XXXXXXXX")"

In practice, this is never going to be an issue, but strictly
speaking, -u doesn't guarantee the name is unique.

On the other hand we don't need a loop (or further directories) with
this, so the simplicity of it might be preferable over correctness...

-- 
Stefano

Re: [PATCH] test: Remove race between commands run in the same context

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1886 bytes --]

On Wed, Apr 05, 2023 at 02:23:24PM +0200, Stefano Brivio wrote:
> On Wed,  5 Apr 2023 11:56:42 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > context_run() has a race condition if two commands are run in close
> > proximity (generally involving at least one in the background).  Because we
> > always use the same name for the temporary fifo files, if another command
> > is issued while the fifos for the first still exist, mkfifo will fail,
> > typically causing the entire test script to jam.
> > 
> > Create unique names for the temporary fifos to avoid this problem.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  test/lib/context | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/test/lib/context b/test/lib/context
> > index d9d8260..09cad89 100644
> > --- a/test/lib/context
> > +++ b/test/lib/context
> > @@ -93,8 +93,8 @@ context_run() {
> >  	__name="$1"
> >  	__log="${LOGDIR}/context_${__name}.log"
> >  	__enter="${STATESETUP}/context_${__name}.enter"
> > -	__stdout="${STATESETUP}/context_${__name}.stdout"
> > -	__stderr="${STATESETUP}/context_${__name}.stderr"
> > +	__stdout="$(mktemp -u "${STATESETUP}/context_${__name}.stdout.XXXXXXXX")"
> > +	__stderr="$(mktemp -u "${STATESETUP}/context_${__name}.stderr.XXXXXXXX")"
> 
> In practice, this is never going to be an issue, but strictly
> speaking, -u doesn't guarantee the name is unique.

Yeah, I know.

> On the other hand we don't need a loop (or further directories) with
> this, so the simplicity of it might be preferable over correctness...

Right, that's the conclusion I came to as well.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] test: Remove race between commands run in the same context

[Stefano Brivio]

On Wed,  5 Apr 2023 11:56:42 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> context_run() has a race condition if two commands are run in close
> proximity (generally involving at least one in the background).  Because we
> always use the same name for the temporary fifo files, if another command
> is issued while the fifos for the first still exist, mkfifo will fail,
> typically causing the entire test script to jam.
> 
> Create unique names for the temporary fifos to avoid this problem.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Also applied.

-- 
Stefano