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

[David Holmes]

On 20/11/2013 12:43 AM, Jaroslav Bachorik wrote:
> Hi David,
>
> On 18.11.2013 22:03, David Holmes wrote:
>> 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.
>
> Yes, thanks for pointing this out. But, wouldn't
> p.arriveAndAwaitAdvance() instead of p.arrive() suffice? I've tried to
> replace Phaser with CyclicBarrier but while it seems to work as expected
> it pollutes code with the necessity to catch various exceptions
> (InterruptedException, BarrierException etc.). So, if the simple fix
> with Phaser would do the trick I would better use that.

In this case yes arriveAndAwaitAdvance would fix it. I don't know if 
other tests have the same issue - the concern would be ensuring there's 
no possibility of deadlocks in general.

Thanks,
David

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