RFR: 8242428: JVMTI thread operations should use Thread-Local Handshake

David Holmes david.holmes at oracle.com
Fri Jul 3 05:12:47 UTC 2020


On 3/07/2020 1:24 pm, Daniel D. Daugherty wrote:
> On 7/2/20 10:50 PM, David Holmes wrote:
>> Sorry I'm responding here without seeing latest webrev but there is 
>> enough context I think ...
>>
>> On 3/07/2020 9:14 am, Yasumasa Suenaga wrote:
>>> Hi Dan,
>>>
>>> Thanks for your comment!
>>>
>>> On 2020/07/03 7:16, Daniel D. Daugherty wrote:
>>>> On 7/2/20 5:19 AM, Yasumasa Suenaga wrote:
>>>>> Hi David,
>>>>>
>>>>> I upload new webrev. Could you review again?
>>>>>
>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.03/
>>>>
>>>> src/hotspot/share/prims/jvmtiEnv.cpp
>>>>      L1542:     // Get stack trace with handshake
>>>>          nit - please add a period at the end.
>>>
>>> I will fix it.
>>>
>>>
>>>>      L1591:     *stack_info_ptr = op.stack_info();
>>>>          The return parameter should not be touched unless the return
>>>>          code in 'err' == JVMTI_ERROR_NONE.
>>>>
>>>>      old L1582:   if (err == JVMTI_ERROR_NONE) {
>>>>          Please restore this check. The return parameter should not
>>>>          be touched unless the return code in 'err' == 
>>>> JVMTI_ERROR_NONE.
>>>
>>> I will fix it.
>>
>> But op.stack_info() will return NULL if the error is not 
>> JVMTI_ERROR_NONE. Are you (Dan) concerned about someone passing in a 
>> non-null/initialized out-pointer that will be reset to NULL if there 
>> was an error?
> 
> Actually the way we used to test this in POSIX tests is to call
> an API with known bad parameters and the return parameter ptr
> set to NULL. If the return parameter ptr was touched when an
> error should have been detected on an earlier parameter, then
> the test failed.

Okay.

> 
>>
>>>
>>>>      L1272:   if (!jt->is_exiting() && (thread_oop != NULL)) {
>>>>          nit - extra parens around the second expression.
>>>
>>> I will fix it.
>>>
>>>
>>>> src/hotspot/share/prims/jvmtiEnvBase.cpp
>>>>      old L1532:   _result = JVMTI_ERROR_THREAD_NOT_ALIVE;
>>>>          This deletion of the _result field threw me for a minute 
>>>> and then
>>>>          I figured out that the field is init to 
>>>> JVMTI_ERROR_THREAD_NOT_ALIVE
>>>>          in the constructor.
>>>>
>>>>      L1553:   if (!jt->is_exiting() && (jt->threadObj() != NULL)) {
>>>>          nit - extra parens around the second expression.
>>>
>>> I will fix it.
>>>
>>>
>>>> src/hotspot/share/prims/jvmtiEnvBase.hpp
>>>>      No comments.
>>>>
>>>> src/hotspot/share/runtime/vmOperations.hpp
>>>>      No comments.
>>>>
>>>> test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/GetThreadListStackTraces.java 
>>>>
>>>>      No comments.
>>>>
>>>> test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/OneGetThreadListStackTraces.java 
>>>>
>>>>      L64:         startSignal.countDown();
>>>>          I was expecting this to be a call to await() instead of
>>>>          countDown(). What am I missing here?
>>>>
>>>>          I think this test might be passing by accident right now, 
>>>> but...
>>>
>>> Main thread (which call JVMTI functions to test) should wait until 
>>> test thread is ready.
>>> So main thread would wait startSignal, and test thread would count down.
>>
>> No!
>>
>> The test thread that previously called obj.wait() now calls 
>> latch.await().
>>
>> The main thread that previously called obj.notify() now calls 
>> latch.countDown().
>>
>> The main thread continues to spin until it sees the target is WAITING 
>> before proceeding with the test.
> 
> Here's the flow as I see it:
> 
> main thread
>    - start worker thread
>    - startSignal.await()
>      - main is now blocked
> 
> worker thread
>    - startSignal.countDown()
>      - main is now unblocked
>    - stopSignal.await()
>      - worker is now blocked
> 
> main thread
>    - checkCallStacks(th)
>    - stopSignal.countDown()
>      - worker is now unblocked
>    - th.join
>      - main is now blocked
> 
> worker thread
>    - runs off the end of run()
>      - main is now unblocked
> 
> main thread
>    - run off the end of main()

That is not all what I wanted when I said to use the CountDownLatch and 
its broken because it is now racy. :(

Cheers,
David
-----

> 
>>
>>>
>>>> test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libGetThreadListStackTraces.c 
>>>>
>>>>      L92:   jthreads = (jthread *)malloc(sizeof(jthread) * 
>>>> num_threads);
>>>>          You don't check for malloc() failure.
>>>>          'jthreads' is allocated but never freed.
>>>
>>> I will fix it.
>>>
>>>
>>>> test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libOneGetThreadListStackTraces.c 
>>>>
>>>>      L91:   result = (*jvmti)->SuspendThread(jvmti, thread);
>>>>          Why are you suspending the thread? GetAllStackTraces() and
>>>>          GetThreadListStackTraces() do not require the target thread(s)
>>>>          to be suspend.
>>>>
>>>>          If you decide not to SuspendThread, then you don't need the
>>>>          AddCapabilities or the ResumeThread calls.
>>>
>>> Test thread might not be entered following code (stopSignal.await()). 
>>> We might see deferent call stack between GetAllStackTraces() and 
>>> GetThreadListStackTraces(). We cannot control to freeze call stack of 
>>> test thread in Java code.
>>> (I didn't use SuspendThread() at first, but I saw some errors which 
>>> causes in above.)
>>>
>>> So we need to call SuspendThread() to ensure we can see same call stack.
>>
>> If you are checking that the thread is in state WAITING then it cannot 
>> escape from that state and you can sample the stack multiple times 
>> from any API and get the same result.
>>
>> I suspect the errors you saw were from the apparent incorrect use of 
>> the CountDownLatch.
> 
> With the flow outlined above, the worker thread should be
> nicely blocked in stopSignal.await() when stuff is sampled.
> 
> Dan
> 
> 
>>
>> Cheers,
>> David
>> -----
>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>>> Dan
>>>>
>>>>>
>>>>> On 2020/07/02 15:05, David Holmes wrote:
>>>>>> Hi Yasumasa,
>>>>>>
>>>>>> On 1/07/2020 11:53 am, Yasumasa Suenaga wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> I uploaded new webrev. Could review again?
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.02/
>>>>>>
>>>>>> Updates look fine - thanks.
>>>>>>
>>>>>> One minor nit:
>>>>>>
>>>>>> 1274     _collector.allocate_and_fill_stacks(1);
>>>>>> 1275     _collector.set_result(JVMTI_ERROR_NONE);
>>>>>>
>>>>>> In the other places where you use _collector you rely on result 
>>>>>> being initialized to JVMTI_ERROR_NONE, rather than setting it 
>>>>>> directly after allocate_and_fill_stacks().
>>>>>
>>>>> Fixed.
>>>>>
>>>>>
>>>>>>>>>>>> src/hotspot/share/prims/jvmtiEnvBase.cpp
>>>>>>>>>>>>
>>>>>>>>>>>>   820 assert(SafepointSynchronize::is_at_safepoint() ||
>>>>>>>>>>>>   821 java_thread->is_thread_fully_suspended(false, 
>>>>>>>>>>>> &debug_bits) ||
>>>>>>>>>>>>   822          current_thread == 
>>>>>>>>>>>> java_thread->active_handshaker(),
>>>>>>>>>>>>   823          "at safepoint / handshake or target thread is 
>>>>>>>>>>>> suspended");
>>>>>>>>>>>>
>>>>>>>>>>>> I don't think the suspension check is necessary, as even if 
>>>>>>>>>>>> the target is suspended we must still be at a safepoint or 
>>>>>>>>>>>> in a handshake with it. Makes me wonder if we used to allow 
>>>>>>>>>>>> a racy stacktrace operation on a suspended thread, assuming 
>>>>>>>>>>>> it would remain suspended?
>>>>>>>
>>>>>>> This function (JvmtiEnvBase::get_stack_trace()) can be called to 
>>>>>>> get own stack trace. For example, we can call GetStackTrace() for 
>>>>>>> current thread at JVMTI event.
>>>>>>> So I changed assert as below:
>>>>>>>
>>>>>>> ```
>>>>>>>   820   assert(current_thread == java_thread ||
>>>>>>>   821          SafepointSynchronize::is_at_safepoint() ||
>>>>>>>   822          current_thread == java_thread->active_handshaker(),
>>>>>>>   823          "call by myself / at safepoint / at handshake");
>>>>>>> ```
>>>>>>
>>>>>> Yep good catch. I hope current tests caught that.
>>>>>
>>>>> They would be tested in 
>>>>> vmTestbase/nsk/jvmti/GetStackTrace/getstacktr001/ (own call 
>>>>> stacks), and getstacktr003 (call stacks in other thread).
>>>>>
>>>>>
>>>>>> Speaking of tests ...
>>>>>>
>>>>>> In the native code I think you need to check the success of all 
>>>>>> JNI methods that can throw exceptions - otherwise I believe the 
>>>>>> tests may trigger warnings if -Xcheck:jni is used with them. See 
>>>>>> for example:
>>>>>>
>>>>>> test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/libHeapMonitorTest.cpp 
>>>>>>
>>>>>
>>>>> I updated testcases to check JNI and JVMTI function calls.
>>>>>
>>>>>
>>>>>> In the Java code the target thread:
>>>>>>
>>>>>>    45     public void run() {
>>>>>>    46       try {
>>>>>>    47           synchronized (lock) {
>>>>>>    48               lock.wait();
>>>>>>    49               System.out.println("OK");
>>>>>>    50           }
>>>>>>
>>>>>> is potentially susceptible to spurious wakeups. Using a 
>>>>>> CountDownLatch would be robust.
>>>>>
>>>>> Fixed.
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>> -----
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>> On 2020/07/01 8:48, David Holmes wrote:
>>>>>>>> Hi Yasumasa,
>>>>>>>>
>>>>>>>> On 1/07/2020 9:05 am, Yasumasa Suenaga wrote:
>>>>>>>>> Hi David,
>>>>>>>>>
>>>>>>>>>>>> 1271     ResourceMark rm;
>>>>>>>>>>>>
>>>>>>>>>>>> IIUC at this point the _calling_thread is the current 
>>>>>>>>>>>> thread, so we can use:
>>>>>>>>>>>>
>>>>>>>>>>>>      ResourceMark rm(_calling_thread);
>>>>>>>>>
>>>>>>>>> If so, we can call make_local() in L1272 without JavaThread (or 
>>>>>>>>> we can pass current thread to make_local()). Is it right?
>>>>>>>>>
>>>>>>>>> ```
>>>>>>>>> 1271     ResourceMark rm;
>>>>>>>>> 1272 
>>>>>>>>> _collector.fill_frames((jthread)JNIHandles::make_local(_calling_thread, 
>>>>>>>>> thread_oop),
>>>>>>>>> 1273                            jt, thread_oop);
>>>>>>>>> ```
>>>>>>>>
>>>>>>>> Sorry I got confused, _calling_thread may not be the current 
>>>>>>>> thread as we could be executing the handshake in the target 
>>>>>>>> thread itself. So the ResourceMark is correct as-is (implicitly 
>>>>>>>> for current thread).
>>>>>>>>
>>>>>>>> The argument to fill_frames will be used in the jvmtiStackInfo 
>>>>>>>> and passed back to the _calling_thread, so it must be created 
>>>>>>>> via make_local(_calling_thread, ...) as you presently have.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> David
>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2020/07/01 7:05, David Holmes wrote:
>>>>>>>>>> On 1/07/2020 12:17 am, Yasumasa Suenaga wrote:
>>>>>>>>>>> Hi David,
>>>>>>>>>>>
>>>>>>>>>>> Thank you for reviewing! I will update new webrev tomorrow.
>>>>>>>>>>>
>>>>>>>>>>>> 466 class MultipleStackTracesCollector : public StackObj {
>>>>>>>>>>>>
>>>>>>>>>>>>   498 class VM_GetAllStackTraces : public VM_Operation {
>>>>>>>>>>>>   499 private:
>>>>>>>>>>>>   500   JavaThread *_calling_thread;
>>>>>>>>>>>>   501   jint _final_thread_count;
>>>>>>>>>>>>   502   MultipleStackTracesCollector _collector;
>>>>>>>>>>>>
>>>>>>>>>>>> You can't have a StackObj as a member of another class like 
>>>>>>>>>>>> that as it may not be on the stack. I think 
>>>>>>>>>>>> MultipleStackTracesCollector should not extend any 
>>>>>>>>>>>> allocation class, and should always be embedded directly in 
>>>>>>>>>>>> another class.
>>>>>>>>>>>
>>>>>>>>>>> I'm not sure what does mean "embedded".
>>>>>>>>>>> Is it ok as below?
>>>>>>>>>>>
>>>>>>>>>>> ```
>>>>>>>>>>> class MultipleStackTracesCollector {
>>>>>>>>>>>     :
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>> class GetAllStackTraces : public VM_Operation {
>>>>>>>>>>>    private:
>>>>>>>>>>>      MultipleStackTracesCollector _collector;
>>>>>>>>>>> }
>>>>>>>>>>> ```
>>>>>>>>>>
>>>>>>>>>> Yes that I what I meant.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> David
>>>>>>>>>> -----
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>>
>>>>>>>>>>> Yasumasa
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 2020/06/30 22:22, David Holmes wrote:
>>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>>
>>>>>>>>>>>> On 30/06/2020 10:05 am, Yasumasa Suenaga wrote:
>>>>>>>>>>>>> Hi David, Serguei,
>>>>>>>>>>>>>
>>>>>>>>>>>>> I updated webrev for 8242428. Could you review again?
>>>>>>>>>>>>> This change migrate to use direct handshake for 
>>>>>>>>>>>>> GetStackTrace() and GetThreadListStackTraces() (when 
>>>>>>>>>>>>> thread_count == 1).
>>>>>>>>>>>>>
>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.01/
>>>>>>>>>>>>
>>>>>>>>>>>> This looks really good now! I only have a few nits below. 
>>>>>>>>>>>> There is one thing I don't like about it but it requires a 
>>>>>>>>>>>> change to the main Handshake logic to address - in 
>>>>>>>>>>>> JvmtiEnv::GetThreadListStackTraces you have to create a 
>>>>>>>>>>>> ThreadsListHandle to convert the jthread to a JavaThread, 
>>>>>>>>>>>> but then the Handshake::execute_direct creates another 
>>>>>>>>>>>> ThreadsListHandle internally. That's a waste. I will discuss 
>>>>>>>>>>>> with Robbin and file a RFE to have an overload of 
>>>>>>>>>>>> execute_direct that takes an existing TLH. Actually it's 
>>>>>>>>>>>> worse than that because we have another TLH in use at the 
>>>>>>>>>>>> entry point for the JVMTI functions, so I think there may be 
>>>>>>>>>>>> some scope for simplifying the use of TLH instances - future 
>>>>>>>>>>>> RFE.
>>>>>>>>>>>>
>>>>>>>>>>>> ---
>>>>>>>>>>>>
>>>>>>>>>>>> src/hotspot/share/prims/jvmtiEnvBase.hpp
>>>>>>>>>>>>
>>>>>>>>>>>>   451   GetStackTraceClosure(JvmtiEnv *env, jint 
>>>>>>>>>>>> start_depth, jint max_count,
>>>>>>>>>>>>   452                        jvmtiFrameInfo* frame_buffer, 
>>>>>>>>>>>> jint* count_ptr)
>>>>>>>>>>>>   453     : HandshakeClosure("GetStackTrace"),
>>>>>>>>>>>>   454       _env(env), _start_depth(start_depth), 
>>>>>>>>>>>> _max_count(max_count),
>>>>>>>>>>>>   455       _frame_buffer(frame_buffer), _count_ptr(count_ptr),
>>>>>>>>>>>>   456 _result(JVMTI_ERROR_THREAD_NOT_ALIVE) {
>>>>>>>>>>>>
>>>>>>>>>>>> Nit: can you do one initializer per line please.
>>>>>>>>>>>>
>>>>>>>>>>>> This looks wrong:
>>>>>>>>>>>>
>>>>>>>>>>>> 466 class MultipleStackTracesCollector : public StackObj {
>>>>>>>>>>>>
>>>>>>>>>>>>   498 class VM_GetAllStackTraces : public VM_Operation {
>>>>>>>>>>>>   499 private:
>>>>>>>>>>>>   500   JavaThread *_calling_thread;
>>>>>>>>>>>>   501   jint _final_thread_count;
>>>>>>>>>>>>   502   MultipleStackTracesCollector _collector;
>>>>>>>>>>>>
>>>>>>>>>>>> You can't have a StackObj as a member of another class like 
>>>>>>>>>>>> that as it may not be on the stack. I think 
>>>>>>>>>>>> MultipleStackTracesCollector should not extend any 
>>>>>>>>>>>> allocation class, and should always be embedded directly in 
>>>>>>>>>>>> another class.
>>>>>>>>>>>>
>>>>>>>>>>>> 481   MultipleStackTracesCollector(JvmtiEnv *env, jint 
>>>>>>>>>>>> max_frame_count) {
>>>>>>>>>>>>   482     _env = env;
>>>>>>>>>>>>   483     _max_frame_count = max_frame_count;
>>>>>>>>>>>>   484     _frame_count_total = 0;
>>>>>>>>>>>>   485     _head = NULL;
>>>>>>>>>>>>   486     _stack_info = NULL;
>>>>>>>>>>>>   487     _result = JVMTI_ERROR_NONE;
>>>>>>>>>>>>   488   }
>>>>>>>>>>>>
>>>>>>>>>>>> As you are touching this can you change it to use an 
>>>>>>>>>>>> initializer list as you did for the HandshakeClosure, and 
>>>>>>>>>>>> please keep one item per line.
>>>>>>>>>>>>
>>>>>>>>>>>> ---
>>>>>>>>>>>>
>>>>>>>>>>>> src/hotspot/share/prims/jvmtiEnvBase.cpp
>>>>>>>>>>>>
>>>>>>>>>>>>   820 assert(SafepointSynchronize::is_at_safepoint() ||
>>>>>>>>>>>>   821 java_thread->is_thread_fully_suspended(false, 
>>>>>>>>>>>> &debug_bits) ||
>>>>>>>>>>>>   822          current_thread == 
>>>>>>>>>>>> java_thread->active_handshaker(),
>>>>>>>>>>>>   823          "at safepoint / handshake or target thread is 
>>>>>>>>>>>> suspended");
>>>>>>>>>>>>
>>>>>>>>>>>> I don't think the suspension check is necessary, as even if 
>>>>>>>>>>>> the target is suspended we must still be at a safepoint or 
>>>>>>>>>>>> in a handshake with it. Makes me wonder if we used to allow 
>>>>>>>>>>>> a racy stacktrace operation on a suspended thread, assuming 
>>>>>>>>>>>> it would remain suspended?
>>>>>>>>>>>>
>>>>>>>>>>>> 1268   oop thread_oop = jt->threadObj();
>>>>>>>>>>>> 1269
>>>>>>>>>>>> 1270   if (!jt->is_exiting() && (jt->threadObj() != NULL)) {
>>>>>>>>>>>>
>>>>>>>>>>>> You can use thread_oop in line 1270.
>>>>>>>>>>>>
>>>>>>>>>>>> 1272 
>>>>>>>>>>>> _collector.fill_frames((jthread)JNIHandles::make_local(_calling_thread, 
>>>>>>>>>>>> thread_oop),
>>>>>>>>>>>> 1273                            jt, thread_oop);
>>>>>>>>>>>>
>>>>>>>>>>>> It is frustrating that this entire call chain started with a 
>>>>>>>>>>>> jthread reference, which we converted to a JavaThread, only 
>>>>>>>>>>>> to eventually need to convert it back to a jthread! I think 
>>>>>>>>>>>> there is some scope for simplification here but not as part 
>>>>>>>>>>>> of this change.
>>>>>>>>>>>>
>>>>>>>>>>>> 1271     ResourceMark rm;
>>>>>>>>>>>>
>>>>>>>>>>>> IIUC at this point the _calling_thread is the current 
>>>>>>>>>>>> thread, so we can use:
>>>>>>>>>>>>
>>>>>>>>>>>>      ResourceMark rm(_calling_thread);
>>>>>>>>>>>>
>>>>>>>>>>>> ---
>>>>>>>>>>>>
>>>>>>>>>>>> Please add @bug lines to the tests.
>>>>>>>>>>>>
>>>>>>>>>>>> I'm still pondering the test logic but wanted to send this now.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> David
>>>>>>>>>>>> -----
>>>>>>>>>>>>> VM_GetThreadListStackTrace (for GetThreadListStackTraces) 
>>>>>>>>>>>>> and VM_GetAllStackTraces (for GetAllStackTraces) have 
>>>>>>>>>>>>> inherited VM_GetMultipleStackTraces VM operation which 
>>>>>>>>>>>>> provides the feature to generate jvmtiStackInfo. I modified 
>>>>>>>>>>>>> VM_GetMultipleStackTraces to a normal C++ class to share 
>>>>>>>>>>>>> with HandshakeClosure for GetThreadListStackTraces 
>>>>>>>>>>>>> (GetSingleStackTraceClosure).
>>>>>>>>>>>>>
>>>>>>>>>>>>> Also I added new testcases which test 
>>>>>>>>>>>>> GetThreadListStackTraces() with thread_count == 1 and with 
>>>>>>>>>>>>> all threads.
>>>>>>>>>>>>>
>>>>>>>>>>>>> This change has been tested in serviceability/jvmti 
>>>>>>>>>>>>> serviceability/jdwp vmTestbase/nsk/jvmti vmTestbase/nsk/jdi 
>>>>>>>>>>>>> vmTestbase/nsk/jdwp.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 2020/06/24 15:50, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Please review this change:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>    JBS: https://bugs.openjdk.java.net/browse/JDK-8242428
>>>>>>>>>>>>>>    webrev: 
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.00/
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> This change replace following VM operations to direct 
>>>>>>>>>>>>>> handshake.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>   - VM_GetFrameCount (GetFrameCount())
>>>>>>>>>>>>>>   - VM_GetFrameLocation (GetFrameLocation())
>>>>>>>>>>>>>>   - VM_GetThreadListStackTraces (GetThreadListStackTrace())
>>>>>>>>>>>>>>   - VM_GetCurrentLocation
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> GetThreadListStackTrace() uses direct handshake if thread 
>>>>>>>>>>>>>> count == 1. In other case (thread count > 1), it would be 
>>>>>>>>>>>>>> performed as VM operation (VM_GetThreadListStackTraces).
>>>>>>>>>>>>>> Caller of VM_GetCurrentLocation 
>>>>>>>>>>>>>> (JvmtiEnvThreadState::reset_current_location()) might be 
>>>>>>>>>>>>>> called at safepoint. So I added safepoint check in its 
>>>>>>>>>>>>>> caller.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> This change has been tested in serviceability/jvmti 
>>>>>>>>>>>>>> serviceability/jdwp vmTestbase/nsk/jvmti 
>>>>>>>>>>>>>> vmTestbase/nsk/jdi vmTestbase/ns
>>>>>>>>>>>>>> k/jdwp.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Also I tested it on submit repo, then it has execution 
>>>>>>>>>>>>>> error 
>>>>>>>>>>>>>> (mach5-one-ysuenaga-JDK-8242428-20200624-0054-12034717) 
>>>>>>>>>>>>>> due to dependency error. So I think it does not occur by 
>>>>>>>>>>>>>> this change.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Yasumasa
>>>>
> 


More information about the serviceability-dev mailing list