6800721: 3/4 JavaThread::jvmti_thread_state() and JvmtiThreadState::state_for() robustness
authordcubed
Mon, 02 Mar 2009 14:00:23 -0700
changeset 2135 f82c3012ec86
parent 2134 125f0fb7e9b1
child 2136 c55428da3cec
6800721: 3/4 JavaThread::jvmti_thread_state() and JvmtiThreadState::state_for() robustness Summary: Check for NULL return values from jvmti_thread_state() and state_for() and return a JVM TI error code as appropriate. Reviewed-by: coleenp, swamyv
hotspot/src/share/vm/prims/jvmtiEnv.cpp
hotspot/src/share/vm/prims/jvmtiEnvBase.cpp
hotspot/src/share/vm/prims/jvmtiEventController.cpp
hotspot/src/share/vm/prims/jvmtiExport.cpp
hotspot/src/share/vm/prims/jvmtiRedefineClasses.cpp
hotspot/src/share/vm/prims/jvmtiThreadState.hpp
hotspot/src/share/vm/runtime/thread.hpp
--- a/hotspot/src/share/vm/prims/jvmtiEnv.cpp	Mon Mar 02 13:57:17 2009 -0700
+++ b/hotspot/src/share/vm/prims/jvmtiEnv.cpp	Mon Mar 02 14:00:23 2009 -0700
@@ -99,6 +99,9 @@
     }
     // otherwise, create the state
     state = JvmtiThreadState::state_for(java_thread);
+    if (state == NULL) {
+      return JVMTI_ERROR_THREAD_NOT_ALIVE;
+    }
   }
   state->env_thread_state(this)->set_agent_thread_local_storage_data((void*)data);
   return JVMTI_ERROR_NONE;
@@ -1308,6 +1311,9 @@
 
   // retrieve or create JvmtiThreadState.
   JvmtiThreadState* state = JvmtiThreadState::state_for(java_thread);
