Hi Erik,
From what I can see this test will no longer try to check that the
number of sleepers started and terminated are valid - it will either
pass or hang (because the latches do not release) - relying on the test
harness to time it out (ditto for the removal of the timeout from the
ss.join()). It seems to me that you could have simplified the changes
(and kept the existing failure modes) if you had simply added a
semaphore.acquire in main after ss.join and before listener.getStarted;
with the semaphore release at the end of vmStatusChanged. But what you
have isn't wrong.
Aside: The CountDownLatch usage is a little odd as the typical pattern
is that different threads call countDown. The same semantics could have
been achieved with a Semaphore initialized to zero and using
acquire(count) and release(recentlyTerminated) etc.
Two minor nits:
- for(Integer lvmid : list) - need space after for (line 92 and 207)
- waitForSleeperToStart() should be waitForSleepersToStart (plural to
match termination function).
Otherwise good to go (unless you choose to revisit the approach taken :) ).
Thanks,
David
On 6/11/2013 4:23 AM, Staffan Larsen wrote:
Looks good!
Thanks,
/Staffan
On 5 nov 2013, at 11:25, Erik Gahlin <erik.gah...@oracle.com> wrote:
Could I please have a review of this intermittently failing test.
Removed Thread.sleep and instead used two count down latches. Also did some
cleaning up; removed an unused import, added generics etc.
Thanks
Erik
Webrev:
http://cr.openjdk.java.net/~egahlin/6543856_1/
Bug:
https://bugs.openjdk.java.net/browse/JDK-6543856
