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

[Harsha Wardhana B]

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