RFR JDK-8152589 : java/lang/management/ThreadMXBean/Locks.java fails intermittently, blocked on wrong object

[Harsha Wardhana B]

Hello,

Please review the below webrev incorporating David's comments.


http://cr.openjdk.java.net/~hb/8152589/webrev.01/

Regards

Harsha


On Wednesday 17 August 2016 11:45 AM, Harsha Wardhana B wrote:
> Hi David,
>
> I will incorporate changes suggested by you. Let's wait for few more 
> review comments and then I will send consolidated webrev.
>
> Regards
> Harsha
>
> On Wednesday 17 August 2016 09:02 AM, David Holmes wrote:
>> On 16/08/2016 11:33 PM, Harsha Wardhana B wrote:
>>> Hi David,
>>>
>>> Agreed that we could fix WaitingThread the way you have said, but in
>>> recent past, there aren't any issues reported w.r.t WaitingThread.
>>
>> Nor are there likely to be - that's what makes spurious wakeup bugs 
>> so difficult to detect!
>>
>>> This test has been fixed several times (3-4) for intermittent failures
>>> and hence I would not like to meddle with code that is not causing any
>>> problems even though there is scope for refactoring.
>>
>> It isn't refactoring it is fixing and we have fixed several tests in 
>> a similar way.
>>
>>> The issue reported was with LockThreadB and hence I have provided
>>> possible fix for the same.
>>
>> That doesn't preclude fixing other issues with the test at the same 
>> time.
>>
>> David
>>
>>> Thanks
>>>
>>> Harsha
>>>
>>>
>>> On Tuesday 16 August 2016 01:32 PM, David Holmes wrote:
>>>> Hi Harsha,
>>>>
>>>> On 16/08/2016 4:08 PM, Harsha Wardhana B wrote:
>>>>> Hello,
>>>>>
>>>>> Please review and provide comments for fix for issue,
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8152589
>>>>>
>>>>> having webrev located at
>>>>>
>>>>> http://cr.openjdk.java.net/~hb/8152589/webrev.00/
>>>>
>>>> These changes look quite good (though I have to admit I struggle to
>>>> read the lambda and stream code :) ).
>>>>
>>>> Note that like many of these kinds of tests there is an issue with
>>>> WaitingThread because it does not wait in a loop and so is susceptible
>>>> to spurious wakeups. The way to fix that is to add a "notified"
>>>> variable and then do:
>>>>
>>>> while (!notified)
>>>>   wait();
>>>>
>>>> and set notified before the notify().
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> Fix details:
>>>>>
>>>>> 1. From nightly failures we see that LockThreadB was blocked on wrong
>>>>> object. We now do a repeated check with timeout if any given 
>>>>> thread is
>>>>> blocked on expected object. It is possible that LockThreadB might 
>>>>> still
>>>>> be in Phaser call stack (Unsafe.park) when 'checkBlockedObject' is
>>>>> invoked.
>>>>>
>>>>> 2. The logs from lock free logger was never printed. It is not being
>>>>> printed.
>>>>>
>>>>> 3. Any time we see failure, thread stack is being logged. This 
>>>>> helps us
>>>>> ascertain if failure is in test case or in the component.
>>>>>
>>>>> 4. Even though we had lock free logger, several 
>>>>> ex.printStackTrace() was
>>>>> used which could be responsible for failure. It is removed.
>>>>>
>>>>> 5. There were several places where tests continue to ran even after
>>>>> failure (testFailed flag). That is fixed.
>>>>>
>>>>> 6. Couple of other minor refactoring.
>>>>>
>>>>> Thanks
>>>>>
>>>>> Harsha
>>>>>
>>>
>