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

[Chris Plummer]

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