automake-patches
[Top][All Lists]
Advanced

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

Re: [PATCH] Remove a race condition from test cond5.test.


From: Stefano Lattarini
Subject: Re: [PATCH] Remove a race condition from test cond5.test.
Date: Wed, 21 Jul 2010 21:24:10 +0200
User-agent: KMail/1.13.3 (Linux/2.6.30-2-686; KDE/4.4.4; i686; ; )

At Wednesday 21 July 2010, Ralf Wildenhues wrote:
> 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
> > [CUT]
> > 
> > -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.
Yes, but that's not a problem IMO, since the test is going to fail 
anyway if this code path has been taken.

> > +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.
Definitely right.  Not waiting for its termination is a more serious 
error which could cause intermittent weird behaviours.

> 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.
OK, let's drop this patch then.  It was quite ugly anyway.
> The change that you already pushed was
> good, though.
Good!

Regards,
   Stefano



reply via email to

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