RFR 6309226: TEST: java/lang/management/ThreadMXBean/SynchronizationStatistics.java didn't check Thread.sleep

[David Holmes]

Hi Jaroslav,

I think your phaser usage is incorrect:

  88             public void run() {
   89                 p.arriveAndAwaitAdvance(); // phase[1]
   90                 synchronized(lock1) {
   91                     System.out.println("[LockerThread obtained 
Lock1]");
   92                     p.arrive(); // phase[2]
   93                 }
   94                 p.arriveAndAwaitAdvance(); // phase[3]
   95             }

Here the current thread can release itself at line 94. You have assumed 
that the phase[2] arrive will be a trigger to release the main thread 
but it may not have reached its arriveAndAwaitAdvance phase[2] statement 
yet, so the current thread arrives then does arrive-and-wait but the 
number of arrivals is 2 so it doesn't wait.

A CyclicBarrier per phase may be clearer.

David

On 18/11/2013 7:59 PM, Jaroslav Bachorik wrote:
> Hi,
>
> after discussing this with Mandy I've rewritten the test to use the
> j.u.concurrent for synchronization - this also makes it much easier to
> follow the test logic.
>
> The waited time, the blocked time and the waited counts are only checked
> for sanity (increasing values) since it is not possible to do the
> reliable checks against hard numbers.
>
> I ran the test in a tight loop for 1500 times using -Xcomp and -Xint and
> the test seems to pass constantly.
>
> New webrev: http://cr.openjdk.java.net/~jbachorik/6309226/webrev.03
>
> Thanks,
>
> -JB-
>
>
> On 21.10.2013 13:55, Jaroslav Bachorik wrote:
>> Please, review this small patch for a test failing due to the updated
>> implementation in JDK6.
>>
>> Issue:  https://bugs.openjdk.java.net/browse/JDK-6309226
>> Webrev: http://cr.openjdk.java.net/~jbachorik/6309226/webrev.00/
>>
>> The test fails due to the change in mustang where
>> ThreadMXBean.getThreadInfo().getWaitedTime() and
>> ThreadMXBean.getThreadInfo().getWaitedCount() include Thread.sleep()
>> too. Unfortunately, Thread.sleep() is used throughout the test for
>> synchronization purposes and this breaks the test.
>>
>> In the patch I propose to replace Thread.sleep() with busy wait and
>> hinting the scheduler by Thread.yield(). While not very elegant it
>> successfully works around inclusion of unknown number of Thread.sleep()s
>> (they are called in loop).
>>
>> Thanks,
>>
>> -JB-
>