RFR: 8242427: JVMTI frame pop operations should use Thread-Local Handshakes
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Fri Sep 4 10:29:18 UTC 2020
Hi Yasumasa,
On 9/4/20 02:49, Yasumasa Suenaga wrote:
> Hi Serguei,
>
> For example, JvmtiThreadState_lock would be grabbed at
> ciEnv::cache_jvmti_state() which is called by
> CompileBroker::invoke_compiler_on_method(). Compiler thread might be
> touch the protected resource even if target thread is suspended with
> direct handshake.
My statement was: "Grabbing the JvmtiThreadState_lock is good enough to
ensure MT-safety".
Compiler code just reads but does not change the thread state.
All modifications of the thread state are protected with the
JvmtiThreadState_lock.
In all cases when the state is changed by non-current thread the target
thread is suspended (please, let me know if it is a wrong statement).
So, it looks like these two handshake closures are not needed:
UpdateForPopTopFrameClosure and SetFramePopClosure.
However, it is hard to prove that it is safe to get rid of these
handshake closures.
Thanks,
Serguei
>
> So I think they are needed both direct handshake and
> JvmtiThreadState_lock.
>
>
> Thanks,
>
> Yasumasa
>
>
> On 2020/09/04 17:38, serguei.spitsyn at oracle.com wrote:
>> Hi Yasumasa,
>>
>> Only PopFrame, ForceEarlyReturn, NotifyFramePop and ResumeThread
>> require the target thread to be suspended.
>> Other JVMTI functions do not require it.
>>
>> I'm still thinking if it is a good idea to get rid of the
>> UpdateForPopTopFrameClosure and SetFramePopClosure hadnshakes.
>> It has to be safe because the target thread is suspended.
>> Grabbing the JvmtiThreadState_lock is good enough to ensure MT-safety.
>>
>> Thanks,
>> Serguei
>>
>>
>> On 9/4/20 01:28, Yasumasa Suenaga wrote:
>>> Hi Serguei,
>>>
>>> Thanks for your review!
>>>
>>> On 2020/09/04 15:43, serguei.spitsyn at oracle.com wrote:
>>>> Hi Yasumasa,
>>>>
>>>> Thanks for the answer and sorry for the latency. It is not easy to
>>>> double check everything related to the JvmtiThreadState_lock use.
>>>> You are right, it is not safe to remove the MutexLockers around the
>>>> direct handshakes (unfortunately, I'm still floating with the
>>>> handshake closures).
>>>>
>>>> I also had this question:
>>>> Q: Why do we need UpdateForPopTopFrameClosure and
>>>> SetFramePopClosure if we already protect data with the
>>>> JvmtiThreadState_lock?
>>>
>>> We need to suspend target thread (e.g. debugee) if we want to do
>>> frame operation. I think we cannot do frame operation safety without
>>> stopping target thread.
>>>
>>> So I chose direct handshake to avoid stopping the other threads.
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>>> However, I'd be even morenervous to get rid of these closures as it
>>>> is hard to predict all the consequences. The concern is the
>>>> JvmtiEnvThreadState functions like get_frame_pops, has_frame_pos
>>>> and is_frame_pop. They were designed to be called on the current
>>>> thread or in a VMops/Handshake. Changing the _frame_pops on other
>>>> threads without handshakes (even under protection of the
>>>> JvmtiThreadState_lock) will break this design as these functions
>>>> are not protected with the JvmtiThreadState_lock.
>>>>
>>>> So, I'm getting along with your sync approach now.
>>>> There is a similar sync pattern with the EnterInterpOnlyModeClosure
>>>> and functions enter_interp_only_mode/leave_interp_only_mode which
>>>> are called in the recompute_thread_enabled and recompute_enabled.
>>>> The JvmtiThreadState_lock is also grabbed around this handshake
>>>> closure.
>>>>
>>>> You fix moved MutexLockers from the JvmtiEventController functions
>>>> set_frame_pop and clear_frame_pop to all their callers which should
>>>> be fine in general. The function
>>>> JvmtiEventController::clear_to_frame_pop is used by the
>>>> JvmtiEnvThreadState::clear_to_frame_pop which in turn not used itself.
>>>>
>>>> So, I'm okay with the fix.
>>>> Thank you for your patience and answers!
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>
>>>> On 9/3/20 02:01, Yasumasa Suenaga wrote:
>>>>> Hi Serguei,
>>>>>
>>>>> Your suggestion would not grab lock if it performs in direct
>>>>> handshake, then other threads can enter these functions. Is it safe?
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>> On 2020/09/03 16:34, serguei.spitsyn at oracle.com wrote:
>>>>>> Hi Yasumasa,
>>>>>>
>>>>>> Wood it be MT-safe to do this to avoid locks at handshake time? :
>>>>>>
>>>>>> JvmtiEventController::set_frame_pop(JvmtiEnvThreadState *ets,
>>>>>> JvmtiFramePop fpop) {
>>>>>> - MutexLocker mu(SafepointSynchronize::is_at_safepoint() ? NULL
>>>>>> : JvmtiThreadState_lock);
>>>>>> + MutexLocker mu(Thread::current ==
>>>>>> ets->get_thread()->active_handshaker() ? NULL :
>>>>>> JvmtiThreadState_lock);
>>>>>> JvmtiEventControllerPrivate::set_frame_pop(ets, fpop);
>>>>>> }
>>>>>>
>>>>>>
>>>>>> void
>>>>>> JvmtiEventController::clear_frame_pop(JvmtiEnvThreadState *ets,
>>>>>> JvmtiFramePop fpop) {
>>>>>> - MutexLocker mu(SafepointSynchronize::is_at_safepoint() ? NULL
>>>>>> : JvmtiThreadState_lock);
>>>>>> + MutexLocker mu(Thread::current ==
>>>>>> ets->get_thread()->active_handshaker() ? NULL :
>>>>>> JvmtiThreadState_lock);
>>>>>> JvmtiEventControllerPrivate::clear_frame_pop(ets, fpop);
>>>>>> }
>>>>>>
>>>>>>
>>>>>> void
>>>>>> JvmtiEventController::clear_to_frame_pop(JvmtiEnvThreadState
>>>>>> *ets, JvmtiFramePop fpop) {
>>>>>> - MutexLocker mu(SafepointSynchronize::is_at_safepoint() ? NULL
>>>>>> : JvmtiThreadState_lock);
>>>>>> + MutexLocker mu(Thread::current ==
>>>>>> ets->get_thread()->active_handshaker() ? NULL :
>>>>>> JvmtiThreadState_lock);
>>>>>> JvmtiEventControllerPrivate::clear_to_frame_pop(ets, fpop);
>>>>>> }
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Serguei
>>>>>>
>>>>>>
>>>>>> On 9/2/20 01:42, serguei.spitsyn at oracle.com wrote:
>>>>>>> Hi Yasumasa,
>>>>>>>
>>>>>>> Thank you for the explanation. At least, I see the real motivation.
>>>>>>> I've overlooked this in the email thread.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Serguei
>>>>>>>
>>>>>>>
>>>>>>> On 9/2/20 00:00, Yasumasa Suenaga wrote:
>>>>>>>> Hi Serguei,
>>>>>>>>
>>>>>>>> On 2020/09/02 15:29, David Holmes wrote:
>>>>>>>>> Hi Serguei,
>>>>>>>>>
>>>>>>>>> On 2/09/2020 4:11 pm, serguei.spitsyn at oracle.com wrote:
>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>
>>>>>>>>>> It seems to me your update for sync with the
>>>>>>>>>> JvmtiThreadState_lock is incorrect.
>>>>>>>>>> Let me explain it.
>>>>>>>>>> The original design was that the functions is_frame_pop,
>>>>>>>>>> set_frame_pop, clear_frame_pop and clear_to_frame_pop are
>>>>>>>>>> always called either on the current thread or in a VMop.
>>>>>>>>>> There are 3 levels of these functions: in
>>>>>>>>>> JvmtiEnvThreadState, JvmtiEventController and
>>>>>>>>>> JvmtiEventControllerPrivate.
>>>>>>>>>> You already found the JvmtiThreadState_lock is grabbed in the
>>>>>>>>>> JvmtiEventController versions of these functions.
>>>>>>>>>> It is for MT-safety of the recompute_thread_enabled() which
>>>>>>>>>> can be called not only on current thread and VMop.
>>>>>>>>>
>>>>>>>>> Right, but now that we use a handshake, not a VMop, we have no
>>>>>>>>> safepoint to guarantee MT-safety and so we have to use the
>>>>>>>>> lock to ensure that.
>>>>>>>>
>>>>>>>> In addition we cannot take the MutexLocker inside a handshakes
>>>>>>>> because we might see deadlock with the handshake semaphore.
>>>>>>>> So we have to grab JvmtiThreadState_lock before execute_direct().
>>>>>>>>
>>>>>>>> https://mail.openjdk.java.net/pipermail/serviceability-dev/2020-August/032754.html
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>>> David
>>>>>>>>> -----
>>>>>>>>>
>>>>>>>>>> So, I think adding MutexLocker's to the jvmtiEnv.cpp and
>>>>>>>>>> jvmtiExport.cpp is not needed:
>>>>>>>>>>
>>>>>>>>>> + MutexLocker mu(JvmtiThreadState_lock);
>>>>>>>>>> + if (java_thread == JavaThread::current()) {
>>>>>>>>>> + state->update_for_pop_top_frame();
>>>>>>>>>> + } else {
>>>>>>>>>> + UpdateForPopTopFrameClosure op(state);
>>>>>>>>>>
>>>>>>>>>> . . .
>>>>>>>>>>
>>>>>>>>>> + MutexLocker mu(JvmtiThreadState_lock);
>>>>>>>>>> if (java_thread == JavaThread::current()) {
>>>>>>>>>> int frame_number = state->count_frames() - depth;
>>>>>>>>>> state->env_thread_state(this)->set_frame_pop(frame_number);
>>>>>>>>>> } else {
>>>>>>>>>> - VM_SetFramePop op(this, state, depth);
>>>>>>>>>> - VMThread::execute(&op);
>>>>>>>>>> - err = op.result();
>>>>>>>>>> + SetFramePopClosure op(this, state, depth);
>>>>>>>>>> + bool executed = Handshake::execute_direct(&op,
>>>>>>>>>> java_thread);
>>>>>>>>>> + err = executed ? op.result() :
>>>>>>>>>> JVMTI_ERROR_THREAD_NOT_ALIVE;
>>>>>>>>>>
>>>>>>>>>> . . .
>>>>>>>>>>
>>>>>>>>>> + MutexLocker mu(JvmtiThreadState_lock);
>>>>>>>>>> ets->clear_frame_pop(cur_frame_number);
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Instead, they have to be restored in the JvmtiEventController
>>>>>>>>>> functions:
>>>>>>>>>>
>>>>>>>>>> void
>>>>>>>>>> JvmtiEventController::set_frame_pop(JvmtiEnvThreadState *ets,
>>>>>>>>>> JvmtiFramePop fpop) {
>>>>>>>>>> - MutexLocker mu(SafepointSynchronize::is_at_safepoint() ?
>>>>>>>>>> NULL : JvmtiThreadState_lock);
>>>>>>>>>> + assert_lock_strong(JvmtiThreadState_lock);
>>>>>>>>>> JvmtiEventControllerPrivate::set_frame_pop(ets, fpop);
>>>>>>>>>> }
>>>>>>>>>> void
>>>>>>>>>> JvmtiEventController::clear_frame_pop(JvmtiEnvThreadState
>>>>>>>>>> *ets, JvmtiFramePop fpop) {
>>>>>>>>>> - MutexLocker mu(SafepointSynchronize::is_at_safepoint() ?
>>>>>>>>>> NULL : JvmtiThreadState_lock);
>>>>>>>>>> + assert_lock_strong(JvmtiThreadState_lock);
>>>>>>>>>> JvmtiEventControllerPrivate::clear_frame_pop(ets, fpop);
>>>>>>>>>> }
>>>>>>>>>> void
>>>>>>>>>> JvmtiEventController::clear_to_frame_pop(JvmtiEnvThreadState
>>>>>>>>>> *ets, JvmtiFramePop fpop) {
>>>>>>>>>> - MutexLocker mu(SafepointSynchronize::is_at_safepoint() ?
>>>>>>>>>> NULL : JvmtiThreadState_lock);
>>>>>>>>>> + assert_lock_strong(JvmtiThreadState_lock);
>>>>>>>>>> JvmtiEventControllerPrivate::clear_to_frame_pop(ets, fpop);
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Serguei
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 9/1/20 21:34, serguei.spitsyn at oracle.com wrote:
>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 9/1/20 21:17, Yasumasa Suenaga wrote:
>>>>>>>>>>>> Hi David,
>>>>>>>>>>>>
>>>>>>>>>>>> On 2020/09/02 13:13, David Holmes wrote:
>>>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 31/08/2020 7:10 pm, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>> Hi David,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I uploaded new webrev. Could you review again?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8242427/webrev.04/
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 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
>>>>>>>>>>>>>
>>>>>>>>>>>>> The message for the assertion:
>>>>>>>>>>>>>
>>>>>>>>>>>>> assert(executed, "Direct handshake failed. Target thread
>>>>>>>>>>>>> is still alive?");
>>>>>>>>>>>>>
>>>>>>>>>>>>> should be phrased:
>>>>>>>>>>>>>
>>>>>>>>>>>>> assert(executed, "Direct handshake failed. Target thread
>>>>>>>>>>>>> is not alive?");
>>>>>>>>>>>>>
>>>>>>>>>>>>> otherwise it sounds like the expectation is that it should
>>>>>>>>>>>>> not be alive.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Other changes fine.
>>>>>>>>>>>>>
>>>>>>>>>>>>> No need to see updated webrev.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks for your review!
>>>>>>>>>>>> I will fix them before pushing.
>>>>>>>>>>>
>>>>>>>>>>> Please, hold on.
>>>>>>>>>>> I'm still reviewing this.
>>>>>>>>>>> It is not clear yet if sync with the JvmtiThreadState_lock
>>>>>>>>>>> is fully correct.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Serguei
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> David
>>>>>>>>>>>>> -----
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 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