RFR(S): 6952105 TEST_BUG: testcase failure, not very often, com/sun/jdi/SuspendThreadTest.java (original) (raw)

Staffan Larsen staffan.larsen at oracle.com
Wed Feb 19 01:01:59 PST 2014

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/

Thanks, /Staffan

On 19 feb 2014, at 07:09, David Holmes <david.holmes at 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.holmes at 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.larsen at 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

-------------- next part -------------- An HTML attachment was scrubbed... URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20140219/352fb18b/attachment-0001.html

More information about the serviceability-dev mailing list