[llvm] r209951 - Fix the behavior of ExecuteAndWait with a non-zero timeout.

[Peter Collingbourne]

On Sat, May 31, 2014 at 05:15:45AM +0300, Alp Toker wrote:
> The real problem here is that the sys::Wait() prototype has unclear  
> semantics but the commit only fixes the symptom.
>
> SecondsToWait and WaitUntilTerminates are mutually exclusive, so having  
> SecondsToWait=-1 signify WaitUntilTerminates and removing the  
> WaitUntilTerminates parameter is a better way to make the interface 
> robust.

Agree; we could probably just use zero to mean no timeout, which is a
convention elsewhere.

In any case, this API needs to be cleaned up in more ways than that. For one
thing, a client cannot definitively tell if there was a timeout (a return
value of -2 can also mean a number of other things).

> With that change it'll be easier to inspect the code for correctness,  
> and you may consider removing/disabling the unit test which steals a  
> second of testing time that could be better used to run a few thousand  
> other tests.

Removing the unit test would mean losing test coverage of ExecuteAndWait's
timeout parameter. The other tests would be running in parallel so it probably
isn't so bad.

We might consider making the timeout more granular -- I believe this is
possible on all platforms. I noticed that there are other Support tests that
also sleep for a second and it may be useful to apply a similar fix there.

Thanks,
-- 
Peter