--- old/src/hotspot/share/prims/jvmtiEnv.cpp 2020-08-26 15:59:39.113080331 +0900 +++ new/src/hotspot/share/prims/jvmtiEnv.cpp 2020-08-26 15:59:38.969074801 +0900 @@ -1714,13 +1714,13 @@ // shall be posted for this PopFrame. // It is only safe to perform the direct operation on the current - // thread. All other usage needs to use a vm-safepoint-op for safety. + // thread. All other usage needs to use a handshake for safety. if (java_thread == JavaThread::current()) { state->update_for_pop_top_frame(); } else { - VM_UpdateForPopTopFrame op(state); - VMThread::execute(&op); - jvmtiError err = op.result(); + UpdateForPopTopFrameClosure op(state); + bool executed = Handshake::execute_direct(&op, java_thread); + jvmtiError err = executed ? op.result() : JVMTI_ERROR_THREAD_NOT_ALIVE; if (err != JVMTI_ERROR_NONE) { return err; } @@ -1800,9 +1800,9 @@ 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; } return err; } /* end NotifyFramePop */ --- old/src/hotspot/share/prims/jvmtiEnvBase.cpp 2020-08-26 15:59:39.418092043 +0900 +++ new/src/hotspot/share/prims/jvmtiEnvBase.cpp 2020-08-26 15:59:39.342089125 +0900 @@ -1504,25 +1504,23 @@ } void -VM_UpdateForPopTopFrame::doit() { +UpdateForPopTopFrameClosure::do_thread(Thread *target) { JavaThread* jt = _state->get_thread(); - ThreadsListHandle tlh; - if (jt != NULL && tlh.includes(jt) && !jt->is_exiting() && jt->threadObj() != NULL) { + assert(jt == target, "just checking"); + if (!jt->is_exiting() && jt->threadObj() != NULL) { _state->update_for_pop_top_frame(); - } else { - _result = JVMTI_ERROR_THREAD_NOT_ALIVE; + _result = JVMTI_ERROR_NONE; } } void -VM_SetFramePop::doit() { +SetFramePopClosure::do_thread(Thread *target) { JavaThread* jt = _state->get_thread(); - ThreadsListHandle tlh; - if (jt != NULL && tlh.includes(jt) && !jt->is_exiting() && jt->threadObj() != NULL) { + assert(jt == target, "just checking"); + if (!jt->is_exiting() && jt->threadObj() != NULL) { int frame_number = _state->count_frames() - _depth; _state->env_thread_state((JvmtiEnvBase*)_env)->set_frame_pop(frame_number); - } else { - _result = JVMTI_ERROR_THREAD_NOT_ALIVE; + _result = JVMTI_ERROR_NONE; } } --- old/src/hotspot/share/prims/jvmtiEnvBase.hpp 2020-08-26 15:59:39.708103180 +0900 +++ new/src/hotspot/share/prims/jvmtiEnvBase.hpp 2020-08-26 15:59:39.631100223 +0900 @@ -336,24 +336,23 @@ JvmtiEnv* next(JvmtiEnvBase* env) { return env->next_environment(); } }; -// VM operation to update for pop top frame. -class VM_UpdateForPopTopFrame : public VM_Operation { +// HandshakeClosure to update for pop top frame. +class UpdateForPopTopFrameClosure : public HandshakeClosure { private: JvmtiThreadState* _state; jvmtiError _result; public: - VM_UpdateForPopTopFrame(JvmtiThreadState* state) { - _state = state; - _result = JVMTI_ERROR_NONE; - } - VMOp_Type type() const { return VMOp_UpdateForPopTopFrame; } + UpdateForPopTopFrameClosure(JvmtiThreadState* state) + : HandshakeClosure("UpdateForPopTopFrame"), + _state(state), + _result(JVMTI_ERROR_THREAD_NOT_ALIVE) {} jvmtiError result() { return _result; } - void doit(); + void do_thread(Thread *target); }; -// VM operation to set frame pop. -class VM_SetFramePop : public VM_Operation { +// HandshakeClosure to set frame pop. +class SetFramePopClosure : public HandshakeClosure { private: JvmtiEnv *_env; JvmtiThreadState* _state; @@ -361,15 +360,14 @@ jvmtiError _result; public: - VM_SetFramePop(JvmtiEnv *env, JvmtiThreadState* state, jint depth) { - _env = env; - _state = state; - _depth = depth; - _result = JVMTI_ERROR_NONE; - } - VMOp_Type type() const { return VMOp_SetFramePop; } + SetFramePopClosure(JvmtiEnv *env, JvmtiThreadState* state, jint depth) + : HandshakeClosure("SetFramePop"), + _env(env), + _state(state), + _depth(depth), + _result(JVMTI_ERROR_THREAD_NOT_ALIVE) {} jvmtiError result() { return _result; } - void doit(); + void do_thread(Thread *target); }; --- old/src/hotspot/share/prims/jvmtiEnvThreadState.cpp 2020-08-26 15:59:39.992114086 +0900 +++ new/src/hotspot/share/prims/jvmtiEnvThreadState.cpp 2020-08-26 15:59:39.916111168 +0900 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -191,8 +191,11 @@ JvmtiFramePops* JvmtiEnvThreadState::get_frame_pops() { - assert(get_thread() == Thread::current() || SafepointSynchronize::is_at_safepoint(), - "frame pop data only accessible from same thread or at safepoint"); +#ifdef ASSERT + Thread *current = Thread::current(); +#endif + assert(get_thread() == current || current == get_thread()->active_handshaker(), + "frame pop data only accessible from same thread or direct handshake"); if (_frame_pops == NULL) { _frame_pops = new JvmtiFramePops(); assert(_frame_pops != NULL, "_frame_pops != NULL"); @@ -206,32 +209,44 @@ } void JvmtiEnvThreadState::set_frame_pop(int frame_number) { - assert(get_thread() == Thread::current() || SafepointSynchronize::is_at_safepoint(), - "frame pop data only accessible from same thread or at safepoint"); +#ifdef ASSERT + Thread *current = Thread::current(); +#endif + assert(get_thread() == current || current == get_thread()->active_handshaker(), + "frame pop data only accessible from same thread or direct handshake"); JvmtiFramePop fpop(frame_number); JvmtiEventController::set_frame_pop(this, fpop); } void JvmtiEnvThreadState::clear_frame_pop(int frame_number) { - assert(get_thread() == Thread::current() || SafepointSynchronize::is_at_safepoint(), - "frame pop data only accessible from same thread or at safepoint"); +#ifdef ASSERT + Thread *current = Thread::current(); +#endif + assert(get_thread() == current || current == get_thread()->active_handshaker(), + "frame pop data only accessible from same thread or direct handshake"); JvmtiFramePop fpop(frame_number); JvmtiEventController::clear_frame_pop(this, fpop); } void JvmtiEnvThreadState::clear_to_frame_pop(int frame_number) { - assert(get_thread() == Thread::current() || SafepointSynchronize::is_at_safepoint(), - "frame pop data only accessible from same thread or at safepoint"); +#ifdef ASSERT + Thread *current = Thread::current(); +#endif + assert(get_thread() == current || current == get_thread()->active_handshaker(), + "frame pop data only accessible from same thread or direct handshake"); JvmtiFramePop fpop(frame_number); JvmtiEventController::clear_to_frame_pop(this, fpop); } bool JvmtiEnvThreadState::is_frame_pop(int cur_frame_number) { - assert(get_thread() == Thread::current() || SafepointSynchronize::is_at_safepoint(), - "frame pop data only accessible from same thread or at safepoint"); +#ifdef ASSERT + Thread *current = Thread::current(); +#endif + assert(get_thread() == current || current == get_thread()->active_handshaker(), + "frame pop data only accessible from same thread or direct handshake"); if (!get_thread()->is_interp_only_mode() || _frame_pops == NULL) { return false; } @@ -240,25 +255,25 @@ } -class VM_GetCurrentLocation : public VM_Operation { +class GetCurrentLocationClosure : public HandshakeClosure { private: - JavaThread *_thread; jmethodID _method_id; int _bci; public: - VM_GetCurrentLocation(JavaThread *thread) { - _thread = thread; - } - VMOp_Type type() const { return VMOp_GetCurrentLocation; } - void doit() { - ResourceMark rmark; // _thread != Thread::current() - RegisterMap rm(_thread, false); + GetCurrentLocationClosure() + : HandshakeClosure("GetCurrentLocation"), + _method_id(NULL), + _bci(0) {} + void do_thread(Thread *target) { + JavaThread *jt = (JavaThread *)target; + ResourceMark rmark; // jt != Thread::current() + RegisterMap rm(jt, false); // There can be a race condition between a VM_Operation reaching a safepoint // and the target thread exiting from Java execution. // We must recheck the last Java frame still exists. - if (!_thread->is_exiting() && _thread->has_last_Java_frame()) { - javaVFrame* vf = _thread->last_java_vframe(&rm); + if (!jt->is_exiting() && jt->has_last_Java_frame()) { + javaVFrame* vf = jt->last_java_vframe(&rm); assert(vf != NULL, "must have last java frame"); Method* method = vf->method(); _method_id = method->jmethod_id(); @@ -307,9 +322,14 @@ jmethodID method_id; int bci; // The java thread stack may not be walkable for a running thread - // so get current location at safepoint. - VM_GetCurrentLocation op(_thread); - VMThread::execute(&op); + // so get current location with direct handshake. + GetCurrentLocationClosure op; + Thread *current = Thread::current(); + if ((current == _thread) || (_thread->active_handshaker() == current)) { + op.do_thread(_thread); + } else { + Handshake::execute_direct(&op, _thread); + } op.get_current_location(&method_id, &bci); set_current_location(method_id, bci); } --- old/src/hotspot/share/prims/jvmtiEventController.cpp 2020-08-26 15:59:40.273124877 +0900 +++ new/src/hotspot/share/prims/jvmtiEventController.cpp 2020-08-26 15:59:40.198121997 +0900 @@ -331,10 +331,13 @@ EC_TRACE(("[%s] # Entering interpreter only mode", JvmtiTrace::safe_get_thread_name(state->get_thread()))); EnterInterpOnlyModeClosure hs; - if (SafepointSynchronize::is_at_safepoint()) { - hs.do_thread(state->get_thread()); + assert(state->get_thread()->is_Java_thread(), "just checking"); + JavaThread *target = (JavaThread *)state->get_thread(); + Thread *current = Thread::current(); + if ((target == current) || (target->active_handshaker() == current)) { + hs.do_thread(target); } else { - Handshake::execute_direct(&hs, state->get_thread()); + Handshake::execute_direct(&hs, target); } } @@ -980,21 +983,21 @@ void JvmtiEventController::set_frame_pop(JvmtiEnvThreadState *ets, JvmtiFramePop fpop) { - MutexLocker mu(SafepointSynchronize::is_at_safepoint() ? NULL : JvmtiThreadState_lock); + MutexLocker mu(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(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(JvmtiThreadState_lock); JvmtiEventControllerPrivate::clear_to_frame_pop(ets, fpop); } --- old/src/hotspot/share/prims/jvmtiThreadState.cpp 2020-08-26 15:59:40.560135898 +0900 +++ new/src/hotspot/share/prims/jvmtiThreadState.cpp 2020-08-26 15:59:40.484132980 +0900 @@ -272,9 +272,9 @@ } int JvmtiThreadState::cur_stack_depth() { - guarantee(SafepointSynchronize::is_at_safepoint() || - (JavaThread *)Thread::current() == get_thread(), - "must be current thread or at safepoint"); + Thread *current = Thread::current(); + guarantee(current == get_thread() || current == get_thread()->active_handshaker(), + "must be current thread or direct handshake"); if (!is_interp_only_mode() || _cur_stack_depth == UNKNOWN_STACK_DEPTH) { _cur_stack_depth = count_frames(); --- old/src/hotspot/share/runtime/handshake.cpp 2020-08-26 15:59:40.844146804 +0900 +++ new/src/hotspot/share/runtime/handshake.cpp 2020-08-26 15:59:40.767143848 +0900 @@ -383,9 +383,9 @@ _operation_direct(NULL), _handshake_turn_sem(1), _processing_sem(1), - _thread_in_process_handshake(false) + _thread_in_process_handshake(false), + _active_handshaker(NULL) { - DEBUG_ONLY(_active_handshaker = NULL;) } void HandshakeState::set_operation(HandshakeOperation* op) { @@ -513,9 +513,9 @@ if (can_process_handshake()) { guarantee(!_processing_sem.trywait(), "we should already own the semaphore"); log_trace(handshake)("Processing handshake by %s", Thread::current()->is_VM_thread() ? "VMThread" : "Handshaker"); - DEBUG_ONLY(_active_handshaker = Thread::current();) + _active_handshaker = Thread::current(); op->do_handshake(_handshakee); - DEBUG_ONLY(_active_handshaker = NULL;) + _active_handshaker = NULL; // Disarm after we have executed the operation. clear_handshake(is_direct); pr = _success; --- old/src/hotspot/share/runtime/handshake.hpp 2020-08-26 15:59:41.125157595 +0900 +++ new/src/hotspot/share/runtime/handshake.hpp 2020-08-26 15:59:41.050154715 +0900 @@ -106,11 +106,8 @@ }; ProcessResult try_process(HandshakeOperation* op); -#ifdef ASSERT Thread* _active_handshaker; Thread* active_handshaker() const { return _active_handshaker; } -#endif - }; #endif // SHARE_RUNTIME_HANDSHAKE_HPP --- old/src/hotspot/share/runtime/thread.hpp 2020-08-26 15:59:41.406168386 +0900 +++ new/src/hotspot/share/runtime/thread.hpp 2020-08-26 15:59:41.330165468 +0900 @@ -1365,11 +1365,9 @@ return _handshake.try_process(op); } -#ifdef ASSERT Thread* active_handshaker() const { return _handshake.active_handshaker(); } -#endif // Suspend/resume support for JavaThread private: --- old/src/hotspot/share/runtime/vmOperations.hpp 2020-08-26 15:59:41.699179638 +0900 +++ new/src/hotspot/share/runtime/vmOperations.hpp 2020-08-26 15:59:41.624176758 +0900 @@ -76,14 +76,11 @@ template(PopulateDumpSharedSpace) \ template(JNIFunctionTableCopier) \ template(RedefineClasses) \ - template(UpdateForPopTopFrame) \ - template(SetFramePop) \ template(GetObjectMonitorUsage) \ template(GetAllStackTraces) \ template(GetThreadListStackTraces) \ template(ChangeBreakpoints) \ template(GetOrSetLocal) \ - template(GetCurrentLocation) \ template(ChangeSingleStep) \ template(HeapWalkOperation) \ template(HeapIterateOperation) \