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.

Reply via email to