On 6.3.2015 17:22, Daniel Fuchs wrote:
Hi Jaroslav,
Looks reasonable to me. Not sure why you replaced Barrier with
Semaphore but I don't really mind.
Well, this goes in line with the recent fixes where it was recommended
to me to switch from using the test-only concurrent primitives to the
standard java.util.concurrent counterparts.
I tried to replace the Barrier with CyclicBarrier but the usage didn't
map well. The easiest to migrate and the easiest to understand was
Semaphore.
-JB-
best regards,
-- daniel
On 06/03/15 17:08, Jaroslav Bachorik wrote:
Please, review the following test change
Issue : https://bugs.openjdk.java.net/browse/JDK-6712222
Webrev: http://cr.openjdk.java.net/~jbachorik/6712222/webrev.00
As the issue subject states this test is susceptible to race condition.
Firstly, it uses an arbitrary time period to wait after a thread has
signalled its exit vie the associated barrier to 'make sure' the thread
is actually terminated when the check for the number of live threads is
performed. This is quite race prone and should be replaces by
Thread.join() performed on the specified threads before moving forward
and checking the reported thread counts.
Secondly, it is using a volatile array of boolean flags which are used
for communication between the main thread and the test threads (setting
an item of this array true will indicate the appropriate test thread to
exit). Declaring the array as volatile does nothing for the thread
safety and reads and writes to this array must be properly synchronized.
I also took the liberty of replacing the arbitrary Barrier
synchronization class with the standard Semaphore.
Thanks,
-JB-
