request for review (S): 6814943: getcpool001 catches more than one JvmtiThreadState problem (original) (raw)

David Holmes David.Holmes at oracle.com
Mon Jan 10 16:07:21 PST 2011

Hi Keith,

Keith McGuigan said the following on 01/11/11 03:36:

This closes a race condition hole between JvmtiThreadState::stateforwhilelocked() and ~JavaThread(). Without this, the stateforwhilelocked() could see a value of false for thread->isexiting(), then the entirety of ~JavaThread() could run, the stateforwhilelocked() 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.

David

More information about the hotspot-runtime-dev mailing list