Code Review - 6736316 TEST_BUG: Timeout value in java/util/concurrent/locks/Lock/FlakyMutex.java is insufficient

[David Holmes]

Hi Gary,

On 22/11/2011 5:26 AM, Gary Adams wrote:
> The original complaint about the flakey mutex regression test is
> that it was failing on slower machines. The delay at the end of
> the processing is unnecessarily restrictive. Since the test harness
> will terminate after 120 seconds if the test hangs, the test does
> not have to terminate more quickly.
>
> Here's a propose compromise that allows the service up to 120 seconds
> to terminate and allows an early exit if the service completes sooner.
> Alternatively, we could just increase the timeout to the full 120 seconds
> which will also exit sooner if the service completes earlier.
>
> http://cr.openjdk.java.net/~gadams/6736316/

The loop serves no purpose. If you want to wait a maximum of 120 seconds 
then use a timeout of 120 seconds. As you said, awaitTermination will 
return earlier if termination completes.

I support raising the timeout rather than waiting forever, as these 
tests should be able to run standalone and in that case they should not 
hang upon encountering a bug. I'm wary of assuming there is a 
higher-level test harness involved. Also note that java/util/concurrent 
tests are generally updated in collaboration with Doug Lea as he also 
maintains these tests for the jsr166 repository - and runs them.

I do have a concern that raising the timeouts/delays too high means that 
they will no longer be of any use in detecting general unexpected 
slowdowns. This is a delicate balance between accommodating slow 
machines and flagging problems with unexpected slowdowns. Maybe there is 
nothing we can do about that, but I want to flag it.

Thanks,
David

>