RFR: JDK-6545967: sp05t003 failed ResumeThread() due to THREAD_NOT_SUSPENDED

[Chris Plummer]

I got it.

Chris

On 6/20/18 11:03 AM, Gary Adams wrote:
> I'll need a sponsor for the push.
> Patch attached.
>
> On 6/20/18, 1:48 PM, serguei.spitsyn at oracle.com wrote:
>> Hi Gary,
>>
>> Looks great modulo the suggestion from Chris.
>>
>> Thanks,
>> Serguei
>>
>>
>> On 6/20/18 10:45, Chris Plummer wrote:
>>> Hi Gary,
>>>
>>> Overall looks good. One minor formatting comment:
>>>
>>> 105 if ((mt.threadState < 1000) &&
>>> 106 agentStatus()) {
>>>  107             passed = true;
>>>  108         }
>>>
>>>
>>> Lines 105 and 106 should be combined.
>>>
>>> thanks
>>>
>>> Chris
>>>
>>> On 6/20/18 5:31 AM, Gary Adams wrote:
>>>> We do need to consider an early return for the case where
>>>> the redefine class or suspend thread operation have detected
>>>> an error.
>>>>
>>>>
>>>> Webrev:http://bussund0416.us.oracle.com/export/users/gradams/work/webrevs/6545967/webrev.01/ 
>>>>
>>>>
>>>>
>>>>    80     public boolean agentMethod() {
>>>>    81         boolean passed = false;
>>>>    82         MyThread mt = new MyThread();
>>>>    83         try {
>>>>    84             mt.start();
>>>>    85             // check if we can can pop the thread.
>>>> 86 // we can not do redefine/pop frame on run method.
>>>> 87 while (!MyThread.resume.get());
>>>>
>>>>    88             // sleep for some few secs to get redefined.
>>>> 89 while (!isRedefined()) {
>>>> 90 if (!agentStatus()) {
>>>> 91 System.out.println("Failed to redefine class");
>>>> 92 return passed;
>>>> 93 }
>>>> 94 Thread.sleep(100);
>>>> 95 }
>>>> 96 popThreadFrame(mt); // pop the frame.
>>>> 97 resumeThread(mt); // resume the thread.
>>>>    98             mt.join();
>>>>    99             // wait till the other thread completes its 
>>>> execution.
>>>> 100 System.out.println("Thread state after popping/redefining = "
>>>> 101 + mt.threadState);
>>>>   102         } catch(Exception ie) {
>>>>   103             ie.printStackTrace();
>>>>   104         }
>>>> 105 if ((mt.threadState < 1000) &&
>>>> 106 agentStatus()) {
>>>>
>>>>   107             passed = true;
>>>>   108         }
>>>>   109         return passed;
>>>>   110     }
>>>>
>>>>
>>>>
>>>> On 6/19/18, 7:53 PM, Chris Plummer wrote:
>>>>> Hi Gary,
>>>>>
>>>>> The sp05t003 changes look fine. Serquei's question about hs203t003 
>>>>> leads me to ask what happens if the redefine never happens, 
>>>>> possibly due to some error. Seems we are destined for a timeout 
>>>>> then (not sure how long that will take), whereas the current 
>>>>> implementation would report the failure after about 10 seconds.
>>>>>
>>>>> thanks,
>>>>>
>>>>> Chris
>>>>>
>>>>> On 6/19/18 2:26 PM, serguei.spitsyn at oracle.com wrote:
>>>>>> Hi Gary,
>>>>>>
>>>>>> The fix looks good in general.
>>>>>>
>>>>>> One comment though:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~gadams/6545967/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS203/hs203t003/hs203t003.java.frames.html 
>>>>>>
>>>>>>
>>>>>>    80     public boolean  agentMethod() {
>>>>>>    81         boolean passed = false;
>>>>>>    82         MyThread mt = new MyThread();
>>>>>>    83                 try {
>>>>>>    84             mt.start();
>>>>>>    85             // check if we can can pop the thread.
>>>>>>    86             // we can not do redefine / pop frame on run 
>>>>>> method.
>>>>>>    87             while(!MyThread.resume.get());
>>>>>>    88             // sleep for some few secs to get redefined.
>>>>>> 89 while(!isRedefined()) {
>>>>>> 90 Thread.sleep(100);
>>>>>> 91 }
>>>>>> 92 popThreadFrame(mt); // pop the frame.
>>>>>> 93 resumeThread(mt); // resume the thread.
>>>>>>    94             mt.join();
>>>>>>    95             // wait till the other thread completes its 
>>>>>> execution.
>>>>>>    96             System.out.println(" Thread state after poping 
>>>>>> / redefining = "+mt.threadState);
>>>>>>    97         } catch(Exception ie) {
>>>>>>    98             ie.printStackTrace();
>>>>>>    99         }
>>>>>>   100         if ( ( mt.threadState < 1000 ) &&
>>>>>>   101                 ( redefineAttempted() && isRedefined()) &&
>>>>>>   102                 agentStatus() ) {
>>>>>>   103             passed = true;
>>>>>>   104         }
>>>>>>   105         return passed;
>>>>>>   106     }
>>>>>>   It seems, the two checks ( redefineAttempted() && 
>>>>>> isRedefined()) at L101 are not needed any more.
>>>>>> Now, there is no way out of the while loop at the line 89.
>>>>>>
>>>>>> Thanks,
>>>>>> Serguei
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 6/19/18 11:42, Gary Adams wrote:
>>>>>>> There are some rare race conditions that impact some jvmti tests 
>>>>>>> that
>>>>>>> suspend and resume threads. These tests were recently moved into
>>>>>>> the open repos.
>>>>>>>
>>>>>>>   Webrev: http://cr.openjdk.java.net/~gadams/6545967/webrev.00/
>>>>>>>
>>>>>>> The fix in hs203t003 replaces a blind 10 second sleep, with a 
>>>>>>> specific
>>>>>>> check to wait for a redefineClass to be performed, which takes 
>>>>>>> place as
>>>>>>> part of callbackFieldAccess. This let's the rest of the 
>>>>>>> operations be performed
>>>>>>> at a known point in the test sequence.
>>>>>>>
>>>>>>> The fix in sp05t003 moves the incrementing of the counters to 
>>>>>>> after the
>>>>>>> suspend thread calls. The agentProc function is watching the 
>>>>>>> counters to know
>>>>>>> when to continue processing and eventually resuming of the 
>>>>>>> suspended threads.
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>
>