RFR: 8242428: JVMTI thread operations should use Thread-Local Handshake
Patricio Chilano
patricio.chilano.mateo at oracle.com
Thu Jul 9 15:00:23 UTC 2020
Hi Yasumasa,
On 7/9/20 9:30 AM, Yasumasa Suenaga wrote:
> On 2020/07/09 17:58, David Holmes wrote:
>> Hi Yasumasa,
>>
>> On 9/07/2020 10:25 am, Yasumasa Suenaga wrote:
>>> Hi Dan,
>>>
>>> Thanks for your comment!
>>> I uploaded new webrev:
>>>
>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.09/
>>> Diff from previous webrev:
>>> http://hg.openjdk.java.net/jdk/submit/rev/5d167adf8524
>>>
>>> I saw similar build errors in libOneGetThreadListStackTraces.cpp on
>>> Windows.
>>> This webrev fixes them.
>>
>> You shouldn't use %p as it may not be portable. In the VM we use
>> INTPTR_FORMAT and convert the arg using p2i. I don't know what exists
>> in the testing code.
>
> I replaced %p to %lx, and also cast values to unsigned long [1] [2],
> but the test on submit repo was failed.
> Can anyone share details of
> mach5-one-ysuenaga-JDK-8242428-20200709-1030-12500928 ?
These are the errors I see for the macOS build:
./open/test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libOneGetThreadListStackTraces.cpp:53:14:
error: format specifies type 'long long' but the argument has type
'jlocation' (aka 'long') [-Werror,-Wformat]
fi1->location, fi2->location);
^~~~~~~~~~~~~
./open/test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libOneGetThreadListStackTraces.cpp:53:29: error: format specifies type 'long long' but the argument has type 'jlocation' (aka 'long') [-Werror,-Wformat]
fi1->location, fi2->location);
^~~~~~~~~~~~~
These are the ones I see for the Windows build:
./open/test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libOneGetThreadListStackTraces.cpp(48): warning C4311: 'type cast': pointer truncation from 'jmethodID' to 'unsigned long'
./open/test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libOneGetThreadListStackTraces.cpp(48): warning C4302: 'type cast': truncation from 'jmethodID' to 'unsigned long'
./open/test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libOneGetThreadListStackTraces.cpp(48): warning C4311: 'type cast': pointer truncation from 'jmethodID' to 'unsigned long'
./open/test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libOneGetThreadListStackTraces.cpp(48): warning C4302: 'type cast': truncation from 'jmethodID' to 'unsigned long'
./open/test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libOneGetThreadListStackTraces.cpp(70): warning C4311: 'type cast': pointer truncation from 'jthread' to 'unsigned long'
./open/test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libOneGetThreadListStackTraces.cpp(70): warning C4302: 'type cast': truncation from 'jthread' to 'unsigned long'
./open/test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libOneGetThreadListStackTraces.cpp(70): warning C4311: 'type cast': pointer truncation from 'jthread' to 'unsigned long'
./open/test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libOneGetThreadListStackTraces.cpp(70): warning C4302: 'type cast': truncation from 'jthread' to 'unsigned long'
You will probably want to use the macros defined in
src/hotspot/share/utilities/globalDefinitions.hpp. Let me know if you
need me to test something.
Thanks,
Patricio
> Thanks,
>
> Yasumasa
>
>
> [1] http://hg.openjdk.java.net/jdk/submit/rev/dfca51958217
> [2] http://hg.openjdk.java.net/jdk/submit/rev/3665361fa91b
>
>
>> David
>> -----
>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>> On 2020/07/09 1:42, Daniel D. Daugherty wrote:
>>>> > http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.08/
>>>>
>>>> src/hotspot/share/prims/jvmtiEnv.cpp
>>>> No comments.
>>>>
>>>> src/hotspot/share/prims/jvmtiEnvBase.cpp
>>>> L1159: Thread *current_thread = Thread::current();
>>>> Please add "#ifdef ASSERT" above and "#endif" below since
>>>> current_thread is only used for the assert() in this
>>>> function.
>>>>
>>>> src/hotspot/share/prims/jvmtiEnvBase.hpp
>>>> L549: jthread java_thread, jint
>>>> max_frame_count)
>>>> L552: _jthread(java_thread),
>>>> Please: s/java_thread/thread/ on both lines.
>>>>
>>>> src/hotspot/share/runtime/vmOperations.hpp
>>>> No comments.
>>>>
>>>> test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/OneGetThreadListStackTraces.java
>>>>
>>>> No comments.
>>>>
>>>> test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libOneGetThreadListStackTraces.cpp
>>>>
>>>> L27: #include <errno.h>
>>>> This include is out of order; should be first in the list.
>>>>
>>>> This file doesn't compile on my MBP13:
>>>>
>>>> ./open/test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libOneGetThreadListStackTraces.cpp:49:14:
>>>> error: format specifies type 'unsigned long' but the argument has
>>>> type 'jmethodID' (aka '_jmethodID *') [-Werror,-Wformat]
>>>> fi1->method, fi2->method);
>>>> ^~~~~~~~~~~
>>>> ./open/test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libOneGetThreadListStackTraces.cpp:49:27:
>>>> error: format specifies type 'unsigned long' but the argument has
>>>> type 'jmethodID' (aka '_jmethodID *') [-Werror,-Wformat]
>>>> fi1->method, fi2->method);
>>>> ^~~~~~~~~~~
>>>> 2 errors generated.
>>>>
>>>> This change made it compile on my MBP13, but that may break it on
>>>> other platforms:
>>>>
>>>> $ hg diff
>>>> diff -r 560847c69fbe
>>>> test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libOneGetThreadListStackTraces.cpp
>>>> ---
>>>> a/test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libOneGetThreadListStackTraces.cpp
>>>> Wed Jul 08 12:13:32 2020 -0400
>>>> +++
>>>> b/test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libOneGetThreadListStackTraces.cpp
>>>> Wed Jul 08 12:40:42 2020 -0400
>>>> @@ -46,7 +46,7 @@
>>>> if (fi1->method != fi2->method) { /* jvmtiFrameInfo::method */
>>>> snprintf(err_msg, sizeof(err_msg),
>>>> "method is different: fi1 = %lx, fi2 = %lx",
>>>> - fi1->method, fi2->method);
>>>> + (unsigned long) fi1->method, (unsigned long)
>>>> fi2->method);
>>>> env->FatalError(err_msg);
>>>> } else if (fi1->location != fi2->location) { /*
>>>> jvmtiFrameInfo::location */
>>>> snprintf(err_msg, sizeof(err_msg),
>>>>
>>>> I'm not sure of the right platform independent way to output
>>>> the 'method' field.
>>>>
>>>> Dan
>>>>
>>>>
>>>> On 7/8/20 4:04 AM, Yasumasa Suenaga wrote:
>>>>> Hi David,
>>>>>
>>>>> On 2020/07/08 15:27, David Holmes wrote:
>>>>>> Hi Yasumasa,
>>>>>>
>>>>>> On 7/07/2020 6:54 pm, Yasumasa Suenaga wrote:
>>>>>>> Hi David, Serguei,
>>>>>>>
>>>>>>> Serguei, thank you for replying even though you are on vacaiton!
>>>>>>>
>>>>>>> I uploaded new webrev:
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.07/
>>>>>>> Diff from previous webrev:
>>>>>>> http://hg.openjdk.java.net/jdk/submit/rev/77243b1dcbfe
>>>>>>>
>>>>>>> c'tor of GetSingleStackTraceClosure has jthread argument in this
>>>>>>> webrev.
>>>>>>> Also it does not contain testcase for GetThreadListStackTraces
>>>>>>> with all threads, and OneGetThreadListStackTraces would test
>>>>>>> main thread only.
>>>>>>
>>>>>> All those changes are fine in principle for me. One nit/suggestion:
>>>>>>
>>>>>> src/hotspot/share/prims/jvmtiEnvBase.hpp
>>>>>>
>>>>>> 544 jthread _java_thread;
>>>>>>
>>>>>> elsewhere "java_thread" refers to a JavaThread, so to avoid
>>>>>> confusion may I suggest this member be named _jthread.
>>>>>
>>>>> I uploaded new webrev:
>>>>>
>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.08/
>>>>> Diff from previous webrev:
>>>>> http://hg.openjdk.java.net/jdk/submit/rev/ca6263dbdc87
>>>>>
>>>>>
>>>>>> I'm going to be away for the next couple of days - sorry - but
>>>>>> will try to check email on this if I can.
>>>>>
>>>>> Thanks!
>>>>>
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>> -----
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>> On 2020/07/07 15:13, David Holmes wrote:
>>>>>>>> On 7/07/2020 2:57 pm, Yasumasa Suenaga wrote:
>>>>>>>>> Hi David,
>>>>>>>>>
>>>>>>>>> On 2020/07/07 11:31, David Holmes wrote:
>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>
>>>>>>>>>> Hard to keep up with the changes - especially without
>>>>>>>>>> incremental webrevs.
>>>>>>>>>
>>>>>>>>> Sorry, I will upload diff from previous webrev in the next.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> If GetSingleStackTraceClosure also took the jthread as a
>>>>>>>>>> constructor arg, then you wouldn't need to recreate a JNI
>>>>>>>>>> local handle when calling _collector.fill_frames. It's a
>>>>>>>>>> small simplification and not essential at this stage.
>>>>>>>>>
>>>>>>>>> I think we should get jthread from an argument of do_thread()
>>>>>>>>> because do_thread() would pass the thread which are stopped
>>>>>>>>> certainly.
>>>>>>>>> It might be simplification if we pass _calling_thread to
>>>>>>>>> MultipleStackTracesCollector. `jthread` is only needed to
>>>>>>>>> store jvmtiStackInfo.thread . What do you think?
>>>>>>>>
>>>>>>>> I'm not quite sure what you mean.
>>>>>>>>
>>>>>>>> I think there is a bit of a design wart with direct handshakes
>>>>>>>> in that do_thread takes the target JavaThread as an argument.
>>>>>>>> That's useful in a case where you want a HandshakeClosure that
>>>>>>>> can be applied to multiple threads, but that's not typically
>>>>>>>> what is needed with direct handshakes - there is only a single
>>>>>>>> target. With a single-target HandshakeClosure you can capture
>>>>>>>> all the "target" information for the operation in the closure
>>>>>>>> instance. So if the actual do_thread operation wants the
>>>>>>>> jthread corresponding to the target thread then we can store
>>>>>>>> that in the closure rather than recomputing it (you could
>>>>>>>> assert it is the same but that seems overkill to me).
>>>>>>>>
>>>>>>>>>
>>>>>>>>>> For the test ... I don't see how
>>>>>>>>>> Java_GetThreadListStackTraces_checkCallStacks is a valid
>>>>>>>>>> test. It gets the stacks of all live threads, then uses that
>>>>>>>>>> information to use GetThreadListStackTraces to get the stack
>>>>>>>>>> for the same set of threads through a different API. It then
>>>>>>>>>> compares the two sets of stacks for each thread expecting
>>>>>>>>>> them to be the same, but that need only be the case for the
>>>>>>>>>> main thread. Other threads could potentially have a different
>>>>>>>>>> stack (e.g. if this test is run with JFR enabled there will
>>>>>>>>>> be additional threads found.) Further I would have expected
>>>>>>>>>> that there already exist tests that check that, for a given
>>>>>>>>>> thread (which may be suspended or known to be blocked) the
>>>>>>>>>> same stack is found through the two different APIs.
>>>>>>>>>
>>>>>>>>> vmTestbase/nsk/jvmti/unit/GetAllStackTraces/getallstktr001
>>>>>>>>> would check all of threads via GetThreadListStackTraces() and
>>>>>>>>> GetAllStackTraces(), so we might be able to remove
>>>>>>>>> GetThreadListStackTraces.java from this webrev.
>>>>>>>>
>>>>>>>> Yes. The existing test only examines a set of test threads that
>>>>>>>> are all blocked on a raw monitor. You do not need to duplicate
>>>>>>>> that test.
>>>>>>>>
>>>>>>>>> OTOH we don't have testcase for GetThreadListStackTraces()
>>>>>>>>> with thread_count == 1, so we need to testcase for it (it is
>>>>>>>>> OneGetThreadListStackTraces.java) It would check whether the
>>>>>>>>> state of target thread is "waiting" before JNI call to call
>>>>>>>>> GetThreadListStackTraces(),
>>>>>>>>
>>>>>>>> Yes we need to test the special cases introduced by your
>>>>>>>> changes - totally agree - and OneGetThreadListStackTraces.java
>>>>>>>> is a good test for that.
>>>>>>>>
>>>>>>>>> and also I expect it would not be run with JFR. (it is not
>>>>>>>>> described @run)
>>>>>>>>
>>>>>>>> The arguments to run with JFR (or a bunch of other things) can
>>>>>>>> be passed to the jtreg test harness to be applied to all tests.
>>>>>>>>
>>>>>>>>> Of course we can check GetThreadListStackTraces() with main
>>>>>>>>> thread, but it is not the test for direct handshake for other
>>>>>>>>> thread.
>>>>>>>>
>>>>>>>> Right - that test already exists as per the above.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> David
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> David
>>>>>>>>>> -----
>>>>>>>>>>
>>>>>>>>>> On 6/07/2020 11:29 pm, Yasumasa Suenaga wrote:
>>>>>>>>>>> Hi Serguei,
>>>>>>>>>>>
>>>>>>>>>>> Thanks for your comment!
>>>>>>>>>>>
>>>>>>>>>>> I think C++ is more simple to implement the test agent as
>>>>>>>>>>> you said.
>>>>>>>>>>> So I implement it in C++ in new webrev. Could you review again?
>>>>>>>>>>>
>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.06/
>>>>>>>>>>>
>>>>>>>>>>> Also I refactored libGetThreadListStackTraces.cpp, and I've
>>>>>>>>>>> kept exception check after IsSameObject().
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>>
>>>>>>>>>>> Yasumasa
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 2020/07/06 16:32, serguei.spitsyn at oracle.com wrote:
>>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>>
>>>>>>>>>>>> Thank you for the update.
>>>>>>>>>>>> I think, a pending exception after IsSameObject needs to be
>>>>>>>>>>>> checked.
>>>>>>>>>>>>
>>>>>>>>>>>> The checkStackInfo() needs one more refactoring as I've
>>>>>>>>>>>> already suggested.
>>>>>>>>>>>> The body of the loop at L68-L78 should be converted to a
>>>>>>>>>>>> function check_frame_info.
>>>>>>>>>>>> The si1->frame_buffer[i] and si2->frame_buffer[i] will be
>>>>>>>>>>>> passed as fi1 and fi2.
>>>>>>>>>>>> The index can be passed as well.
>>>>>>>>>>>> I'm still suggesting to simplify the local exception_msg to
>>>>>>>>>>>> something shorter like err_msg or exc_msg.
>>>>>>>>>>>>
>>>>>>>>>>>> I'm not sure using fatal is right here:
>>>>>>>>>>>>
>>>>>>>>>>>> This fragment looks strange:
>>>>>>>>>>>>
>>>>>>>>>>>> 152 if ((*env)->IsSameObject(env,
>>>>>>>>>>>> stack_info[i].thread, thread)) {
>>>>>>>>>>>> 153 target_info = &stack_info[i];
>>>>>>>>>>>> 154 break;
>>>>>>>>>>>> 155 } else if ((*env)->ExceptionOccurred(env)) {
>>>>>>>>>>>> 156 (*env)->ExceptionDescribe(env);
>>>>>>>>>>>> 157 (*env)->FatalError(env, __FILE__);
>>>>>>>>>>>> 158 }
>>>>>>>>>>>>
>>>>>>>>>>>> I expected it to be:
>>>>>>>>>>>>
>>>>>>>>>>>> jboolean same = (*env)->IsSameObject(env,
>>>>>>>>>>>> stack_info[i].thread, thread);
>>>>>>>>>>>> if ((*env)->ExceptionOccurred(env)) {
>>>>>>>>>>>> (*env)->ExceptionDescribe(env);
>>>>>>>>>>>> (*env)->FatalError(env, __FILE__);
>>>>>>>>>>>> }
>>>>>>>>>>>> if (same) {
>>>>>>>>>>>> target_info = &stack_info[i];
>>>>>>>>>>>> break;
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>> Would it better to port this agent to C++ to simplify this
>>>>>>>>>>>> code nicer?
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Serguei
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On 7/5/20 06:13, Yasumasa Suenaga wrote:
>>>>>>>>>>>>> Hi Serguei,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks for your comment!
>>>>>>>>>>>>> I refactored testcase. Could you review again?
>>>>>>>>>>>>>
>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.05/
>>>>>>>>>>>>>
>>>>>>>>>>>>> It would check Java exception after IsSameObject() call.
>>>>>>>>>>>>> Does it need?
>>>>>>>>>>>>> Any exceptions are not described in JNI document[1], and
>>>>>>>>>>>>> JNI implementation (jni_IsSameObject()) does not seem to
>>>>>>>>>>>>> throw it.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> [1]
>>>>>>>>>>>>> https://docs.oracle.com/en/java/javase/14/docs/specs/jni/functions.html#issameobject
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 2020/07/05 14:46, serguei.spitsyn at oracle.com wrote:
>>>>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Okay, thanks.
>>>>>>>>>>>>>> Then I'm okay to keep the GetSingleStackTraceClosure.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.04/test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libGetThreadListStackTraces.c.html
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.04/test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libOneGetThreadListStackTraces.c.html
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I'm not sure the function 'is_same_thread() is needed.
>>>>>>>>>>>>>> Why do not use the JNI IsSameObject instead?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> It seems to be a typo at L132 and L137.
>>>>>>>>>>>>>> You, probably. did not want to print the same information
>>>>>>>>>>>>>> for stack_info_1[i].frame_buffer[j].XXX twice.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The code at lines 112-142 is not readable.
>>>>>>>>>>>>>> I'd suggest to make a couple of refactoring steps.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> First step to simplify this a little bit would be with
>>>>>>>>>>>>>> some renaming and getting rid of indexes:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 71 char err_msg[EXCEPTION_MSG_LEN] = {0};
>>>>>>>>>>>>>> ...
>>>>>>>>>>>>>> 112 /* Iterate all jvmtiStackInfo to check */
>>>>>>>>>>>>>> 113 for (i = 0; i < num_threads, *exception_msg !=
>>>>>>>>>>>>>> '\0'; i++) {
>>>>>>>>>>>>>> jvmtiStackInfo *si1 = stack_info_1[i];
>>>>>>>>>>>>>> jvmtiStackInfo *si2 = stack_info_2[i];
>>>>>>>>>>>>>> 114 if (!IsSameObject(env, si1.thread, si2.thread))
>>>>>>>>>>>>>> { /* jvmtiStackInfo::thread */
>>>>>>>>>>>>>> 115 snprintf(err_msg, sizeof(err_msg),
>>>>>>>>>>>>>> 116 "thread[%d] is different:
>>>>>>>>>>>>>> stack_info_1 = %p, stack_info_2 = %p",
>>>>>>>>>>>>>> 117 i, sinfo1.thread, sinfo2.thread);
>>>>>>>>>>>>>> 118 } else if (si1.state != si2.state) { /*
>>>>>>>>>>>>>> jvmtiStackInfo::state */
>>>>>>>>>>>>>> 119 snprintf(err_msg, sizeof(err_msg),
>>>>>>>>>>>>>> 120 "state[%d] is different:
>>>>>>>>>>>>>> stack_info_1 = %d, stack_info_2 = %d",
>>>>>>>>>>>>>> 121 i, si1.state, si2.state);
>>>>>>>>>>>>>> 122 } else if (si1.frame_count != si2.frame_count)
>>>>>>>>>>>>>> { /* jvmtiStackInfo::frame_count */
>>>>>>>>>>>>>> 123 snprintf(err_msg, sizeof(err_msg),
>>>>>>>>>>>>>> 124 "frame_count[%d] is different:
>>>>>>>>>>>>>> stack_info_1 = %d, stack_info_2 = %d",
>>>>>>>>>>>>>> 125 i, si1.frame_count, si2.frame_count);
>>>>>>>>>>>>>> 126 } else {
>>>>>>>>>>>>>> 127 /* Iterate all jvmtiFrameInfo to check */
>>>>>>>>>>>>>> 128 for (j = 0; j < si1.frame_count; j++) {
>>>>>>>>>>>>>> 129 if (si1.frame_buffer[j].method !=
>>>>>>>>>>>>>> si1.frame_buffer[j].method) { /* jvmtiFrameInfo::method */
>>>>>>>>>>>>>> 130 snprintf(err_msg, sizeof(err_msg),
>>>>>>>>>>>>>> 131 "thread [%d]
>>>>>>>>>>>>>> frame_buffer[%d].method is different: stack_info_1 = %lx,
>>>>>>>>>>>>>> stack_info_2 = %lx",
>>>>>>>>>>>>>> 132 i, j,
>>>>>>>>>>>>>> si1.frame_buffer[j].method, si2.frame_buffer[j].method);
>>>>>>>>>>>>>> 133 break;
>>>>>>>>>>>>>> 134 } else if (si1.frame_buffer[j].location !=
>>>>>>>>>>>>>> si1.frame_buffer[j].location) { /*
>>>>>>>>>>>>>> jvmtiFrameInfo::location */
>>>>>>>>>>>>>> 135 snprintf(err_msg, sizeof(err_msg),
>>>>>>>>>>>>>> 136 "thread [%d]
>>>>>>>>>>>>>> frame_buffer[%d].location is different: stack_info_1 =
>>>>>>>>>>>>>> %ld, stack_info_2 = %ld",
>>>>>>>>>>>>>> 137 i, j,
>>>>>>>>>>>>>> si1.frame_buffer[j].location, si2.frame_buffer[j].location);
>>>>>>>>>>>>>> 138 break;
>>>>>>>>>>>>>> 139 }
>>>>>>>>>>>>>> 140 }
>>>>>>>>>>>>>> 141 }
>>>>>>>>>>>>>> 142 }
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Another step would be to create functions that implement
>>>>>>>>>>>>>> a body of each loop.
>>>>>>>>>>>>>> You can use the same techniques to simplify similar place
>>>>>>>>>>>>>> (L127-L138) in the libOneGetThreadListStackTraces.c.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> Serguei
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 7/3/20 15:55, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>> Hi Serguei,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I'm not an Oracle employee, so I cannot know real
>>>>>>>>>>>>>>> request(s) from your customers.
>>>>>>>>>>>>>>> However JDK-8201641 says Dynatrace has requested this
>>>>>>>>>>>>>>> enhancement.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> BTW I haven't heared any request from my customers about
>>>>>>>>>>>>>>> this.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 2020/07/04 4:32, serguei.spitsyn at oracle.com wrote:
>>>>>>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> This difference is not that big to care about.
>>>>>>>>>>>>>>>> I feel this is really rare case and so, does not worth
>>>>>>>>>>>>>>>> these complications.
>>>>>>>>>>>>>>>> Do we have a real request from customers to optimize it?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>> Serguei
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 7/3/20 01:16, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>> Hi Serguei,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Generally I agree with you, but I have concern about
>>>>>>>>>>>>>>>>> the difference of the result of GetStackTrace() and
>>>>>>>>>>>>>>>>> GetThreadListStackTraces().
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> GetStackTrace: jvmtiFrameInfo
>>>>>>>>>>>>>>>>> GetThreadListStackTraces: jvmtiStackInfo
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> jvmtiStackInfo contains thread state, and it is
>>>>>>>>>>>>>>>>> ensured it is the state of the call stack.
>>>>>>>>>>>>>>>>> If we want to get both call stack and thread state, we
>>>>>>>>>>>>>>>>> need to suspend target thread, and call both
>>>>>>>>>>>>>>>>> GetStackTrace() and GetThreadState(). Is it ok?
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I was wondering if JDK-8201641 (parent ticket of this
>>>>>>>>>>>>>>>>> change) needed them for profiling (dynatrace?)
>>>>>>>>>>>>>>>>> If it is responsibility of JVMTI agent implementor, I
>>>>>>>>>>>>>>>>> remove this closure.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On 2020/07/03 16:45, serguei.spitsyn at oracle.com wrote:
>>>>>>>>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> After some thinking I've concluded that I do not like
>>>>>>>>>>>>>>>>>> this optimization
>>>>>>>>>>>>>>>>>> of the GetThreadListStackTraces with
>>>>>>>>>>>>>>>>>> GetSingleStackTraceClosure.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> We may need more opinions on this but these are my
>>>>>>>>>>>>>>>>>> points:
>>>>>>>>>>>>>>>>>> - it adds some complexity and ugliness
>>>>>>>>>>>>>>>>>> - a win is doubtful because it has to be a rare
>>>>>>>>>>>>>>>>>> case, so that total overhead should not be high
>>>>>>>>>>>>>>>>>> - if it is really high for some use cases then it
>>>>>>>>>>>>>>>>>> is up to the user
>>>>>>>>>>>>>>>>>> to optimize it with using GetStackTrace instead
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> In such cases with doubtful overhead I usually prefer
>>>>>>>>>>>>>>>>>> the simplicity.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Good examples where it makes sense to optimize are
>>>>>>>>>>>>>>>>>> checks for target thread to be current thread.
>>>>>>>>>>>>>>>>>> In such cases there is no need to suspend the target
>>>>>>>>>>>>>>>>>> thread, or use a VMop/HandshakeClosure.
>>>>>>>>>>>>>>>>>> For instance, please, see the Monitor functions with
>>>>>>>>>>>>>>>>>> the check: (java_thread == calling_thread).
>>>>>>>>>>>>>>>>>> Getting information for current thread is frequently
>>>>>>>>>>>>>>>>>> used case, e.g. to get info at an event point.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>> Serguei
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> On 7/2/20 23:29, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>>>> Hi Dan, David,
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> I uploaded new webrev. Could you review again?
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.04/
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> OneGetThreadListStackTraces.java in this webrev
>>>>>>>>>>>>>>>>>>> would wait until thread state is transited to
>>>>>>>>>>>>>>>>>>> "waiting" with spin wait.
>>>>>>>>>>>>>>>>>>> CountDownLatch::await call as Dan pointed is fixed
>>>>>>>>>>>>>>>>>>> in it :)
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Diff from webrev.03:
>>>>>>>>>>>>>>>>>>> http://hg.openjdk.java.net/jdk/submit/rev/c9aeb7001e50
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> On 2020/07/03 14:15, David Holmes wrote:
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> On 3/07/2020 2:27 pm, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>>>>>> On 2020/07/03 12:24, 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.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> 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.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> If I add spin wait to wait until transit target
>>>>>>>>>>>>>>>>>>>>> thread state is WAITING (as following), we don't
>>>>>>>>>>>>>>>>>>>>> need to call SuspendThread().
>>>>>>>>>>>>>>>>>>>>> Which is better?
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> The original spin-wait loop checking for WAITING
>>>>>>>>>>>>>>>>>>>> is better because it is the only guarantee that the
>>>>>>>>>>>>>>>>>>>> target thread is blocked where you need it to be.
>>>>>>>>>>>>>>>>>>>> suspending the thread is racy as you don't know
>>>>>>>>>>>>>>>>>>>> exactly where the suspend will hit.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>>>>>>> -----
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> ```
>>>>>>>>>>>>>>>>>>>>> /* Wait until the thread state transits to
>>>>>>>>>>>>>>>>>>>>> "waiting" */
>>>>>>>>>>>>>>>>>>>>> while (th.getState() != Thread.State.WAITING) {
>>>>>>>>>>>>>>>>>>>>> Thread.onSpinWait();
>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>> ```
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> For simplify, spin wait is prefer to
>>>>>>>>>>>>>>>>>>>>> OneGetThreadListStackTraces.java in webrev.03.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> 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()
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> 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