On 23/06/2015 5:33 PM, Jaroslav Bachorik wrote:
Hi David,

On 23.6.2015 08:04, David Holmes wrote:
On 23/06/2015 1:36 AM, Jaroslav Bachorik wrote:
Please, review the following test change

Issue : https://bugs.openjdk.java.net/browse/JDK-8071487
Webrev: http://cr.openjdk.java.net/~jbachorik/8071487/webrev.00

GaugeMonitorDeadlockTest fails intermittently due data race - the call
to monitorProxy.start() on L105 will eventually result in incrementing
the GetCount attribute. If that happens before L109 had the chance to
run the loop on L113-128 will become infinite. The initial value will
contain the already incremented GetCount value and GetCount attribute
value will not get further incremented.

The reordering of the start() and initial observedProxy.getGetCount
seems reasonable.

The changes to the timeout handling less so. The alarm code doesn't look
right to me. Each time you call check(true) in the loop you advance the
time when the alarm "goes off". ???

Stupid me :( Thanks for catching this.

http://cr.openjdk.java.net/~jbachorik/8071487/webrev.01

That looks better, but the Alarm class would benefit from some simple descriptive comments. I must admit I don't really see the need for introducing it.

Thanks,
David

-JB-


David



I took the liberty to fix the same issue in
test/javax/management/monitor/StringMonitorDeadlockTest.java, not
waiting for the real test failure.

Thanks,

-JB-

Reply via email to