On Jan 10, 2011, at 7:07 PM, David Holmes wrote:
Hi Keith,
Keith McGuigan said the following on 01/11/11 03:36:
This closes a race condition hole between
JvmtiThreadState::state_for_while_locked() and ~JavaThread().
Without this, the state_for_while_locked() could see a value of
false for thread->is_exiting(), then the entirety of ~JavaThread()
could run, the state_for_while_locked() could then finish leaving
the JvmtiThreadState referring to a zombie thread.
webrev: http://cr.openjdk.java.net/~kamg/6814943/webrev.00/
I think it would be slightly cleaner, as per my private email, to
keep the locking internal to the JVMTI methods ie:
void
JvmtiEventControllerPrivate::thread_ended(JavaThread *thread) {
// Removes the JvmtiThreadState associated with the specified thread.
// May be called after all environments have been disposed.
EC_TRACE(("JVMTI [%s] # thread ended",
JvmtiTrace::safe_get_thread_name(thread)));
MutexLocker mu(JvmtiThreadState_lock);
JvmtiThreadState *state = thread->jvmti_thread_state();
if (state != NULL) {
delete state;
}
}
and then in JavaThread::exit just do:
if (JvmtiEnv::environments_might_exist()) {
JvmtiExport::cleanup_thread(this);
}
as cleanup_thread already does a null check. Note that if an
environment comes into existence we are still relying on another
thread seeing that this thread has terminated prior to the above
check.
Sure that sounds reasonable. I'll change the code to do that.
Thanks!
--
- Keith
