RFR: 8242427: JVMTI frame pop operations should use Thread-Local Handshakes
Daniel D. Daugherty
daniel.daugherty at oracle.com
Fri Sep 4 20:27:49 UTC 2020
On 8/31/20 5:10 AM, Yasumasa Suenaga wrote:
> Hi David,
>
> I uploaded new webrev. Could you review again?
>
> http://cr.openjdk.java.net/~ysuenaga/JDK-8242427/webrev.04/
src/hotspot/share/prims/jvmtiEnvBase.hpp
No comments.
src/hotspot/share/prims/jvmtiEnv.cpp
L1636: JvmtiEnv::PopFrame(JavaThread* java_thread) {
L1719: MutexLocker mu(JvmtiThreadState_lock);
L1721: state->update_for_pop_top_frame();
Okay, this new JvmtiThreadState_lock grab replaces this old
code from jvmtiEventController.cpp (update_for_pop_top_frame()
calls clear_frame_pop()):
old L989:
JvmtiEventController::clear_frame_pop(JvmtiEnvThreadState *ets,
JvmtiFramePop fpop) {
old L990: MutexLocker
mu(SafepointSynchronize::is_at_safepoint() ? NULL : JvmtiThreadState_lock);
I'm good with that.
L1801: // thread. All other usage needs to use a vm-safepoint-op
for safety.
You forgot to update this comment for the new code:
// thread. All other usage needs to use a handshake for
safety.
L1771: JvmtiEnv::NotifyFramePop(JavaThread* java_thread, jint depth) {
L1802: MutexLocker mu(JvmtiThreadState_lock);
L1805: state->env_thread_state(this)->set_frame_pop(frame_number);
Okay, this new JvmtiThreadState_lock grab replaces this old
code from jvmtiEventController.cpp
old L982:
JvmtiEventController::set_frame_pop(JvmtiEnvThreadState *ets,
JvmtiFramePop fpop) {
old L983: MutexLocker
mu(SafepointSynchronize::is_at_safepoint() ? NULL : JvmtiThreadState_lock);
I'm good with that.
src/hotspot/share/prims/jvmtiEnvBase.cpp
old L1509: ThreadsListHandle tlh;
VM_UpdateForPopTopFrame::doit() held a ThreadsListHandle to keep
the target JavaThread safe.
UpdateForPopTopFrameClosure::do_thread()
doesn't have a ThreadsListHandle, but the closure is executed
during
a handshake so do_thread() is only called if the target thread is
in a handshake state, i.e., on a ThreadsList held by the handshake
dispatch code.
old L1520: ThreadsListHandle tlh;
VM_SetFramePop::doit() held a ThreadsListHandle to keep
the target JavaThread safe. SetFramePopClosure::do_thread()
doesn't have a ThreadsListHandle, but the closure is executed
during
a handshake so do_thread() is only called if the target thread is
in a handshake state, i.e., on a ThreadsList held by the handshake
dispatch code.
src/hotspot/share/prims/jvmtiEnvThreadState.cpp
L328 : if ((current == _thread) ||
(_thread->active_handshaker() == current)) {
nit - please remove the excess parens so switch to this:
if (current == _thread || _thread->active_handshaker()
== current) {
L331: bool executed = Handshake::execute_direct(&op, _thread);
The 'executed' variable might be considered unused by the compiler
in a non-ASSERT build, e.g., release build in Tier1 should
catch this.
L332: assert(executed, "Direct handshake failed. Target
thread is still alive?");
Perhaps:
... Target thread must be alive."
If I remember correctly, the target thread still has to be alive
because we're dealing with a single-step or breakpoint event and
it must be alive for those to be posted. It's been a long time
since I've been down those code paths so you might want to make
sure that's still true.
src/hotspot/share/prims/jvmtiEventController.cpp
L337: if ((target == current) || (target->active_handshaker() ==
current)) {
nit - please remove the excess parens so switch to this:
if (target == current || target->active_handshaker() ==
current) {
L340: bool executed = Handshake::execute_direct(&hs, target);
The 'executed' variable might be considered unused by the compiler
in a non-ASSERT build, e.g., release build in Tier1 should
catch this.
L341: assert(executed, "Direct handshake failed. Target thread
is still alive?");
Perhaps:
... Target thread must be alive."
If I remember correctly, the target thread still has to be alive
because we're forcing the target thread to switch to interpreted
mode. It's been a long time since I've been down those code paths
so you might want to make sure that's still true.
old L996:
JvmtiEventController::clear_to_frame_pop(JvmtiEnvThreadState *ets,
JvmtiFramePop fpop) {
old L997: MutexLocker mu(SafepointSynchronize::is_at_safepoint()
? NULL : JvmtiThreadState_lock);
This function JvmtiEventController::clear_to_frame_pop() is only
called by JvmtiEnvThreadState::clear_to_frame_pop(int
frame_number).
I cannot find any code that calls
JvmtiEnvThreadState::clear_to_frame_pop().
JvmtiEnvThreadState::set_frame_pop(int frame_number) is
called from jvmtiEnv.cpp and jvmtiEnvBase.cpp.
JvmtiEnvThreadState::clear_frame_pop(int frame_number) is
called from jvmtiExport.cpp and jvmtiThreadState.cpp.
It looks like JvmtiEnvThreadState::clear_to_frame_pop() is
unused to me, but another pair of eyes would be good. If it
is unused, a followup bug for analysis and removal would be
good. Just want to make sure that if the function was used
in the past, that the removal was intentional rather than a
typo. Easy to do with similarly named functions...
src/hotspot/share/prims/jvmtiExport.cpp
L1563: void JvmtiExport::post_method_exit()
L1649: MutexLocker mu(JvmtiThreadState_lock);
L1650: ets->clear_frame_pop(cur_frame_number);
Okay, this new JvmtiThreadState_lock grab replaces this old
code from jvmtiEventController.cpp:
old L989:
JvmtiEventController::clear_frame_pop(JvmtiEnvThreadState *ets,
JvmtiFramePop fpop) {
old L990: MutexLocker
mu(SafepointSynchronize::is_at_safepoint() ? NULL : JvmtiThreadState_lock);
src/hotspot/share/prims/jvmtiThreadState.cpp
L276: guarantee(current == get_thread() || current ==
get_thread()->active_handshaker(),
L277: "must be current thread or direct handshake");
nit - please indent L277 so the double-quote lines with with
the 'c' in current.
src/hotspot/share/runtime/handshake.hpp
No comments.
src/hotspot/share/runtime/handshake.cpp
No comments.
src/hotspot/share/runtime/thread.hpp
No comments.
src/hotspot/share/runtime/vmOperations.hpp
No comments.
I think there are only nits in my comments so I don't really
need to see another webrev.
>
> This webrev includes two changes:
>
> 1. Use assert_lock_strong() for JvmtiThreadState_lock
> http://hg.openjdk.java.net/jdk/submit/rev/c85f93d2042d
>
> 2. Check return value from execute_direct() with assert()
> http://hg.openjdk.java.net/jdk/submit/rev/8746e1651343
>
>
> Thanks,
>
> Yasumasa
>
>
> On 2020/08/31 15:22, Yasumasa Suenaga wrote:
>> Hi David,
>>
>> On 2020/08/31 14:43, David Holmes wrote:
>>> Hi Yasumasa,
>>>
>>> On 28/08/2020 1:01 pm, Yasumasa Suenaga wrote:
>>>> Hi David,
>>>>
>>>> On 2020/08/28 11:04, David Holmes wrote:
>>>>> Hi Yasumasa,
>>>>>
>>>>> On 28/08/2020 11:24 am, Yasumasa Suenaga wrote:
>>>>>> Hi David,
>>>>>>
>>>>>> On 2020/08/27 15:49, David Holmes wrote:
>>>>>>> Sorry I just realized I reviewed version 00 :(
>>>>>
>>>>> Note that my comments on version 00 in my earlier email still apply.
>>>>
>>>> I copied here your comment on webrev.00:
>>>>
>>>>>>>> I see. It is a pity that we have now lost that critical
>>>>>>>> indicator that shows how this operation can be nested within
>>>>>>>> another operation. The possibility of nesting is even more
>>>>>>>> obscure with JvmtiEnvThreadState::reset_current_location. And
>>>>>>>> the fact it is now up to the caller to handle that case
>>>>>>>> explicitly raises some concern - what will happen if you call
>>>>>>>> execute_direct whilst already in a handshake with the target
>>>>>>>> thread?
>>>>
>>>> I heard deadlock would be happen if execute_direct() calls in
>>>> direct handshake. Thus we need to use active_handshaker() in this
>>>> change.
>>>
>>> Okay. This is something we need to clarify with direct handshake
>>> usage information. I think it would be preferable if this was
>>> handled in execute_direct rather than the caller ... though it may
>>> also be the case that we need the writer of the handshake operation
>>> to give due consideration to nesting ...
>>
>> Agree, I also prefer to check whether caller is in direct handshake
>> in execute_direct().
>> But I think this is another enhancement because we need to change the
>> behavior of execute_direct().
>>
>>
>>>>>>>> Further comments:
>>>>>>>>
>>>>>>>> src/hotspot/share/prims/jvmtiEnvThreadState.cpp
>>>>>>>>
>>>>>>>> 194 #ifdef ASSERT
>>>>>>>> 195 Thread *current = Thread::current();
>>>>>>>> 196 #endif
>>>>>>>> 197 assert(get_thread() == current || current ==
>>>>>>>> get_thread()->active_handshaker(),
>>>>>>>> 198 "frame pop data only accessible from same thread
>>>>>>>> or direct handshake");
>>>>>>>>
>>>>>>>> Can you factor this out into a separate function so that it is
>>>>>>>> not repeated so often. Seems to me that there should be a
>>>>>>>> global function on Thread:
>>>>>>>> assert_current_thread_or_handshaker() [yes unpleasant name but
>>>>>>>> ...] that will allow us to stop repeating this code fragment
>>>>>>>> across numerous files. A follow up RFE for that would be okay
>>>>>>>> too (I see some guarantees that should probably just be asserts
>>>>>>>> so they need a bit more checking).
>>>>
>>>> I filed it as another RFE:
>>>> https://bugs.openjdk.java.net/browse/JDK-8252479
>>>
>>> Thanks.
>>>
>>>>
>>>>>>>> 331 Handshake::execute_direct(&op, _thread);
>>>>>>>>
>>>>>>>> You aren't checking the return value of execute_direct, but I
>>>>>>>> can't tell where _thread was checked for still being alive ??
>>>>>>>>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> src/hotspot/share/prims/jvmtiEventController.cpp
>>>>>>>>
>>>>>>>> 340 Handshake::execute_direct(&hs, target);
>>>>>>>>
>>>>>>>> I know this is existing code but I have the same query as above
>>>>>>>> - no return value check and no clear check that the JavaThread
>>>>>>>> is still alive?
>>>>
>>>> Existing code seems to assume that target thread is alive, frame
>>>> operations (e.g. PopFrame()) should be performed on live thread.
>>>> And also existing code would not set any JVMTI error and cannot
>>>> propagate it to caller. So I do not add the check for thread state.
>>>
>>> Okay. But note that for PopFrame the tests for isAlive and
>>> is-suspended have already been performed before we do the
>>> execute_direct; so in that case we could simply assert that
>>> execute_direct returns true. Similarly for other cases.
>>
>> Ok, I will change as following in next webrev:
>>
>> ```
>> bool result = Handshake::execute_direct(&hs, target);
>> guarantee(result, "Direct handshake failed. Target thread is still
>> alive?");
>> ```
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>>>>>>>> Do we know if the existing tests actually test the nested cases?
>>>>
>>>> I saw some error with assertion for JvmtiThreadState_lock and
>>>> safepoint in vmTestbase at first, so I guess nested call would be
>>>> tested, but I'm not sure.
>>>>
>>>>
>>>>>>> I have concerns with the added locking:
>>>>>>>
>>>>>>> MutexLocker mu(JvmtiThreadState_lock);
>>>>>>>
>>>>>>> Who else may be holding that lock? Could it be our target thread
>>>>>>> that we have already initiated a handshake with? (The lock
>>>>>>> ranking checks related to safepoints don't help us detect
>>>>>>> deadlocks between a target thread and its handshaker. :( )
>>>>>>
>>>>>> I checked source code again, then I couldn't find the point that
>>>>>> target thread already locked JvmtiThreadState_lock at direct
>>>>>> handshake.
>>>>>
>>>>> I'm very unclear exactly what state this lock guards and under
>>>>> what conditions. But looking at:
>>>>>
>>>>> src/hotspot/share/prims/jvmtiEnv.cpp
>>>>>
>>>>> Surely the lock is only needed in the direct-handshake case and
>>>>> not when operating on the current thread? Or is it there because
>>>>> you've removed the locking from the lower-level
>>>>> JvmtiEventController methods and so now you need to take the lock
>>>>> higher-up the call chain? (I find it hard to follow the call
>>>>> chains in the JVMTI code.)
>>>>
>>>> We need to take the lock higher-up the call chain. It is suggested
>>>> by Robbin, and works fine.
>>>
>>> Okay. It seems reasonably safe in this context as there is little
>>> additional work done while holding the lock.
>>>
>>>>
>>>>>>> It is far from clear now which functions are reachable from
>>>>>>> handshakes, which from safepoint VM_ops and which from both.
>>>>>>>
>>>>>>> ! assert(SafepointSynchronize::is_at_safepoint() ||
>>>>>>> JvmtiThreadState_lock->is_locked(), "Safepoint or must be locked");
>>>>>>>
>>>>>>> This can be written as:
>>>>>>>
>>>>>>> assert_locked_or_safepoint(JvmtiThreadState_lock);
>>>>>>>
>>>>>>> or possibly the weak variant of that. ('m puzzled by the extra
>>>>>>> check in the strong version ... I think it is intended for the
>>>>>>> case of the VMThread executing a non-safepoint VMop.)
>>>>>>
>>>>>>> JvmtiEventController::set_frame_pop(),
>>>>>>> JvmtiEventController::clear_frame_pop() and
>>>>>>> JvmtiEventController::clear_to_frame_pop() are no longer called
>>>>>>> at safepoint, so I remove safepoint check from assert() in new
>>>>>>> webrev.
>>>>>
>>>>> You should use assert_lock_strong for this.
>>>>
>>>> I will do that.
>>>
>>> Thanks,
>>> David
>>> -----
>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> webrev:
>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8242427/webrev.03/
>>>>>> diff from previous webrev:
>>>>>> http://hg.openjdk.java.net/jdk/submit/rev/2a2c02ada080
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>> -----
>>>>>>>
>>>>>>>
>>>>>>> On 27/08/2020 4:34 pm, David Holmes wrote:
>>>>>>>> Hi Yasumasa,
>>>>>>>>
>>>>>>>> On 27/08/2020 9:40 am, Yasumasa Suenaga wrote:
>>>>>>>>> Hi David,
>>>>>>>>>
>>>>>>>>> On 2020/08/27 8:09, David Holmes wrote:
>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>
>>>>>>>>>> On 26/08/2020 5:34 pm, Yasumasa Suenaga wrote:
>>>>>>>>>>> Hi Patricio, David,
>>>>>>>>>>>
>>>>>>>>>>> Thanks for your comment!
>>>>>>>>>>>
>>>>>>>>>>> I updated webrev which includes the fix which is commented
>>>>>>>>>>> by Patricio, and it passed submit repo. So I switch this
>>>>>>>>>>> mail thread to RFR.
>>>>>>>>>>>
>>>>>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8242427
>>>>>>>>>>> webrev:
>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8242427/webrev.00/
>>>>>>>>>>>
>>>>>>>>>>> I understand David said same concerns as Patricio about
>>>>>>>>>>> active handshaker. This webrev checks active handshaker is
>>>>>>>>>>> current thread or not.
>>>>>>>>>>
>>>>>>>>>> How can the current thread already be in a handshake with the
>>>>>>>>>> target when you execute this code?
>>>>>>>>>
>>>>>>>>> EnterInterpOnlyModeClosure might be called in handshake with
>>>>>>>>> UpdateForPopTopFrameClosure or with SetFramePopClosure.
>>>>>>>>>
>>>>>>>>> EnterInterpOnlyModeClosure is introduced in JDK-8238585 as an
>>>>>>>>> alternative in VM_EnterInterpOnlyMode.
>>>>>>>>> VM_EnterInterpOnlyMode returned true in
>>>>>>>>> allow_nested_vm_operations(). Originally, it could have been
>>>>>>>>> called from other VM operations.
>>>>>>>>
>>>>>>>> I see. It is a pity that we have now lost that critical
>>>>>>>> indicator that shows how this operation can be nested within
>>>>>>>> another operation. The possibility of nesting is even more
>>>>>>>> obscure with JvmtiEnvThreadState::reset_current_location. And
>>>>>>>> the fact it is now up to the caller to handle that case
>>>>>>>> explicitly raises some concern - what will happen if you call
>>>>>>>> execute_direct whilst already in a handshake with the target
>>>>>>>> thread?
>>>>>>>>
>>>>>>>> I can't help but feel that we need a more rigorous and
>>>>>>>> automated way of dealing with nesting ... perhaps we don't even
>>>>>>>> need to care and handshakes should always allow nested
>>>>>>>> handshake requests? (Question more for Robbin and Patricio.)
>>>>>>>>
>>>>>>>> Further comments:
>>>>>>>>
>>>>>>>> src/hotspot/share/prims/jvmtiEnvThreadState.cpp
>>>>>>>>
>>>>>>>> 194 #ifdef ASSERT
>>>>>>>> 195 Thread *current = Thread::current();
>>>>>>>> 196 #endif
>>>>>>>> 197 assert(get_thread() == current || current ==
>>>>>>>> get_thread()->active_handshaker(),
>>>>>>>> 198 "frame pop data only accessible from same thread
>>>>>>>> or direct handshake");
>>>>>>>>
>>>>>>>> Can you factor this out into a separate function so that it is
>>>>>>>> not repeated so often. Seems to me that there should be a
>>>>>>>> global function on Thread:
>>>>>>>> assert_current_thread_or_handshaker() [yes unpleasant name but
>>>>>>>> ...] that will allow us to stop repeating this code fragment
>>>>>>>> across numerous files. A follow up RFE for that would be okay
>>>>>>>> too (I see some guarantees that should probably just be asserts
>>>>>>>> so they need a bit more checking).
>>>>>>>>
>>>>>>>> 331 Handshake::execute_direct(&op, _thread);
>>>>>>>>
>>>>>>>> You aren't checking the return value of execute_direct, but I
>>>>>>>> can't tell where _thread was checked for still being alive ??
>>>>>>>>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> src/hotspot/share/prims/jvmtiEventController.cpp
>>>>>>>>
>>>>>>>> 340 Handshake::execute_direct(&hs, target);
>>>>>>>>
>>>>>>>> I know this is existing code but I have the same query as above
>>>>>>>> - no return value check and no clear check that the JavaThread
>>>>>>>> is still alive?
>>>>>>>>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> Do we know if the existing tests actually test the nested cases?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> David
>>>>>>>> -----
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> David
>>>>>>>>>> -----
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Cheers,
>>>>>>>>>>>
>>>>>>>>>>> Yasumasa
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 2020/08/26 10:13, Patricio Chilano wrote:
>>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>>
>>>>>>>>>>>> On 8/23/20 11:40 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>
>>>>>>>>>>>>> I want to hear your opinions about the change for
>>>>>>>>>>>>> JDK-8242427.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I'm trying to migrate following operations to direct
>>>>>>>>>>>>> handshake.
>>>>>>>>>>>>>
>>>>>>>>>>>>> - VM_UpdateForPopTopFrame
>>>>>>>>>>>>> - VM_SetFramePop
>>>>>>>>>>>>> - VM_GetCurrentLocation
>>>>>>>>>>>>>
>>>>>>>>>>>>> Some operations (VM_GetCurrentLocation and
>>>>>>>>>>>>> EnterInterpOnlyModeClosure) might be called at safepoint,
>>>>>>>>>>>>> so I want to use JavaThread::active_handshaker() in
>>>>>>>>>>>>> production VM to detect the process is in direct handshake
>>>>>>>>>>>>> or not.
>>>>>>>>>>>>>
>>>>>>>>>>>>> However this function is available in debug VM only, so I
>>>>>>>>>>>>> want to hear the reason why it is for debug VM only, and
>>>>>>>>>>>>> there are no problem to use it in production VM. Of course
>>>>>>>>>>>>> another solutions are welcome.
>>>>>>>>>>>> I added the _active_handshaker field to the HandshakeState
>>>>>>>>>>>> class when working on 8230594 to adjust some asserts, where
>>>>>>>>>>>> instead of checking for the VMThread we needed to check for
>>>>>>>>>>>> the active handshaker of the target JavaThread. Since there
>>>>>>>>>>>> were no other users of it, there was no point in declaring
>>>>>>>>>>>> it and having to write to it for the release bits. There
>>>>>>>>>>>> are no issues with having it in production though so you
>>>>>>>>>>>> could change that if necessary.
>>>>>>>>>>>>
>>>>>>>>>>>>> webrev is here. It passed jtreg tests
>>>>>>>>>>>>> (vmTestbase/nsk/{jdi,jdwp,jvmti} serviceability/{jdwp,jvmti})
>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8242427/proposal/
>>>>>>>>>>>> Some comments on the proposed change.
>>>>>>>>>>>>
>>>>>>>>>>>> src/hotspot/share/prims/jvmtiEnvThreadState.cpp,
>>>>>>>>>>>> src/hotspot/share/prims/jvmtiEventController.cpp
>>>>>>>>>>>> Why is the check to decide whether to call the handshake or
>>>>>>>>>>>> execute the operation with the current thread different for
>>>>>>>>>>>> GetCurrentLocationClosure vs EnterInterpOnlyModeClosure?
>>>>>>>>>>>>
>>>>>>>>>>>> (GetCurrentLocationClosure)
>>>>>>>>>>>> if ((Thread::current() == _thread) ||
>>>>>>>>>>>> (_thread->active_handshaker() != NULL)) {
>>>>>>>>>>>> op.do_thread(_thread);
>>>>>>>>>>>> } else {
>>>>>>>>>>>> Handshake::execute_direct(&op, _thread);
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>> vs
>>>>>>>>>>>>
>>>>>>>>>>>> (EnterInterpOnlyModeClosure)
>>>>>>>>>>>> if (target->active_handshaker() != NULL) {
>>>>>>>>>>>> hs.do_thread(target);
>>>>>>>>>>>> } else {
>>>>>>>>>>>> Handshake::execute_direct(&hs, target);
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>> If you change VM_SetFramePop to use handshakes then it
>>>>>>>>>>>> seems you could reach
>>>>>>>>>>>> JvmtiEventControllerPrivate::enter_interp_only_mode() with
>>>>>>>>>>>> the current thread being the target.
>>>>>>>>>>>> Also I think you want the second expression of that check
>>>>>>>>>>>> to be (target->active_handshaker() == Thread::current()).
>>>>>>>>>>>> So either you are the target or the current
>>>>>>>>>>>> active_handshaker for that target. Otherwise
>>>>>>>>>>>> active_handshaker() could be not NULL because there is
>>>>>>>>>>>> another JavaThread handshaking the same target. Unless you
>>>>>>>>>>>> are certain that it can never happen, so if
>>>>>>>>>>>> active_handshaker() is not NULL it is always the current
>>>>>>>>>>>> thread, but even in that case this way is safer.
>>>>>>>>>>>>
>>>>>>>>>>>> src/hotspot/share/prims/jvmtiThreadState.cpp
>>>>>>>>>>>> The guarantee() statement exists in release builds too so
>>>>>>>>>>>> the "#ifdef ASSERT" directive should be removed, otherwise
>>>>>>>>>>>> "current" will not be declared.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks!
>>>>>>>>>>>>
>>>>>>>>>>>> Patricio
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>
More information about the serviceability-dev
mailing list