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

[Gary Adams]

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

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 6545967.patch
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180620/1e698609/6545967-0001.patch>