RFR (M): 8231595 [TEST] develop a test case for SuspendThreadList including current thread

[David Holmes]

Hi Serguei,

On 9/10/2019 4:50 am, serguei.spitsyn at oracle.com wrote:
> Ping...

Sorry this one slipped through the cracks.

The updates seem fine to me. Using the lock as a gate works fine in 
these kinds of situations.

Thanks,
David

> Thanks,
> Serguei
> 
> On 10/7/19 5:25 PM, serguei.spitsyn at oracle.com wrote:
>> On 10/7/19 17:14, serguei.spitsyn at oracle.com wrote:
>>> Hi Alex, Chris and David,
>>>
>>> The mach5 testing in 100 runs loop on all platform discovered a race 
>>> in new test.
>>> It is between the native suspendTestedThreads() called on the 
>>> suspender thread
>>> and the checkSuspendedStatus() calling native 
>>> checkTestedThreadsSuspended().
>>>
>>> It occurred that the iteration count and sleep time in 
>>> checkSuspendedStatus() can be not enough:
>>>   100     private boolean checkSuspendedStatus(ThreadToSuspend[] threads) throws RuntimeException  {
>>>   101         boolean success = false;
>>>   102
>>>   103         log("Main: checking all tested threads have been suspended");
>>>   104
>>>   105         // wait for tested threads to reach suspended status
>>>   106         for (int i = 0; !success && i < 20; i++) {
>>>   107             try {
>>>   108                 Thread.sleep(10);
>>>   109             } catch (InterruptedException e) {
>>>   110               throw new RuntimeException("Main: sleep was interrupted\n\t" + e);
>>>   111             }
>>>   112             success = checkTestedThreadsSuspended();
>>>   113         }
>>>   114         return success;
>>>   115     }
>>>
>>> So, I've decided to refactor/update the test a little bit:
>>>
>>> 1. Now the checkSuspendedStatus() is:
>>>   100     private boolean checkSuspendedStatus() throws RuntimeException  {
>>>   101         log("Main: checking all tested threads have been suspended");
>>>   102         return checkTestedThreadsSuspended();
>>>   103     }
>>> 2. A raw monitor agent_monitor is added to the native agent.
>>>    It is created and locked in the Java_ThreadToSuspend_init() 
>>> function called at
>>>    the beginning of the ThreadToSuspend.run() execution of the 
>>> suspender thread
>>>    and unlocked at the end of 
>>> Java_ThreadToSuspend_suspendTestedThreads().
>>>
>>>    The Java_SuspendWithCurrentThread_checkTestedThreadsSuspended() 
>>> grabs the agent_monitor
>>>    on the Main thread to wait until the suspendTestedThreads() 
>>> completes its work.
>>>
>>> 3. Also, the results[] array is always created and used locally in 
>>> the functions
>>>    where it is needed: suspendTestedThreads() and resumeTestedThreads().
>>
>> Forgot to tell about one more change:
>>
>> 4. The releaseTestedThreads() is renamed to releaseTestedThreadsInfo() 
>> and moved
>>    to the end of the Main thread run() method.
>>
>> Also wanted to thank Alex for sharing good suggestions on the sync 
>> approaches.
>>
>> Thanks,
>> Serguei
>>
>>>
>>> Updated webrev version:
>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8231595-jvmti-susp-tlist.4/
>>>
>>>
>>> Testing:
>>>   One mach5 run with 100 rep counter successfully passed.
>>>   To be more confident, I've submitted one more mach5 job which is in 
>>> progress.
>>>
>>> Thanks,
>>> Serguei
>>
>