Code Review 7021010: java/lang/Thread/ThreadStateTest.java fails intermittently (original) (raw)

David Holmes David.Holmes at oracle.com
Tue Jun 21 10:49:22 UTC 2011

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 servicesdk 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

More information about the core-libs-dev mailing list