Hi Stefano, * Stefano Lattarini wrote on Mon, Jul 19, 2010 at 10:41:13PM CEST: > And here is a follow-up patch to reduce possible race conditions > w.r.t. reuse of process PIDs. The patch is against the maint branch.
I'm sorry but I have issues here, see below. > 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). > Necessity of this fix suggested by Ralf Wildenhues. > --- a/tests/cond5.test > +++ b/tests/cond5.test > @@ -44,25 +44,42 @@ 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. > + > +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 & > > # 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. That doesn't seem ideal to me though. If the file is removed at the right time, cat or kill produces an unwanted error message. > +echo "$me: automake process `cat prog-not-finished` hung" > +(kill `cat prog-not-finished`) # some shells require this subshell > Exit 1 Here, we'd probably want to wait for the $SHELL -x process to finish. In reading the original code (i.e., the one before this patch), I see that I was mistaken last time in that the default code path does not try to kill a process with an old, long-gone PID. Sorry about my misinterpretation, but upon reading again I think there is no change warranted. The change that you already pushed was good, though. Thanks. Cheers, Ralf