[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR (M): 8231595 [TEST] develop a test case for SuspendThreadList including current thread
From: "serguei.spitsyn () oracle ! com" <serguei ! spitsyn () oracle ! com>
Date: 2019-10-09 17:53:12
Message-ID: 3baa2946-72f5-6918-8c2f-d5520f0c2e74 () oracle ! com
[Download RAW message or body]
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Thanks, Chris!<br>
Serguei<br>
<br>
<br>
On 10/9/19 10:14, Chris Plummer wrote:<br>
</div>
<blockquote type="cite"
cite="mid:a9ae9ab3-d913-a7c0-31eb-35569ba99f74@oracle.com">
<div class="moz-cite-prefix">Hi Serguei,<br>
<br>
It's ok to push.<br>
<br>
thanks,<br>
<br>
Chris<br>
<br>
On 10/8/19 10:47 PM, <a class="moz-txt-link-abbreviated"
href="mailto:serguei.spitsyn@oracle.com"
moz-do-not-send="true">serguei.spitsyn@oracle.com</a> wrote:<br>
</div>
<blockquote type="cite"
cite="mid:78f75616-d0da-21bd-7188-d5f1e6169eaf@oracle.com">
<div class="moz-cite-prefix">Hi Chris,<br>
<br>
Please, let me know if you have more thoughts on this or it is
okay to push as it is.<br>
And thank you a lot for reviewing this!<br>
<br>
Thanks,<br>
Serguei <br>
<br>
<br>
On 10/8/19 13:20, <a class="moz-txt-link-abbreviated"
href="mailto:serguei.spitsyn@oracle.com"
moz-do-not-send="true">serguei.spitsyn@oracle.com</a> wrote:<br>
</div>
<blockquote type="cite"
cite="mid:dd97ce26-9853-07f8-d3b5-ea3d26d83a48@oracle.com"> Hi
Chris,<br>
<br>
<div class="moz-cite-prefix">On 10/8/19 12:56 PM, Chris
Plummer wrote:<br>
</div>
<blockquote type="cite"
cite="mid:6d484361-9e5b-3d05-8a23-bcf391992839@oracle.com">
<div class="moz-cite-prefix">Hi Serguei,<br>
<br>
The just seems like an odd coding pattern to me:<br>
<br>
// Block until the suspender thread competes the tested
threads suspension<br>
agent_lock(jni);<br>
agent_unlock(jni);<br>
</div>
</blockquote>
<br>
I do not consider it as odd but normal.<br>
We have similar coding patterns in the jdwp agent.<br>
The agent_unlock(jni) call can be moved to the end of the
function if you like.<br>
It does not matter in this particular case but will look
"better".<br>
<br>
<blockquote type="cite"
cite="mid:6d484361-9e5b-3d05-8a23-bcf391992839@oracle.com">
<div class="moz-cite-prefix"> Wouldn't you normally use
wait/notify in this situation? Some like:<br>
<br>
agent_lock(jni);<br>
if (!suspenderDone) {<br>
agent_wait(jni);<br>
}<br>
agent_unlock(jni);<br>
</div>
</blockquote>
<br>
The above will make synchronization more complex.
<blockquote type="cite"
cite="mid:6d484361-9e5b-3d05-8a23-bcf391992839@oracle.com">
<div class="moz-cite-prefix"> <br>
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?).<br>
</div>
</blockquote>
Yes, it is the case.<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
<blockquote type="cite"
cite="mid:6d484361-9e5b-3d05-8a23-bcf391992839@oracle.com">
<div class="moz-cite-prefix"> thanks,<br>
<br>
Chris<br>
<br>
On 10/8/19 11:50 AM, <a class="moz-txt-link-abbreviated"
href="mailto:serguei.spitsyn@oracle.com"
moz-do-not-send="true">serguei.spitsyn@oracle.com</a>
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:bbbd93a0-fcdf-b840-1297-5818139d2725@oracle.com">
Ping...<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<div class="moz-cite-prefix">On 10/7/19 5:25 PM, <a
class="moz-txt-link-abbreviated"
href="mailto:serguei.spitsyn@oracle.com"
moz-do-not-send="true">serguei.spitsyn@oracle.com</a>
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:4ca3e8f8-0b6d-c94b-44d8-45661ed24f49@oracle.com">
<div class="moz-cite-prefix">On 10/7/19 17:14, <a
class="moz-txt-link-abbreviated"
href="mailto:serguei.spitsyn@oracle.com"
moz-do-not-send="true">serguei.spitsyn@oracle.com</a>
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:01e82088-3ea0-8af8-840e-c4c9a6bc52c1@oracle.com">
Hi Alex, Chris and David,<br>
<br>
The mach5 testing in 100 runs loop on all platform
discovered a race in new test.<br>
It is between the native suspendTestedThreads() called
on the suspender thread<br>
and the checkSuspendedStatus() calling native
checkTestedThreadsSuspended().<br>
<br>
It occurred that the iteration count and sleep time in
checkSuspendedStatus() can be not enough:<br>
<pre> 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 }</pre>
<br>
So, I've decided to refactor/update the test a little
bit: <br>
<br>
1. Now the checkSuspendedStatus() is:<br>
<pre> 100 private boolean checkSuspendedStatus() throws \
RuntimeException { 101 log("Main: checking all tested threads have been \
suspended"); 102 return checkTestedThreadsSuspended();
103 }</pre>
2. A raw monitor agent_monitor is added to the native
agent.<br>
It is created and locked in the
Java_ThreadToSuspend_init() function called at<br>
the beginning of the ThreadToSuspend.run()
execution of the suspender thread<br>
and unlocked at the end of
Java_ThreadToSuspend_suspendTestedThreads().<br>
<br>
The
Java_SuspendWithCurrentThread_checkTestedThreadsSuspended()
grabs the agent_monitor<br>
on the Main thread to wait until the
suspendTestedThreads() completes its work.<br>
<br>
3. Also, the results[] array is always created and
used locally in the functions<br>
where it is needed: suspendTestedThreads() and
resumeTestedThreads().<br>
</blockquote>
<br>
Forgot to tell about one more change:<br>
<br>
4. The releaseTestedThreads() is renamed to
releaseTestedThreadsInfo() and moved<br>
to the end of the Main thread run() method.<br>
<br>
Also wanted to thank Alex for sharing good suggestions
on the sync approaches. <br>
<br>
Thanks,<br>
Serguei<br>
<br>
<blockquote type="cite"
cite="mid:01e82088-3ea0-8af8-840e-c4c9a6bc52c1@oracle.com">
<br>
Updated webrev version:<br>
<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2019/8231595-jvmti-susp-tlist.4/"
moz-do-not-send="true">http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8231595-jvmti-susp-tlist.4/</a><br>
<br>
<br>
Testing:<br>
One mach5 run with 100 rep counter successfully
passed.<br>
To be more confident, I've submitted one more mach5
job which is in progress. <br>
<br>
Thanks,<br>
Serguei<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic