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



Reply via email to