# HG changeset patch
# User sspitsyn
# Date 1393689955 28800
# Node ID e87156376bed6e630b3703475d75019f7bc12431
# Parent  c9ebeb289d1c3aee9132d28e72031d81243294de
6471769: Error: assert(_cur_stack_depth == count_frames(),"cur_stack_depth out of sync")
Summary: It is more safe to get/update data for suspended threads at a safepoint
Reviewed-by: dcubed, twisti, dholmes
Contributed-by: serguei.spitsyn@oracle.com

diff -r c9ebeb289d1c -r e87156376bed hotspot/src/share/vm/prims/jvmtiEnv.cpp
--- a/hotspot/src/share/vm/prims/jvmtiEnv.cpp	Sat Mar 01 01:36:48 2014 -0800
+++ b/hotspot/src/share/vm/prims/jvmtiEnv.cpp	Sat Mar 01 08:05:55 2014 -0800
@@ -1464,7 +1464,19 @@
     // It's fine to update the thread state here because no JVMTI events
     // shall be posted for this PopFrame.
 
-    state->update_for_pop_top_frame();
+    // 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.
+    if (java_thread == JavaThread::current()) {
+      state->update_for_pop_top_frame();
+    } else {
+      VM_UpdateForPopTopFrame op(state);
+      VMThread::execute(&op);
+      jvmtiError err = op.result();
+      if (err != JVMTI_ERROR_NONE) {
+        return err;
+      }
+    }
+
     java_thread->set_popframe_condition(JavaThread::popframe_pending_bit);
     // Set pending step flag for this popframe and it is cleared when next
     // step event is posted.
@@ -1505,6 +1517,7 @@
 // depth - pre-checked as non-negative
 jvmtiError
 JvmtiEnv::NotifyFramePop(JavaThread* java_thread, jint depth) {
+  jvmtiError err = JVMTI_ERROR_NONE;
   ResourceMark rm;
   uint32_t debug_bits = 0;
 
@@ -1532,10 +1545,17 @@
 
   assert(vf->frame_pointer() != NULL, "frame pointer mustn't be NULL");
 
-  int frame_number = state->count_frames() - depth;
-  state->env_thread_state(this)->set_frame_pop(frame_number);
-
-  return 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.
+  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();
+  }
+  return err;
 } /* end NotifyFramePop */
 
 
diff -r c9ebeb289d1c -r e87156376bed hotspot/src/share/vm/prims/jvmtiEnvBase.hpp
--- a/hotspot/src/share/vm/prims/jvmtiEnvBase.hpp	Sat Mar 01 01:36:48 2014 -0800
+++ b/hotspot/src/share/vm/prims/jvmtiEnvBase.hpp	Sat Mar 01 08:05:55 2014 -0800
@@ -29,6 +29,7 @@
 #include "prims/jvmtiEnvThreadState.hpp"
 #include "prims/jvmtiEventController.hpp"
 #include "prims/jvmtiThreadState.hpp"
+#include "prims/jvmtiThreadState.inline.hpp"
 #include "runtime/fieldDescriptor.hpp"
 #include "runtime/frame.hpp"
 #include "runtime/handles.inline.hpp"
@@ -334,6 +335,60 @@
   JvmtiEnv* next(JvmtiEnvBase* env) { return env->next_environment(); }
 };
 
