On 23/07/2020 1:13 pm, Yasumasa Suenaga wrote:
Hi David,
Thanks for your comment!
On 2020/07/23 11:01, David Holmes wrote:
Hi Yasumasa,
On 23/07/2020 12:38 am, Yasumasa Suenaga wrote:
Hi all,
Please review this change:
JBS: https://bugs.openjdk.java.net/browse/JDK-8248362
webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8248362/webrev.00/
Migrate JVMTI frame operations to Thread-Local Handshakes from VM
Operations.
- VM_GetFrameCount
- VM_GetFrameLocation
They affects both GetFrameCount() and GetFrameLocation() JVMTI
functions.
Your changes all seem fine.
But I'm a bit confused about the existing code. In
JvmtiEnv::GetFrameLocation we have:
if (java_thread == JavaThread::current()) {
err = get_frame_location(java_thread, depth, method_ptr,
location_ptr);
but then in get_frame_location we have:
assert((SafepointSynchronize::is_at_safepoint() ||
java_thread->is_thread_fully_suspended(false, &debug_bits)),
"at safepoint or target thread is suspended");
and that assert must surely fire if called by the current thread for
itself! ???
is_thread_fully_suspended() returns true when it calles from current
thread, so this assert would not fire.
I had not realized it was defined that way. Seems like a bit of a kludge
to avoid making the call conditional on a check of the current thread. :(
Cheers,
David
Yasumasa
Thanks,
David
This change has passed all tests on submit repo
(mach5-one-ysuenaga-JDK-8248362-20200722-1249-12859056), and
vmTestbase/nsk/{jdi,jdw,jvmti}, serviceability/{jdwp,jvmti} .
Thanks,
Yasumasa
