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 <stefano.lattar...@gmail.com> 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 (<p...@lysator.liu.se>), 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 <stefano.lattar...@gmail.com> + + 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 (<p...@lysator.liu.se>), + final patch by Stefano Lattarini. + 2010-07-21 Stefano Lattarini <stefano.lattar...@gmail.com> 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