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