automake-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] Remove a race condition from test cond5.test (was: cond5.test sp


From: Stefano Lattarini
Subject: [PATCH] Remove a race condition from test cond5.test (was: cond5.test spurious failure)
Date: Fri, 6 Aug 2010 16:17:50 +0200
User-agent: KMail/1.13.3 (Linux/2.6.30-2-686; KDE/4.4.4; i686; ; )

Peter, thanks for your analysis.  So it seems a bug in the kill 
builtin of the MSYS shell after all, just like you suggested.

At Friday 06 August 2010, Peter Rosin wrote:
> [CUT]
> 
> tmp3.test "always" works, but takes different branches depending
> on if kill worked or not.
> 
> kill failure:
> 
> === Running test ./tmp3.test
> ++ pwd
> /c/cygwin/home/peda/automake/git/automake/tests/tmp3.dir
> + set -e
> + cat
> + cat
> + aclocal-1.11 -Werror
> + pid=10128
> + try=1
> + automake-1.11 --foreign -Werror -Wall
> + test 1 -le 30
> + kill -0 10128
> ./tmp3.test: line 54: kill: (10128) - No such process
> + cat stderr
So, it's probably still empty because the automake process hasn't had 
the time to write the error message to stderr...
> + grep 'variable.*OPT_SRC.*recursively defined' stderr
> + sleep 20
... but if we wait a little longer ...
> + cat stderr
> Makefile.am:8: variable `OPT_SRC' recursively defined
> + grep 'variable.*OPT_SRC.*recursively defined' stderr
> Makefile.am:8: variable `OPT_SRC' recursively defined
... automake will finally print the error message, as expected.
No bug in automake thus.
> + Exit 0

> kill success:
> [[CUT: everything works smoothly, as expected]]
 
> So, nothing unexpected given my previous message...
Yep.

Can you please try the attached patch (a "revived" version of an older 
patch of mine), and see if the problem disappear?

Thanks,
   Stefano

From 18013a06dbea6b59e4cebd653c481786fc0cafe1 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Fri, 6 Aug 2010 16:04:54 +0200
Subject: [PATCH] Remove a race condition from test cond5.test.

* tests/cond5.test: Do not blindly try to kill the pid of the
backgrounded Automake process after the sleep, since it might
have terminated by itself, and its PID reused by a new and
unrelated process.  Instead, rely on a more complex but more
correct combo of wrapper script(s) and temporary file(s).
This also works around a bug in the MSYS/MinGW shell.
Report and analysis by Peter Rosin (<address@hidden>),
final patch by Stefano Lattarini.
---
 ChangeLog        |   12 ++++++++++++
 tests/cond5.test |   32 ++++++++++++++++++++++++++------
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 7edcae8..474b655 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2010-08-06  Stefano Lattarini  <address@hidden>
+
+       Remove a race condition from test cond5.test.
+       * tests/cond5.test: Do not blindly try to kill the pid of the
+       backgrounded Automake process after the sleep, since it might
+       have terminated by itself, and its PID reused by a new and
+       unrelated process.  Instead, rely on a more complex but more
+       correct combo of wrapper script(s) and temporary file(s).
+       This also works around a bug in the MSYS/MinGW shell.
+       Report and analysis by Peter Rosin (<address@hidden>),
+       final patch by Stefano Lattarini.
+
 2010-07-21  Stefano Lattarini  <address@hidden>
 
        Modernize and improve test scripts `subdir*.test'.
diff --git a/tests/cond5.test b/tests/cond5.test
index 84afdd0..671c6b0 100755
--- a/tests/cond5.test
+++ b/tests/cond5.test
@@ -44,25 +44,45 @@ END
 
 # The bug is that automake hangs.  So we give it an appropriate grace
 # time, then kill it if necessary.
-$ACLOCAL
-$AUTOMAKE 2>stderr &
+# We use a hacky wrapper script to avoid (or reduce to a really low
+# minimum) race conditions w.r.t. process PID, and to avoid possible
+# weird bugs of the kill builtin of MSYS/MinGW shell(s).
+
+cat > no-race.sh <<'END'
+#!/bin/sh
+"$@" 2>stderr &
 pid=$!
+echo $pid > prog-not-finished
+wait $pid
+rc=$?
+rm -f prog-not-finished
+echo $rc > rc
+exit $rc
+END
+
+$ACLOCAL
+: > prog-not-finished  # to be sure it will truly be there from the start
+$SHELL -x ./no-race.sh $AUTOMAKE &
+norace_pid=$!
 
 # Make at most 30 tries, one every 10 seconds (= 300 seconds = 5 min).
 try=1
 while test $try -le 30; do
-  if kill -0 $pid; then
-    : process $pid is still alive, wait and retry
+  if test -f prog-not-finished; then
+    : process `cat prog-not-finished` is still alive, wait and retry
     sleep 10
     try=`expr $try + 1`
   else
     cat stderr >&2
     # Automake must fail with a proper error message.
+    test x"1" = x"`cat rc`"
     grep 'variable.*OPT_SRC.*recursively defined' stderr
     Exit 0
   fi
 done
 # The automake process probably hung.  Kill it, and exit with failure.
-echo "$me: automake process $pid hung"
-kill $pid
+# And yes, the repated command substitutions with `cat' are meant.
+echo "$me: automake process `cat prog-not-finished` hung"
+(kill `cat prog-not-finished`) # some shells require this subshell
+wait $norace_pid
 Exit 1
-- 
1.7.1


reply via email to

[Prev in Thread] Current Thread [Next in Thread]