This issue came up during the code review of the fix for:

    6419370 4/4 new jmethodID code has tiny holes in synchronization

since the comments were about existing code, I filed this new bug.

JVM/TI's uses of Threads::number_of_threads() need to be closely
examined and verified to be valid. Consider this use:

src/share/vm/prims/jvmtiEnv.cpp:

  2933  // rmonitor - pre-checked for validity
  2934  jvmtiError
  2935  JvmtiEnv::RawMonitorEnter(JvmtiRawMonitor * rmonitor) {
  2936    if (Threads::number_of_threads() == 0) {
  2937      // No JavaThreads exist so ObjectMonitor enter cannot be
  2938      // used, add this raw monitor to the pending list.
  2939      // The pending monitors will be actually entered when
  2940      // the VM is setup.
  2941      // See transition_pending_raw_monitors in create_vm()
  2942      // in thread.cpp.
  2943      JvmtiPendingMonitors::enter(rmonitor);
  2944    } else {
  2945      int r;
  2946      Thread* thread = Thread::current();

RawMonitors are built on top of ObjectMonitors and thus cannot be
used before we have JavaThreads so the above use is generally
correct. The only part I wonder about is proper coordination
between agent thread(s) adding to JvmtiPendingMonitors and the
create Java VM thread cleaning up JvmtiPendingMonitors after
the main JavaThread has been started. More investigation is
needed there.


Now consider this usage:

src/share/vm/prims/jvmtiEnv.cpp

   293  // prefix_count - pre-checked to be greater than or equal to 0
   294  // prefixes - pre-checked for NULL
   295  jvmtiError
   296  JvmtiEnv::SetNativeMethodPrefixes(jint prefix_count, char** prefixes) {
   297    // Have to grab JVMTI thread state lock to be sure that some thread
   298    // isn't accessing the prefixes at the same time we are setting them.
   299    // No locks during VM bring-up.
   300    if (Threads::number_of_threads() == 0) {
   301      return set_native_method_prefixes(prefix_count, prefixes);
   302    } else {
   303      MutexLocker mu(JvmtiThreadState_lock);
   304      return set_native_method_prefixes(prefix_count, prefixes);
   305    }
   306  } /* end SetNativeMethodPrefixes */

The comment on line 299 explains why we don't grab the
JvmtiThreadState_lock, but I'm not sure that it is right.
Consider src/share/vm/runtime/mutexLocker.hpp:

extern Mutex*   JvmtiThreadState_lock;           // a lock on modification of JVMTI thread data

A Mutex object is different than a JvmtiRawMonitor/ObjectMonitor
object so I think we need to figure out if there is a real reason
for not grabbing various Mutex objects in JVM/TI.


And finally consider this usage:

src/share/vm/prims/jvmtiExport.cpp:

   438  char**
   439  JvmtiExport::get_all_native_method_prefixes(int* count_ptr) {
   440    // Have to grab JVMTI thread state lock to be sure environment doesn't   441    // go away while we iterate them.  No locks during VM bring-up.
   442    if (Threads::number_of_threads() == 0 || SafepointSynchronize::is_at_safepoint()) {
   443      return JvmtiEnvBase::get_all_native_method_prefixes(count_ptr);
   444    } else {
   445      MutexLocker mu(JvmtiThreadState_lock);
   446      return JvmtiEnvBase::get_all_native_method_prefixes(count_ptr);
   447    }
   448  }

The "Threads::number_of_threads() == 0" check has the same concerns
as mentioned before. The SafepointSynchronize::is_at_safepoint()
check allows the VMThread to call the function without trying to
acquire the Mutex. However, because that check does not also verify
that the caller is the VMThread there is concern that another thread
could be permitted unlocked access. If there are other threads that
could be using this unlocked code path during a safepoint, then there
is also concern that the system could leave the safepoint so we'd
have unlocked access racing with locked access. More investigation
is needed here also.