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

[David Holmes]

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.