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

[David Holmes]

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
>>>>>
>>>
>