8248362: JVMTI frame operations should use Thread-Local Handshake

Reviewed-by: sspitsyn, dholmes, dcubed

Author: YaSuenag

src/hotspot/share/prims/jvmtiEnv.cpp

@@ -1618,14 +1618,14 @@ JvmtiEnv::GetFrameCount(JavaThread* java_thread, jint* count_ptr) {
   }
 
   // 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 direct handshake for safety.
   if (java_thread == JavaThread::current()) {
     err = get_frame_count(state, count_ptr);
   } else {
-    // get java stack frame count at safepoint.
-    VM_GetFrameCount op(this, state, count_ptr);
-    VMThread::execute(&op);
-    err = op.result();
+    // get java stack frame count with handshake.
+    GetFrameCountClosure op(this, state, count_ptr);
+    bool executed = Handshake::execute_direct(&op, java_thread);
+    err = executed ? op.result() : JVMTI_ERROR_THREAD_NOT_ALIVE;
   }
   return err;
 } /* end GetFrameCount */
@@ -1748,14 +1748,14 @@ JvmtiEnv::GetFrameLocation(JavaThread* java_thread, jint depth, jmethodID* metho
   jvmtiError err = JVMTI_ERROR_NONE;
 
   // 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 direct handshake for safety.
   if (java_thread == JavaThread::current()) {
     err = get_frame_location(java_thread, depth, method_ptr, location_ptr);
   } else {
-    // JVMTI get java stack frame location at safepoint.
-    VM_GetFrameLocation op(this, java_thread, depth, method_ptr, location_ptr);
-    VMThread::execute(&op);
-    err = op.result();
+    // JVMTI get java stack frame location via direct handshake.
+    GetFrameLocationClosure op(this, depth, method_ptr, location_ptr);
+    bool executed = Handshake::execute_direct(&op, java_thread);
+    err = executed ? op.result() : JVMTI_ERROR_THREAD_NOT_ALIVE;
   }
   return err;
 } /* end GetFrameLocation */

src/hotspot/share/prims/jvmtiEnvBase.cpp

