Mandy Chung said the following on 06/21/11 20:08:
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.
Thanks Mandy I missed that detail.
David
-----
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
