RFR 6712222: 6712222: Race condition in java/lang/management/ThreadMXBean/AllThreadIds.java

[Jaroslav Bachorik]

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