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.
