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

[serguei.spitsyn at oracle.com]

Hi David,

Thank you a lot for re-review!
Serguei

On 10/8/19 20:29, David Holmes wrote:
> 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
>>>
>>