[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-jmx-dev
Subject:    Re: jmx-dev RFR: 6543856: MonitorVmStartTerminate.sh fails intermittently
From:       David Holmes <david.holmes () oracle ! com>
Date:       2013-11-06 0:52:37
Message-ID: 527992D5.50408 () oracle ! com
[Download RAW message or body]

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.gahlin@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
> 


[prev in list] [next in list] [prev in thread] [next in thread]