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

[David Holmes]

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.

David