Code Review 7021010: java/lang/Thread/ThreadStateTest.java fails intermittently

[Mandy Chung]

  Hi Chris,

On 6/20/11 8:54 PM, Chris Hegarty wrote:
> java/lang/Thread/ThreadStateTest.java can fail with when 
> checkThreadState finds an unexpected state.
>
> Exception in thread "main" java.lang.RuntimeException: MyThread 
> expected to have TERMINATED but got RUNNABLE
>     at ThreadStateTest.checkThreadState(ThreadStateTest.java:119)
>     at ThreadStateTest.main(ThreadStateTest.java:96)
>
> There is a race between the thread being put in a specific state and 
> the thread testing for that state. The test should retry the thread 
> state check a number of times before failing. Also, some minor cleanup 
> and update to use a more recent j.u.c reusable synchronization barrier.
>
> http://cr.openjdk.java.net/~chegar/7021010/jdk8.webrev.00/webrev/

L130-116: I agree with David that this test is not needed.
Like the RuntimeException listed in ProblemList.txt shows
that the target thread is in WAITING state but expected to
be RUNNABLE.

L98: Are you seeing some timing issue if you keep this checkThreadState
      before the join and thus you moved it after the join?

As to David's comment about the use of Phaser that delays the thread
from making the state change.  The test was modified to use
ThreadExecutionSynchronizer  as a fix to:
    5039275 ThreadBlockedCount jtreg test fails on Solaris sparc on service_sdk nightly

ThreadExecutionSynchronizer.signal() actually awaits until the
main thread calls waitForSignal.  If I understand correctly, it's
essentially doing the same thing as Phaser.arriveAndAwaitAdvance().
So Chris' change to use Phaser only follows the existing code.

I thinkThreadExecutionSynchronizer was initially created to fix
ThreadBlockCount testthat can make sure the thread enters the BLOCKED
state an exact number of times and applied to other tests as it
yields tighter timing window but might not be necessary.

Anyway, like David said, it isn't incorrect. If you don't have
time to eliminate the need of the main thread to loop, I'm fine
with the change to use Phaser but worths tuning the sleep time in L117.

Thanks
Mandy