+  if (state == NULL) {
+    return JVMTI_ERROR_THREAD_NOT_ALIVE;
+  }
   uint32_t debug_bits = 0;
   if (is_thread_fully_suspended(java_thread, true, &debug_bits)) {
     err = get_frame_count(state, count_ptr);
@@ -1329,6 +1335,12 @@
   HandleMark hm(current_thread);
   uint32_t debug_bits = 0;
 
+  // retrieve or create the state
+  JvmtiThreadState* state = JvmtiThreadState::state_for(java_thread);
+  if (state == NULL) {
+    return JVMTI_ERROR_THREAD_NOT_ALIVE;
+  }
+
   // Check if java_thread is fully suspended
   if (!is_thread_fully_suspended(java_thread, true /* wait for suspend completion */, &debug_bits)) {
     return JVMTI_ERROR_THREAD_NOT_SUSPENDED;
@@ -1399,9 +1411,6 @@
     // It's fine to update the thread state here because no JVMTI events
     // shall be posted for this PopFrame.
 
-    // retreive or create the state
-    JvmtiThreadState* state = JvmtiThreadState::state_for(java_thread);
-
     state->update_for_pop_top_frame();
     java_thread->set_popframe_condition(JavaThread::popframe_pending_bit);
     // Set pending step flag for this popframe and it is cleared when next
@@ -1445,6 +1454,11 @@
   ResourceMark rm;
   uint32_t debug_bits = 0;
 
+  JvmtiThreadState *state = JvmtiThreadState::state_for(java_thread);
+  if (state == NULL) {
+    return JVMTI_ERROR_THREAD_NOT_ALIVE;
+  }
+
   if (!JvmtiEnv::is_thread_fully_suspended(java_thread, true, &debug_bits)) {
       return JVMTI_ERROR_THREAD_NOT_SUSPENDED;
   }
@@ -1464,7 +1478,6 @@
 
   assert(vf->frame_pointer() != NULL, "frame pointer mustn't be NULL");
 
-  JvmtiThreadState *state = JvmtiThreadState::state_for(java_thread);
   int frame_number = state->count_frames() - depth;
   state->env_thread_state(this)->set_frame_pop(frame_number);
 
--- a/hotspot/src/share/vm/prims/jvmtiEnvBase.cpp	Mon Mar 02 13:57:17 2009 -0700
+++ b/hotspot/src/share/vm/prims/jvmtiEnvBase.cpp	Mon Mar 02 14:00:23 2009 -0700
@@ -1322,6 +1322,12 @@
   HandleMark   hm(current_thread);
   uint32_t debug_bits = 0;
 
+  // retrieve or create the state
+  JvmtiThreadState* state = JvmtiThreadState::state_for(java_thread);
+  if (state == NULL) {
+    return JVMTI_ERROR_THREAD_NOT_ALIVE;
+  }
+
   // Check if java_thread is fully suspended
   if (!is_thread_fully_suspended(java_thread,
                                  true /* wait for suspend completion */,
@@ -1329,9 +1335,6 @@
     return JVMTI_ERROR_THREAD_NOT_SUSPENDED;
   }
 
-  // retreive or create the state
-  JvmtiThreadState* state = JvmtiThreadState::state_for(java_thread);
-
   // Check to see if a ForceEarlyReturn was already in progress
   if (state->is_earlyret_pending()) {
     // Probably possible for JVMTI clients to trigger this, but the
--- a/hotspot/src/share/vm/prims/jvmtiEventController.cpp	Mon Mar 02 13:57:17 2009 -0700
+++ b/hotspot/src/share/vm/prims/jvmtiEventController.cpp	Mon Mar 02 14:00:23 2009 -0700
@@ -478,6 +478,11 @@
 // set external state accordingly.  Only thread-filtered events are included.
 jlong
 JvmtiEventControllerPrivate::recompute_thread_enabled(JvmtiThreadState *state) {
+  if (state == NULL) {
+    // associated JavaThread is exiting
+    return (jlong)0;
+  }
+
   jlong was_any_env_enabled = state->thread_event_enable()->_event_enabled.get_bits();
   jlong any_env_enabled = 0;
 
@@ -553,6 +558,7 @@
     {
       MutexLocker mu(Threads_lock);   //hold the Threads_lock for the iteration
       for (JavaThread *tp = Threads::first(); tp != NULL; tp = tp->next()) {
+        // state_for_while_locked() makes tp->is_exiting() check
         JvmtiThreadState::state_for_while_locked(tp);  // create the thread state if missing
       }
     }// release Threads_lock
--- a/hotspot/src/share/vm/prims/jvmtiExport.cpp	Mon Mar 02 13:57:17 2009 -0700
+++ b/hotspot/src/share/vm/prims/jvmtiExport.cpp	Mon Mar 02 14:00:23 2009 -0700
@@ -1872,6 +1872,9 @@
 {
   // register the stub with the current dynamic code event collector
   JvmtiThreadState* state = JvmtiThreadState::state_for(JavaThread::current());
+  // state can only be NULL if the current thread is exiting which
+  // should not happen since we're trying to post an event
+  guarantee(state != NULL, "attempt to register stub via an exiting thread");
   JvmtiDynamicCodeEventCollector* collector = state->get_dynamic_code_event_collector();
   guarantee(collector != NULL, "attempt to register stub without event collector");
   collector->register_stub(name, code_begin, code_end);
@@ -2253,6 +2256,9 @@
 void JvmtiEventCollector::setup_jvmti_thread_state() {
   // set this event collector to be the current one.
   JvmtiThreadState* state = JvmtiThreadState::state_for(JavaThread::current());
+  // state can only be NULL if the current thread is exiting which
+  // should not happen since we're trying to configure for event collection
+  guarantee(state != NULL, "exiting thread called setup_jvmti_thread_state");
   if (is_vm_object_alloc_event()) {
     _prev = state->get_vm_object_alloc_event_collector();
     state->set_vm_object_alloc_event_collector((JvmtiVMObjectAllocEventCollector *)this);
--- a/hotspot/src/share/vm/prims/jvmtiRedefineClasses.cpp	Mon Mar 02 13:57:17 2009 -0700
+++ b/hotspot/src/share/vm/prims/jvmtiRedefineClasses.cpp	Mon Mar 02 14:00:23 2009 -0700
@@ -831,6 +831,9 @@
   ResourceMark rm(THREAD);
 
   JvmtiThreadState *state = JvmtiThreadState::state_for(JavaThread::current());
+  // state can only be NULL if the current thread is exiting which
+  // should not happen since we're trying to do a RedefineClasses
+  guarantee(state != NULL, "exiting thread calling load_new_class_versions");
   for (int i = 0; i < _class_count; i++) {
     oop mirror = JNIHandles::resolve_non_null(_class_defs[i].klass);
     // classes for primitives cannot be redefined
--- a/hotspot/src/share/vm/prims/jvmtiThreadState.hpp	Mon Mar 02 13:57:17 2009 -0700
+++ b/hotspot/src/share/vm/prims/jvmtiThreadState.hpp	Mon Mar 02 14:00:23 2009 -0700
@@ -314,6 +314,7 @@
   void update_for_pop_top_frame();
 
   // already holding JvmtiThreadState_lock - retrieve or create JvmtiThreadState
+  // Can return NULL if JavaThread is exiting.
   inline static JvmtiThreadState *state_for_while_locked(JavaThread *thread) {
     assert(JvmtiThreadState_lock->is_locked(), "sanity check");
 
@@ -330,6 +331,7 @@
   }
 
   // retrieve or create JvmtiThreadState
+  // Can return NULL if JavaThread is exiting.
   inline static JvmtiThreadState *state_for(JavaThread *thread) {
     JvmtiThreadState *state = thread->jvmti_thread_state();
     if (state == NULL) {
--- a/hotspot/src/share/vm/runtime/thread.hpp	Mon Mar 02 13:57:17 2009 -0700
+++ b/hotspot/src/share/vm/runtime/thread.hpp	Mon Mar 02 14:00:23 2009 -0700
@@ -1345,6 +1345,13 @@
  public:
   // Thread local information maintained by JVMTI.
   void set_jvmti_thread_state(JvmtiThreadState *value)                           { _jvmti_thread_state = value; }
+  // A JvmtiThreadState is lazily allocated. This jvmti_thread_state()
+  // getter is used to get this JavaThread's JvmtiThreadState if it has
+  // one which means NULL can be returned. JvmtiThreadState::state_for()
+  // is used to get the specified JavaThread's JvmtiThreadState if it has
+  // one or it allocates a new JvmtiThreadState for the JavaThread and
+  // returns it. JvmtiThreadState::state_for() will return NULL only if
+  // the specified JavaThread is exiting.
   JvmtiThreadState *jvmti_thread_state() const                                   { return _jvmti_thread_state; }
   static ByteSize jvmti_thread_state_offset()                                    { return byte_offset_of(JavaThread, _jvmti_thread_state); }
   void set_jvmti_get_loaded_classes_closure(JvmtiGetLoadedClassesClosure* value) { _jvmti_get_loaded_classes_closure = value; }