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.