JDK-6800721 : JavaThread::jvmti_thread_state() and JvmtiThreadState::state_for() robustness
  • Type: Bug
  • Component: hotspot
  • Sub-Component: jvmti
  • Affected Version: hs14
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2009-02-03
  • Updated: 2010-04-02
  • Resolved: 2009-03-18
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 6 JDK 7 Other
6u14Fixed 7Fixed hs14Fixed
Related Reports
Relates :  
JDK-6700114 - Assertion (_thread->get_interp_only_mode() == 1,"leaving interp only when mode not one")
Description
Some JavaThread::jvmti_thread_state()/JvmtiThreadState::state_for()
callers are written correctly:

src/share/vm/prims/jni.cpp: jni_ExceptionClear()

   620    JvmtiThreadState *state = JavaThread::current()->jvmti_thread_state();
   621    if (state != NULL && state->is_exception_detected()) {
   622      state->set_exception_caught();
   623    }


And some JavaThread::jvmti_thread_state()/JvmtiThreadState::state_for()
callers assume that NULL can never be returned:

src/share/vm/prims/jvmtiEnv.cpp: JvmtiEnv::SetThreadLocalStorage()

   100      // otherwise, create the state
   101      state = JvmtiThreadState::state_for(java_thread);
   102    }
   103    state->env_thread_state(this)->set_agent_thread_local_storage_data((void*)data);
Comments
EVALUATION http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/ea20d7ce26b0
03-03-2009
SUGGESTED FIX See the attached 6800721-webrev-cr0.tgz for the proposed fix.
23-02-2009
EVALUATION Some of the NULL checks need to return an error code: 101 state = JvmtiThreadState::state_for(java_thread); 102 if (state == NULL) { 103 return JVMTI_ERROR_THREAD_NOT_ALIVE; 104 } Some of the state_for() calls and new error code return checks are moved above the check that returns JVMTI_ERROR_THREAD_NOT_SUSPENDED. I think it makes sense to check for liveness before suspension and return the error codes in that order; an exiting thread won't show up as being not suspended. Some of the NULL checks need to return a benign value: 481 if (state == NULL) { 482 // associated JavaThread is exiting 483 return (jlong)0; 484 } Some of the NULL checks are guarantee() calls since a NULL JvmtiThreadState value shouldn't happen in that location, i.e., when the caller is the current thread, it can't be exiting.
03-02-2009