+// VM operation to update for pop top frame.
+class VM_UpdateForPopTopFrame : public VM_Operation {
+private:
+  JvmtiThreadState* _state;
+  jvmtiError _result;
+
+public:
+  VM_UpdateForPopTopFrame(JvmtiThreadState* state) {
+    _state = state;
+    _result = JVMTI_ERROR_NONE;
+  }
+  VMOp_Type type() const { return VMOp_UpdateForPopTopFrame; }
+  jvmtiError result() { return _result; }
+  void doit() {
+    JavaThread* jt = _state->get_thread();
+    if (Threads::includes(jt) && !jt->is_exiting() && jt->threadObj() != NULL) {
+      _state->update_for_pop_top_frame();
+    } else {
+      _result = JVMTI_ERROR_THREAD_NOT_ALIVE;
+    }
+  }
+};
+
+// VM operation to set frame pop.
+class VM_SetFramePop : public VM_Operation {
+private:
+  JvmtiEnv *_env;
+  JvmtiThreadState* _state;
+  jint _depth;
+  jvmtiError _result;
+
+public:
+  VM_SetFramePop(JvmtiEnv *env, JvmtiThreadState* state, jint depth) {
+    _env = env;
+    _state = state;
+    _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() {
+    JavaThread* jt = _state->get_thread();
+    if (Threads::includes(jt) && !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;
+    }
+  }
+};
+
 
 // VM operation to get monitor information with stack depth.
 class VM_GetOwnedMonitorInfo : public VM_Operation {
diff -r c9ebeb289d1c -r e87156376bed hotspot/src/share/vm/prims/jvmtiEnvThreadState.cpp
--- a/hotspot/src/share/vm/prims/jvmtiEnvThreadState.cpp	Sat Mar 01 01:36:48 2014 -0800
+++ b/hotspot/src/share/vm/prims/jvmtiEnvThreadState.cpp	Sat Mar 01 08:05:55 2014 -0800
@@ -190,12 +190,8 @@
 
 
 JvmtiFramePops* JvmtiEnvThreadState::get_frame_pops() {
-#ifdef ASSERT
-  uint32_t debug_bits = 0;
-#endif
-  assert(get_thread() == Thread::current() || JvmtiEnv::is_thread_fully_suspended(get_thread(), false, &debug_bits),
-         "frame pop data only accessible from same thread or while suspended");
-
+  assert(get_thread() == Thread::current() || SafepointSynchronize::is_at_safepoint(),
+         "frame pop data only accessible from same thread or at safepoint");
   if (_frame_pops == NULL) {
     _frame_pops = new JvmtiFramePops();
     assert(_frame_pops != NULL, "_frame_pops != NULL");
@@ -209,44 +205,32 @@
 }
 
 void JvmtiEnvThreadState::set_frame_pop(int frame_number) {
-#ifdef ASSERT
-  uint32_t debug_bits = 0;
-#endif
-  assert(get_thread() == Thread::current() || JvmtiEnv::is_thread_fully_suspended(get_thread(), false, &debug_bits),
-         "frame pop data only accessible from same thread or while suspended");
+  assert(get_thread() == Thread::current() || SafepointSynchronize::is_at_safepoint(),
+         "frame pop data only accessible from same thread or at safepoint");
   JvmtiFramePop fpop(frame_number);
   JvmtiEventController::set_frame_pop(this, fpop);
 }
 
 
 void JvmtiEnvThreadState::clear_frame_pop(int frame_number) {
-#ifdef ASSERT
-  uint32_t debug_bits = 0;
-#endif
-  assert(get_thread() == Thread::current() || JvmtiEnv::is_thread_fully_suspended(get_thread(), false, &debug_bits),
-         "frame pop data only accessible from same thread or while suspended");
+  assert(get_thread() == Thread::current() || SafepointSynchronize::is_at_safepoint(),
+         "frame pop data only accessible from same thread or at safepoint");
   JvmtiFramePop fpop(frame_number);
   JvmtiEventController::clear_frame_pop(this, fpop);
 }
 
 
 void JvmtiEnvThreadState::clear_to_frame_pop(int frame_number)  {
-#ifdef ASSERT
-  uint32_t debug_bits = 0;
-#endif
-  assert(get_thread() == Thread::current() || JvmtiEnv::is_thread_fully_suspended(get_thread(), false, &debug_bits),
-         "frame pop data only accessible from same thread or while suspended");
+  assert(get_thread() == Thread::current() || SafepointSynchronize::is_at_safepoint(),
+         "frame pop data only accessible from same thread or at safepoint");
   JvmtiFramePop fpop(frame_number);
   JvmtiEventController::clear_to_frame_pop(this, fpop);
 }
 
 
 bool JvmtiEnvThreadState::is_frame_pop(int cur_frame_number) {
-#ifdef ASSERT
-  uint32_t debug_bits = 0;
-#endif
-  assert(get_thread() == Thread::current() || JvmtiEnv::is_thread_fully_suspended(get_thread(), false, &debug_bits),
-         "frame pop data only accessible from same thread or while suspended");
+  assert(get_thread() == Thread::current() || SafepointSynchronize::is_at_safepoint(),
+         "frame pop data only accessible from same thread or at safepoint");
   if (!get_thread()->is_interp_only_mode() || _frame_pops == NULL) {
     return false;
   }
diff -r c9ebeb289d1c -r e87156376bed hotspot/src/share/vm/prims/jvmtiEventController.cpp
--- a/hotspot/src/share/vm/prims/jvmtiEventController.cpp	Sat Mar 01 01:36:48 2014 -0800
+++ b/hotspot/src/share/vm/prims/jvmtiEventController.cpp	Sat Mar 01 08:05:55 2014 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2014, 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
@@ -989,21 +989,21 @@
 
 void
 JvmtiEventController::set_frame_pop(JvmtiEnvThreadState *ets, JvmtiFramePop fpop) {
-  MutexLocker mu(JvmtiThreadState_lock);
+  MutexLockerEx mu(SafepointSynchronize::is_at_safepoint() ? NULL : JvmtiThreadState_lock);
   JvmtiEventControllerPrivate::set_frame_pop(ets, fpop);
 }
 
 
 void
 JvmtiEventController::clear_frame_pop(JvmtiEnvThreadState *ets, JvmtiFramePop fpop) {
-  MutexLocker mu(JvmtiThreadState_lock);
+  MutexLockerEx mu(SafepointSynchronize::is_at_safepoint() ? NULL : JvmtiThreadState_lock);
   JvmtiEventControllerPrivate::clear_frame_pop(ets, fpop);
 }
 
 
 void
 JvmtiEventController::clear_to_frame_pop(JvmtiEnvThreadState *ets, JvmtiFramePop fpop) {
-  MutexLocker mu(JvmtiThreadState_lock);
+  MutexLockerEx mu(SafepointSynchronize::is_at_safepoint() ? NULL : JvmtiThreadState_lock);
   JvmtiEventControllerPrivate::clear_to_frame_pop(ets, fpop);
 }
 
diff -r c9ebeb289d1c -r e87156376bed hotspot/src/share/vm/prims/jvmtiThreadState.cpp
--- a/hotspot/src/share/vm/prims/jvmtiThreadState.cpp	Sat Mar 01 01:36:48 2014 -0800
+++ b/hotspot/src/share/vm/prims/jvmtiThreadState.cpp	Sat Mar 01 08:05:55 2014 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2014, 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
@@ -63,6 +63,7 @@
   _vm_object_alloc_event_collector = NULL;
   _the_class_for_redefinition_verification = NULL;
   _scratch_class_for_redefinition_verification = NULL;
+  _cur_stack_depth = UNKNOWN_STACK_DEPTH;
 
   // JVMTI ForceEarlyReturn support
   _pending_step_for_earlyret = false;
@@ -213,12 +214,9 @@
 
 // Helper routine used in several places
 int JvmtiThreadState::count_frames() {
-#ifdef ASSERT
-  uint32_t debug_bits = 0;
-#endif
-  assert(SafepointSynchronize::is_at_safepoint() ||
-         JvmtiEnv::is_thread_fully_suspended(get_thread(), false, &debug_bits),
-         "at safepoint or must be suspended");
+  guarantee(SafepointSynchronize::is_at_safepoint() ||
+    (JavaThread *)Thread::current() == get_thread(),
+    "must be current thread or at safepoint");
 
   if (!get_thread()->has_last_Java_frame()) return 0;  // no Java frames
 
@@ -243,15 +241,9 @@
 
 
 void JvmtiThreadState::invalidate_cur_stack_depth() {
-  Thread *cur = Thread::current();
-  uint32_t debug_bits = 0;
-
-  // The caller can be the VMThread at a safepoint, the current thread
-  // or the target thread must be suspended.
-  guarantee((cur->is_VM_thread() && SafepointSynchronize::is_at_safepoint()) ||
-    (JavaThread *)cur == get_thread() ||
-    JvmtiEnv::is_thread_fully_suspended(get_thread(), false, &debug_bits),
-    "sanity check");
+  guarantee(SafepointSynchronize::is_at_safepoint() ||
+    (JavaThread *)Thread::current() == get_thread(),
+    "must be current thread or at safepoint");
 
   _cur_stack_depth = UNKNOWN_STACK_DEPTH;
 }
@@ -280,10 +272,9 @@
 }
 
 int JvmtiThreadState::cur_stack_depth() {
-  uint32_t debug_bits = 0;
-  guarantee(JavaThread::current() == get_thread() ||
-    JvmtiEnv::is_thread_fully_suspended(get_thread(), false, &debug_bits),
-    "must be current thread or suspended");
+  guarantee(SafepointSynchronize::is_at_safepoint() ||
+    (JavaThread *)Thread::current() == get_thread(),
+    "must be current thread or at safepoint");
 
   if (!is_interp_only_mode() || _cur_stack_depth == UNKNOWN_STACK_DEPTH) {
     _cur_stack_depth = count_frames();
diff -r c9ebeb289d1c -r e87156376bed hotspot/src/share/vm/runtime/vm_operations.hpp
--- a/hotspot/src/share/vm/runtime/vm_operations.hpp	Sat Mar 01 01:36:48 2014 -0800
+++ b/hotspot/src/share/vm/runtime/vm_operations.hpp	Sat Mar 01 08:05:55 2014 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2014, 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
@@ -74,6 +74,8 @@
   template(PopulateDumpSharedSpace)               \
   template(JNIFunctionTableCopier)                \
   template(RedefineClasses)                       \
+  template(UpdateForPopTopFrame)                  \
+  template(SetFramePop)                           \
   template(GetOwnedMonitorInfo)                   \
   template(GetObjectMonitorUsage)                 \
   template(GetCurrentContendedMonitor)            \