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

[serguei.spitsyn at oracle.com]

Hi Chris,

On 10/8/19 12:56 PM, Chris Plummer wrote:
> Hi Serguei,
>
> The just seems like an odd coding pattern to me:
>
>   // Block until the suspender thread competes the tested threads 
> suspension
>   agent_lock(jni);
>   agent_unlock(jni);

I do not consider it as odd but normal.
We have similar coding patterns in the jdwp agent.
The agent_unlock(jni) call can be moved to the end of the function if 
you like.
It does not matter in this particular case but will look "better".

> Wouldn't you normally use wait/notify in this situation? Some like:
>
>   agent_lock(jni);
>   if (!suspenderDone) {
>     agent_wait(jni);
>   }
>   agent_unlock(jni);

The above will make synchronization more complex.
>
> I think maybe you left out the middle part because you know that this 
> code will never grab the agent_lock before the suspender thread does, 
> and therefore once this code gets the lock you know the suspender 
> thread is done (is that actually the case?).
Yes, it is the case.

Thanks,
Serguei


> thanks,
>
> Chris
>
> On 10/8/19 11:50 AM, serguei.spitsyn at oracle.com wrote:
>> Ping...
>>
>> 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
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20191008/6b9ad941/attachment.html>