United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: 6800721 JavaThread::jvmti_thread_state() and JvmtiThreadState::state_for() robustness
6800721 : JavaThread::jvmti_thread_state() and JvmtiThreadState::state_for() robustness

Details
Type:
Bug
Submit Date:
2009-02-03
Status:
Resolved
Updated Date:
2010-04-02
Project Name:
JDK
Resolved Date:
2009-03-18
Component:
hotspot
OS:
generic
Sub-Component:
jvmti
CPU:
generic
Priority:
P3
Resolution:
Fixed
Affected Versions:
hs14
Fixed Versions:
hs15

Related Reports
Backport:
JDK-2173781 - JavaThread::jvmti_thread_state() and JvmtiThreadState::state_for() robustness
Backport:
JDK-2174068 - JavaThread::jvmti_thread_state() and JvmtiThreadState::state_for() robustness
Backport:
JDK-2174466 - JavaThread::jvmti_thread_state() and JvmtiThreadState::state_for() robustness
Backport:
JDK-2180522 - JavaThread::jvmti_thread_state() and JvmtiThreadState::state_for() robustness
Relates:
JDK-6700114 - Assertion (_thread->get_interp_only_mode() == 1,"leaving interp only when mode not one")

Sub Tasks

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
2009-03-03 
SUGGESTED FIX

See the attached 6800721-webrev-cr0.tgz for the proposed fix.
2009-02-23 
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.
2009-02-03


Hardware and Software, Engineered to Work Together