RFR: JDK-8168141: javax/management/remote/mandatory/notif/EmptyDomainNotificationTest.java: No notif received!

[Ujwal Vangapally]

Thanks for the Review, please find the new webrev incorporating the 
review comments.

webrev : 
http://cr.openjdk.java.net/~asapre/sponsorships/Ujwal/JDK-8168141/webrev.02/

-Ujwal.

On 11/8/2016 5:18 PM, David Holmes wrote:
> Sorry didn't see Staffan's earlier reply :)
>
> David
>
> On 8/11/2016 9:23 PM, David Holmes wrote:
>> On 8/11/2016 8:44 PM, Robbin Ehn wrote:
>>> Hi Ujwal,
>>>
>>> synchronized(li) {
>>>     while (li.received < 1) {
>>>         li.wait(100);
>>>     }
>>> }
>>>
>>> I don't see why we need while loop ?
>>
>> You should always perform a wait() in a loop checking the condition that
>> is being waited upon. This guards against lost-notifications and also
>> spurious wakeups.
>>
>>> To me it looks like you could just do:
>>>
>>> synchronized(li) {
>>>     li.wait();
>>> }
>>>
>>> Since either we got notification and received must be bigger than 0.
>>> Or jtreg timed out.
>>
>> If the notifyAll() happened before you get here then you will wait()
>> until jtreg does time you out - even though the notification correctly
>> occurred.
>>
>> That said, in this particular case doing a timed-wait achieves nothing
>> other than waking the thread so that it can go back to waiting again.
>> The "received" value will only change when a notifyAll occurs so there
>> is no need to poll it (unless aborting due to a timeout as per the
>> previous version).
>>
>> Because the loop will never exit, unless the thread is interrupted, this
>> subsequent code has no affect:
>>
>> 112         if (li.received < 1) {
>> 113             throw new RuntimeException("No notif received!");
>>
>> David
>> -----
>>
>>> /Robbin ('r'eviewer)
>>>
>>> On 11/04/2016 12:03 PM, Ujwal Vangapally wrote:
>>>> Please review this small change for the bug below
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8168141
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~asapre/sponsorships/Ujwal/JDK-8168141/webrev.01/ 
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> Thanks,
>>>> Ujwal.