8238585: JvmtiEventControllerPrivate::enter_interp_only_mode() should not make compiled methods on stack not_entrant

Reviewed-by: mdoerr, kvn, sspitsyn

Author: reinrich

src/hotspot/share/prims/jvmtiEnvBase.hpp

@@ -367,9 +367,6 @@ class VM_SetFramePop : public VM_Operation {
     _depth = depth;
     _result = JVMTI_ERROR_NONE;
   }
-  // Nested operation must be allowed for the VM_EnterInterpOnlyMode that is
-  // called from the JvmtiEventControllerPrivate::recompute_thread_enabled.
-  bool allow_nested_vm_operations() const { return true; }
   VMOp_Type type() const { return VMOp_SetFramePop; }
   jvmtiError result() { return _result; }
   void doit();

src/hotspot/share/prims/jvmtiEventController.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2019, 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
@@ -33,7 +33,7 @@
 #include "prims/jvmtiImpl.hpp"
 #include "prims/jvmtiThreadState.inline.hpp"
 #include "runtime/deoptimization.hpp"
-#include "runtime/frame.hpp"
+#include "runtime/frame.inline.hpp"
 #include "runtime/thread.inline.hpp"
 #include "runtime/threadSMR.hpp"
 #include "runtime/vframe.hpp"
@@ -190,60 +190,38 @@ JvmtiEnvEventEnable::~JvmtiEnvEventEnable() {
 
 ///////////////////////////////////////////////////////////////
 //
-// VM_EnterInterpOnlyMode
+// EnterInterpOnlyModeClosure
 //
 
-class VM_EnterInterpOnlyMode : public VM_Operation {
-private:
-  JvmtiThreadState *_state;
+class EnterInterpOnlyModeClosure : public HandshakeClosure {
 
 public:
-  VM_EnterInterpOnlyMode(JvmtiThreadState *state);
-
-  bool allow_nested_vm_operations() const        { return true; }
-  VMOp_Type type() const { return VMOp_EnterInterpOnlyMode; }
-  void doit();
-
-  // to do: this same function is in jvmtiImpl - should be in one place
-  bool can_be_deoptimized(vframe* vf) {
-    return (vf->is_compiled_frame() && vf->fr().can_be_deoptimized());
-  }
-};
-
-VM_EnterInterpOnlyMode::VM_EnterInterpOnlyMode(JvmtiThreadState *state)
-  : _state(state)
-{
-}
-
-
-void VM_EnterInterpOnlyMode::doit() {
-  // Set up the current stack depth for later tracking
-  _state->invalidate_cur_stack_depth();
-
-  _state->enter_interp_only_mode();
-
-  JavaThread *thread = _state->get_thread();
-  if (thread->has_last_Java_frame()) {
-    // If running in fullspeed mode, single stepping is implemented
-    // as follows: first, the interpreter does not dispatch to
-    // compiled code for threads that have single stepping enabled;
-    // second, we deoptimize all methods on the thread's stack when
-    // interpreted-only mode is enabled the first time for a given
-    // thread (nothing to do if no Java frames yet).
-    int num_marked = 0;
-    ResourceMark resMark;
-    RegisterMap rm(thread, false);
-    for (vframe* vf = thread->last_java_vframe(&rm); vf; vf = vf->sender()) {
-      if (can_be_deoptimized(vf)) {
-        ((compiledVFrame*) vf)->code()->mark_for_deoptimization();
-        ++num_marked;
+  EnterInterpOnlyModeClosure() : HandshakeClosure("EnterInterpOnlyMode") { }
+  void do_thread(Thread* th) {
+    JavaThread* jt = (JavaThread*) th;
+    JvmtiThreadState* state = jt->jvmti_thread_state();
+
+    // Set up the current stack depth for later tracking
+    state->invalidate_cur_stack_depth();
+
+    state->enter_interp_only_mode();
+
+    if (jt->has_last_Java_frame()) {
+      // If running in fullspeed mode, single stepping is implemented
+      // as follows: first, the interpreter does not dispatch to
+      // compiled code for threads that have single stepping enabled;
+      // second, we deoptimize all compiled java frames on the thread's stack when
+      // interpreted-only mode is enabled the first time for a given
+      // thread (nothing to do if no Java frames yet).
+      ResourceMark resMark;
+      for (StackFrameStream fst(jt, false); !fst.is_done(); fst.next()) {
+        if (fst.current()->can_be_deoptimized()) {
+          Deoptimization::deoptimize(jt, *fst.current());
+        }
       }
     }
-    if (num_marked > 0) {
-      Deoptimization::deoptimize_all_marked();
-    }
   }
-}
+};
 
 
 ///////////////////////////////////////////////////////////////
@@ -352,9 +330,12 @@ void VM_ChangeSingleStep::doit() {
 void JvmtiEventControllerPrivate::enter_interp_only_mode(JvmtiThreadState *state) {
   EC_TRACE(("[%s] # Entering interpreter only mode",
             JvmtiTrace::safe_get_thread_name(state->get_thread())));
-
-  VM_EnterInterpOnlyMode op(state);
-  VMThread::execute(&op);
+  EnterInterpOnlyModeClosure hs;
+  if (SafepointSynchronize::is_at_safepoint()) {
+    hs.do_thread(state->get_thread());
+  } else {
+    Handshake::execute_direct(&hs, state->get_thread());
+  }
 }
 
 

src/hotspot/share/prims/jvmtiThreadState.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2019, 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
@@ -236,9 +236,10 @@ int JvmtiThreadState::count_frames() {
 
 
 void JvmtiThreadState::invalidate_cur_stack_depth() {
-  guarantee(SafepointSynchronize::is_at_safepoint() ||
-    (JavaThread *)Thread::current() == get_thread(),
-    "must be current thread or at safepoint");
+  assert(SafepointSynchronize::is_at_safepoint() ||
+         (JavaThread *)Thread::current() == get_thread() ||
+         Thread::current() == get_thread()->active_handshaker(),
+         "bad synchronization with owner thread");
 
   _cur_stack_depth = UNKNOWN_STACK_DEPTH;
 }

src/hotspot/share/runtime/thread.hpp

@@ -2032,9 +2032,11 @@ class JavaThread: public Thread {
 
   // Used by the interpreter in fullspeed mode for frame pop, method
   // entry, method exit and single stepping support. This field is
-  // only set to non-zero by the VM_EnterInterpOnlyMode VM operation.
-  // It can be set to zero asynchronously (i.e., without a VM operation
-  // or a lock) so we have to be very careful.
+  // only set to non-zero at a safepoint or using a direct handshake
+  // (see EnterInterpOnlyModeClosure).
+  // It can be set to zero asynchronously to this threads execution (i.e., without
+  // safepoint/handshake or a lock) so we have to be very careful.
+  // Accesses by other threads are synchronized using JvmtiThreadState_lock though.
   int               _interp_only_mode;
 
  public:

src/hotspot/share/runtime/vmOperations.hpp

@@ -90,7 +90,6 @@
   template(ChangeBreakpoints)                     \
   template(GetOrSetLocal)                         \
   template(GetCurrentLocation)                    \
-  template(EnterInterpOnlyMode)                   \
   template(ChangeSingleStep)                      \
   template(HeapWalkOperation)                     \
   template(HeapIterateOperation)                  \