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

Keith McGuigan keith.mcguigan at oracle.com
Mon Jan 10 17🔞02 PST 2011

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::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::threadended(JavaThread *thread) { // Removes the JvmtiThreadState associated with the specified thread. // May be called after all environments have been disposed. ECTRACE(("JVMTI [%s] # thread ended", JvmtiTrace::safegetthreadname(thread))); MutexLocker mu(JvmtiThreadStatelock); JvmtiThreadState *state = thread->jvmtithreadstate(); if (state != NULL) { delete state; } } and then in JavaThread::exit just do: if (JvmtiEnv::environmentsmightexist()) { JvmtiExport::cleanupthread(this); } as cleanupthread 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!

--

More information about the hotspot-runtime-dev mailing list