@@ -900,10 +900,10 @@ JvmtiEnvBase::get_frame_location(JavaThread *java_thread, jint depth,
 #ifdef ASSERT
   uint32_t debug_bits = 0;
 #endif
-  assert((SafepointSynchronize::is_at_safepoint() ||
-          java_thread->is_thread_fully_suspended(false, &debug_bits)),
-         "at safepoint or target thread is suspended");
   Thread* current_thread = Thread::current();
+  assert(current_thread == java_thread ||
+         current_thread == java_thread->active_handshaker(),
+         "call by myself or at direct handshake");
   ResourceMark rm(current_thread);
 
   vframe *vf = vframeFor(java_thread, depth);
@@ -1558,22 +1558,20 @@ GetStackTraceClosure::do_thread(Thread *target) {
 }
 
 void
-VM_GetFrameCount::doit() {
-  _result = JVMTI_ERROR_THREAD_NOT_ALIVE;
+GetFrameCountClosure::do_thread(Thread *target) {
   JavaThread* jt = _state->get_thread();
-  ThreadsListHandle tlh;
-  if (jt != NULL && tlh.includes(jt) && !jt->is_exiting() && jt->threadObj() != NULL) {
+  assert(target == jt, "just checking");
+  if (!jt->is_exiting() && jt->threadObj() != NULL) {
     _result = ((JvmtiEnvBase*)_env)->get_frame_count(_state, _count_ptr);
   }
 }
 
 void
-VM_GetFrameLocation::doit() {
-  _result = JVMTI_ERROR_THREAD_NOT_ALIVE;
-  ThreadsListHandle tlh;
-  if (_java_thread != NULL && tlh.includes(_java_thread)
-      && !_java_thread->is_exiting() && _java_thread->threadObj() != NULL) {
-    _result = ((JvmtiEnvBase*)_env)->get_frame_location(_java_thread, _depth,
+GetFrameLocationClosure::do_thread(Thread *target) {
+  assert(target->is_Java_thread(), "just checking");
+  JavaThread *jt = (JavaThread *)target;
+  if (!jt->is_exiting() && jt->threadObj() != NULL) {
+    _result = ((JvmtiEnvBase*)_env)->get_frame_location(jt, _depth,
                                                         _method_ptr, _location_ptr);
   }
 }

src/hotspot/share/prims/jvmtiEnvBase.hpp

@@ -557,47 +557,47 @@ class GetSingleStackTraceClosure : public HandshakeClosure {
   jvmtiError result()             { return _collector.result(); }
 };
 
-// VM operation to count stack frames at safepoint.
-class VM_GetFrameCount : public VM_Operation {
+// HandshakeClosure to count stack frames.
+class GetFrameCountClosure : public HandshakeClosure {
 private:
   JvmtiEnv *_env;
   JvmtiThreadState *_state;
   jint *_count_ptr;
   jvmtiError _result;
 
 public:
-  VM_GetFrameCount(JvmtiEnv *env, JvmtiThreadState *state, jint *count_ptr) {
-    _env = env;
-    _state = state;
-    _count_ptr = count_ptr;
+  GetFrameCountClosure(JvmtiEnv *env, JvmtiThreadState *state, jint *count_ptr)
+    : HandshakeClosure("GetFrameCount"),
+      _env(env),
+      _state(state),
+      _count_ptr(count_ptr),
+      _result(JVMTI_ERROR_THREAD_NOT_ALIVE) {
   }
-  VMOp_Type type() const { return VMOp_GetFrameCount; }
   jvmtiError result()    { return _result; }
-  void doit();
+  void do_thread(Thread *target);
 };
 
-// VM operation to frame location at safepoint.
-class VM_GetFrameLocation : public VM_Operation {
+// HandshakeClosure to get frame location.
+class GetFrameLocationClosure : public HandshakeClosure {
 private:
   JvmtiEnv *_env;
-  JavaThread* _java_thread;
   jint _depth;
   jmethodID* _method_ptr;
   jlocation* _location_ptr;
   jvmtiError _result;
 
 public:
-  VM_GetFrameLocation(JvmtiEnv *env, JavaThread* java_thread, jint depth,
-                      jmethodID* method_ptr, jlocation* location_ptr) {
-    _env = env;
-    _java_thread = java_thread;
-    _depth = depth;
-    _method_ptr = method_ptr;
-    _location_ptr = location_ptr;
+  GetFrameLocationClosure(JvmtiEnv *env, jint depth,
+                          jmethodID* method_ptr, jlocation* location_ptr)
+    : HandshakeClosure("GetFrameLocation"),
+      _env(env),
+      _depth(depth),
+      _method_ptr(method_ptr),
+      _location_ptr(location_ptr),
+      _result(JVMTI_ERROR_THREAD_NOT_ALIVE) {
   }
-  VMOp_Type type() const { return VMOp_GetFrameLocation; }
   jvmtiError result()    { return _result; }
-  void doit();
+  void do_thread(Thread *target);
 };
 
 

src/hotspot/share/prims/jvmtiThreadState.cpp

@@ -216,9 +216,13 @@ void JvmtiThreadState::leave_interp_only_mode() {
 
 // Helper routine used in several places
 int JvmtiThreadState::count_frames() {
-  guarantee(SafepointSynchronize::is_at_safepoint() ||
-    (JavaThread *)Thread::current() == get_thread(),
-    "must be current thread or at safepoint");
+#ifdef ASSERT
+  Thread *current_thread = Thread::current();
+#endif
+  assert(current_thread == get_thread() ||
+         SafepointSynchronize::is_at_safepoint() ||
+         current_thread == get_thread()->active_handshaker(),
+         "call by myself / at safepoint / at handshake");
 
   if (!get_thread()->has_last_Java_frame()) return 0;  // no Java frames
 

src/hotspot/share/runtime/vmOperations.hpp

@@ -81,8 +81,6 @@
   template(GetObjectMonitorUsage)                 \
   template(GetAllStackTraces)                     \
   template(GetThreadListStackTraces)              \
-  template(GetFrameCount)                         \
-  template(GetFrameLocation)                      \
   template(ChangeBreakpoints)                     \
   template(GetOrSetLocal)                         \
   template(GetCurrentLocation)                    \