Fix acquireVMaccessIfNeeded for Non-Compiler Threads

runtime/compiler/control/CompilationThread.cpp

@@ -5806,12 +5806,16 @@ static void deleteMethodHandleRef(J9::MethodHandleThunkDetails & details, J9VMTh
       vmThread->javaVM->internalVMFunctions->j9jni_deleteGlobalRef((JNIEnv*)vmThread, (jobject)details.getArgRef(), false);
    }
 
-
+/**
+ * @attention Must be called with VMAccess in hand.
+ */
 void *TR::CompilationInfo::compileMethod(J9VMThread * vmThread, TR::IlGeneratorMethodDetails & details, void *oldStartPC,
                                         TR_YesNoMaybe requireAsyncCompile,
                                         TR_CompilationErrorCode *compErrCode,
                                         bool *queued, TR_OptimizationPlan * optimizationPlan)
    {
+   TR_ASSERT_FATAL(vmThread->publicFlags & J9_PUBLIC_FLAGS_VM_ACCESS, "%p does not have VMAccess\n", vmThread);
+
    TR_J9VMBase * fe = TR_J9VMBase::get(_jitConfig, vmThread);
    TR_ASSERT(!fe->isAOT_DEPRECATED_DO_NOT_USE(), "We need a non-AOT vm here.");
 
@@ -5952,6 +5956,9 @@ extern bool
 validateSharedClassAOTHeader(J9JavaVM *javaVM, J9VMThread *curThread, TR::CompilationInfo *compInfo, TR_FrontEnd *fe);
 #endif
 
+/**
+ * @attention Must be called with VMAccess in hand.
+ */
 void *TR::CompilationInfo::compileOnSeparateThread(J9VMThread * vmThread, TR::IlGeneratorMethodDetails & details,
                                                   void *oldStartPC, TR_YesNoMaybe requireAsyncCompile,
                                                   TR_CompilationErrorCode *compErrCode,

runtime/compiler/env/VMJ9.cpp

@@ -278,50 +278,46 @@ TR_J9VMBase::instanceOfOrCheckCastNoCacheUpdate(J9Class *instanceClass, J9Class*
    return ::instanceOfOrCheckCastNoCacheUpdate(instanceClass, castClass);
    }
 
-// Points to address: what if vmThread is not the compilation thread.
 // What if the compilation thread does not have the classUnloadMonitor.
 // What if jitConfig does not exist.
 // What if we already have the compilationShouldBeInterrupted flag set.
 // How do we set the error code when we throw an exception
 
-// IMPORTANT: acquireVMAccessIfNeeded could throw an exception,
-// hence it is important to call this within a try block.
+// IMPORTANT: acquireVMAccessIfNeeded could throw an exception if the vmThread
+// **IS** a Compilation Thread; hence it is important to call this within a try
+// block.
 bool acquireVMaccessIfNeeded(J9VMThread *vmThread, TR_YesNoMaybe isCompThread)
       {
-      bool haveAcquiredVMAccess = false;
-      if (TR::Options::getCmdLineOptions() == 0 || // if options haven't been created yet, there is no danger
-          TR::Options::getCmdLineOptions()->getOption(TR_DisableNoVMAccess))
-          return false; // don't need to acquire VM access
-
       // we need to test if the thread has VM access
       TR_ASSERT(vmThread, "vmThread must be not null");
 
-      // We need to acquire VMaccess only for the compilation thread
-      if (isCompThread == TR_no)
-         {
-         TR_ASSERT((vmThread->publicFlags & J9_PUBLIC_FLAGS_VM_ACCESS), "Must have vm access if this is not a compilation thread");
-         return false;
-         }
-
-      // scan all compilation threads
-      J9JITConfig        *jitConfig = vmThread->javaVM->jitConfig;
-      TR::CompilationInfo *compInfo  = TR::CompilationInfo::get(jitConfig);
-      TR::CompilationInfoPerThread *compInfoPT = compInfo->getCompInfoForThread(vmThread);
-
-      // We need to acquire VMaccess only for the compilation thread
-      if (isCompThread == TR_maybe)
+      bool haveAcquiredVMAccess = false;
+      J9JITConfig *jitConfig = vmThread->javaVM->jitConfig;
+      TR::CompilationInfo *compInfo = TR::CompilationInfo::get(jitConfig);
+      TR::CompilationInfoPerThread *compInfoPT = isCompThread != TR_no
+                                                 ? compInfo->getCompInfoForThread(vmThread)
+                                                 : NULL;
+
+      // If the current thread is not a compilation thread, acquire VM Access
+      // and return immediately.
+      if (!compInfoPT)
          {
-         if (!compInfoPT)
+         if (!(vmThread->publicFlags & J9_PUBLIC_FLAGS_VM_ACCESS))
             {
-            TR_ASSERT((vmThread->publicFlags & J9_PUBLIC_FLAGS_VM_ACCESS), "Must have vm access if this is not a compilation thread");
-            return false;
+            acquireVMAccessNoSuspend(vmThread);
+            haveAcquiredVMAccess = true;
             }
+         return haveAcquiredVMAccess;
          }
 
       // At this point we know we deal with a compilation thread
-      TR_ASSERT(compInfoPT, "A compilation thread must have an associated compilationInfo");
 
-      if (!(vmThread->publicFlags &  J9_PUBLIC_FLAGS_VM_ACCESS)) // I don't already have VM access
+      if (TR::Options::getCmdLineOptions() == 0 || // if options haven't been created yet, there is no danger
+          TR::Options::getCmdLineOptions()->getOption(TR_DisableNoVMAccess))
+         return false; // don't need to acquire VM access
+
+      // I don't already have VM access
+      if (!(vmThread->publicFlags &  J9_PUBLIC_FLAGS_VM_ACCESS))
          {
          if (0 == vmThread->javaVM->internalVMFunctions->internalTryAcquireVMAccessWithMask(vmThread, J9_PUBLIC_FLAGS_HALT_THREAD_ANY_NO_JAVA_SUSPEND))
             {

runtime/compiler/runtime/IProfiler.cpp

@@ -50,6 +50,7 @@
 #include "env/PersistentInfo.hpp"
 #include "env/jittypes.h"
 #include "env/VerboseLog.hpp"
+#include "env/VMAccessCriticalSection.hpp"
 #include "il/Block.hpp"
 #include "il/DataTypes.hpp"
 #include "il/Node.hpp"
@@ -4814,23 +4815,12 @@ void TR_IProfiler::dumpIPBCDataCallGraph(J9VMThread* vmThread)
       return;
       }
 
-   // Need to have VM access to block GC from invalidating entries
-   // Application threads may still add data, but that is safe
-   // Ideally we should use  TR::VMAccessCriticalSection dumpInfoCS(fe);
-   // here, but this construct wants asserts that an application thread must have vmAccess.
-   // This might not be true at shutdown when dumpInfo is likely to be called
-   //
-   bool haveAcquiredVMAccess = false;
-   if (!(vmThread->publicFlags &  J9_PUBLIC_FLAGS_VM_ACCESS)) // I don't already have VM access
-      {
-      acquireVMAccessNoSuspend(vmThread);
-      haveAcquiredVMAccess = true;
-      }
-
    J9JavaVM *javaVM = vmThread->javaVM;
    J9InternalVMFunctions *vmFunctions = javaVM->internalVMFunctions;
    TR_J9VMBase * fe = TR_J9VMBase::get(javaVM->jitConfig, vmThread);
 
+   TR::VMAccessCriticalSection dumpCallGraph(fe);
+
    fprintf(stderr, "Aggregating per method ...\n");
    for (int32_t bucket = 0; bucket < TR::Options::_iProfilerBcHashTableSize; bucket++)
       {
@@ -4883,8 +4873,6 @@ void TR_IProfiler::dumpIPBCDataCallGraph(J9VMThread* vmThread)
          }
       }
    aggregationHT.sortByNameAndPrint(fe);
-   if (haveAcquiredVMAccess)
-      releaseVMAccessNoSuspend(vmThread);
 
    fprintf(stderr, "Finished dumping info\n");
    }