<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<div class="moz-cite-prefix">Hi Yasumasa,<br>
<br>
It looks pretty good in general.<br>
A couple of comments though.<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~ysuenaga/JDK-8242425/webrev.00/src/hotspot/share/prims/jvmtiEnvBase.cpp.frames.html">http://cr.openjdk.java.net/~ysuenaga/JDK-8242425/webrev.00/src/hotspot/share/prims/jvmtiEnvBase.cpp.frames.html</a><br>
<pre><span class="changed"> 650 JvmtiEnvBase::get_current_contended_monitor(JavaThread *java_thread, jobject *monitor_ptr) {</span>
<span class="changed"> 651 assert(!Thread::current()->is_VM_thread() &&</span>
<span class="changed"> 652 (Thread::current() == java_thread ||</span>
<span class="changed"> 653 Thread::current() == java_thread->active_handshaker()),</span>
<span class="changed"> 654 "call by myself or at direct handshake");</span>
...
</pre>
<pre><span class="changed"> 685 JvmtiEnvBase::get_owned_monitors(JavaThread* java_thread,</span>
686 GrowableArray<jvmtiMonitorStackDepthInfo*> *owned_monitors_list) {
687 jvmtiError err = JVMTI_ERROR_NONE;
<span class="changed"> 688 assert(!Thread::current()->is_VM_thread() &&</span>
<span class="changed"> 689 (Thread::current() == java_thread ||</span>
<span class="changed"> 690 Thread::current() == java_thread->active_handshaker()),</span>
<span class="changed"> 691 "call by myself or at direct handshake");</span>
</pre>
I'd suggest to add this at the beginning:<span class="changed"><br>
 JavaThread *current_jt = JavaThread::current();</span><br>
<br>
<br>
<pre><span class="changed"> 676 JavaThread *current_jt = reinterpret_cast<JavaThread *>(JavaThread::current());
</span></pre>
There is not need in <span class="changed">reinterpret_cast<JavaThread
*>. Also, you can use </span><span class="changed">the
current_jt</span>
defined above.<br>
<br>
Also, please, removed these two definitions as they became unused
with your fix:<br>
  src/hotspot/share/runtime/vmOperations.hpp:Â
template(GGetCurrentContendedMonitoretOwnedMonitorInfo)Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â
\<br>
  src/hotspot/share/runtime/vmOperations.hpp:Â
template()Â Â Â Â Â Â Â Â Â Â Â \<br>
<br>
The impacted JVMTI monitor functions are used in the JDWP agent to
support the JDI ThreadReference implementation.<br>
To be safe the JDI & JDWP tests have to be run as well. It is
well covered by the tier-5.<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
On 4/9/20 21:46, Yasumasa Suenaga wrote:<br>
</div>
<blockquote type="cite"
cite="mid:4752820b-21d7-789b-b0e8-8c96af05da6a@oss.nttdata.com">Hi
all,
<br>
<br>
Please review this change:
<br>
<br>
 JBS: <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8242425">https://bugs.openjdk.java.net/browse/JDK-8242425</a>
<br>
 webrev:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~ysuenaga/JDK-8242425/webrev.00/">http://cr.openjdk.java.net/~ysuenaga/JDK-8242425/webrev.00/</a>
<br>
<br>
We've discussed to use Thread-Local Handshake in some JVMTI
functions [1].
<br>
This change is for monitor functions. It affects
GetOwnedMonitorInfo(), GetOwnedMonitorStackDepthInfo(),
GetCurrentContendedMonitor().
<br>
<br>
It passed tests on submit repo
(mach5-one-ysuenaga-JDK-8242425-20200410-0228-10075639), and also
I confirmed to pass following tests:
<br>
<br>
 - serviceability/jvmti/GetOwnedMonitorInfo
<br>
 - serviceability/jvmti/GetOwnedMonitorStackDepthInfo
<br>
 - vmTestbase/nsk/jvmti/GetCurrentContendedMonitor
<br>
 - vmTestbase/nsk/jvmti/GetOwnedMonitorInfo
<br>
<br>
<br>
Thanks,
<br>
<br>
Yasumasa
<br>
<br>
<br>
[1]
<a class="moz-txt-link-freetext" href="https://mail.openjdk.java.net/pipermail/serviceability-dev/2020-March/030890.html">https://mail.openjdk.java.net/pipermail/serviceability-dev/2020-March/030890.html</a><br>
</blockquote>
<br>
</body>
</html>