On 19/02/2014 7:01 PM, Staffan Larsen wrote:
Thanks for the feedback!
I chose to use yet another variable to avoid the spurious wakeups. I’ve
also increased the range of the synchronized statement to avoid the race.
http://cr.openjdk.java.net/~sla/6952105/webrev.01/
Slightly simpler to just do:
bkptSignal.wait(5000);
if (!signalSent)
continue;
but what you have works.
Also signalSent doesn't need to be volatile as it is only accessed
within the sync blocks.
Thanks,
David
Thanks,
/Staffan
On 19 feb 2014, at 07:09, David Holmes <david.hol...@oracle.com> wrote:
On 19/02/2014 7:17 AM, shanliang wrote:
I am looking at the old file:
143 while (bkptCount < maxBkpts) {
144 prevBkptCount = bkptCount;
suppose the following execution sequence:
1) when Line 143 was called by Thread1, we had bkptCount ==
maxBkpts - 1;
2) bkptCount++ was executed by thread2;
3) Line 144 was called by thread1,
in this case it was sure that the line
152 failure("failure: test hung");
would be called.
Yes I was looking at that race too. The comments suggest that we
should never reach a point where we get to maxBkpts, so this failure
would be very rare and would likely indicate a real problem.
It is good to add:
synchronized (bkptSignal)
in the fix, but we need to put Line 143 and 144 into synchronization too.
To deal with a spurious wakeup, we might do like this:
long stopTime = System.currentTimeMillis() + 5000;
do {
try {
bkptSignal.wait(100);
} catch (InterruptedException e){}
} while(prevBkptCount == bkptCount && System.currentTimeMillis()
< stopTime);
It is better to use System.nanoTime() rather than the non-monotonic
currentTimeMillis(). And you really want a while loop rather than
do-while so we don't always do that 100ms wait.
David
Shanliang
David Holmes wrote:
On 18/02/2014 11:03 PM, Staffan Larsen wrote:
On 18 feb 2014, at 13:09, David Holmes <david.hol...@oracle.com> wrote:
Hi Staffan,
If you get a spurious wakeup from wait():
151 try {
152 synchronized (bkptSignal) {
153 bkptSignal.wait(5000);
154 }
155 } catch (InterruptedException ee) {
156 }
157 if (prevBkptCount == bkptCount) {
158 failure("failure: test hung");
you could report failure. But that is far less likely than the
current problem using sleep.
Right. Adding “continue;” inside the catch(InterruptedException)
block should guard against that.
No, a spurious wakeup is not an interrupt - the wait() will simply
return.
David
/Staffan
David
On 18/02/2014 8:19 PM, Staffan Larsen wrote:
Still looking for Reviewer for this change.
Thanks,
/Staffan
On 11 feb 2014, at 15:12, Staffan Larsen
<staffan.lar...@oracle.com> wrote:
Updated the test to use proper synchronization and notification
between threads. Should be more stable and much faster.
bug: https://bugs.openjdk.java.net/browse/JDK-6952105
webrev: http://cr.openjdk.java.net/~sla/6952105/webrev.00/
Thanks,
/Staffan
