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

[David Holmes]

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.