Merge pull request #12278 from fjeremic/fix-jitdump-lock

Fix deadlock and potential assert in JitDump
eclipse-openj9 · Apr 6, 2021 · 0d86a4f · 0d86a4f
2 parents 6d005e0 + 40ae89f
commit 0d86a4f
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 29 deletions.
diff --git a/runtime/compiler/control/CompilationRuntime.hpp b/runtime/compiler/control/CompilationRuntime.hpp
@@ -863,7 +863,7 @@ class CompilationInfo
    UDATA getVMStateOfCrashedThread() { return _vmStateOfCrashedThread; }
    void setVMStateOfCrashedThread(UDATA vmState) { _vmStateOfCrashedThread = vmState; }
    void printCompQueue();
-   TR::CompilationInfoPerThread *getCompilationInfoForDumpThread() const { return _compInfoForDiagnosticCompilationThread; }
+   TR::CompilationInfoPerThread *getCompilationInfoForDiagnosticThread() const { return _compInfoForDiagnosticCompilationThread; }
    TR::CompilationInfoPerThread * const *getArrayOfCompilationInfoPerThread() const { return _arrayOfCompilationInfoPerThread; }
    uint32_t getAotQueryTime() { return _statTotalAotQueryTime; }
    void     setAotQueryTime(uint32_t queryTime) { _statTotalAotQueryTime = queryTime; }

diff --git a/runtime/compiler/control/CompilationThread.cpp b/runtime/compiler/control/CompilationThread.cpp
@@ -2464,7 +2464,10 @@ void TR::CompilationInfoPerThread::suspendCompilationThread()
    if (compilationThreadIsActive())
       {
       setCompilationThreadState(COMPTHREAD_SIGNAL_SUSPEND);
-      getCompilationInfo()->decNumCompThreadsActive();
+
+      if (!isDiagnosticThread())
+         getCompilationInfo()->decNumCompThreadsActive();
+
       if (TR::Options::getCmdLineOptions()->getVerboseOption(TR_VerboseCompilationThreads))
          {
          TR_VerboseLog::writeLineLocked(TR_Vlog_PERF,"t=%6u Suspension request for compThread %d sleeping=%s",

diff --git a/runtime/compiler/control/JitDump.cpp b/runtime/compiler/control/JitDump.cpp
@@ -251,6 +251,7 @@ runJitdump(char *label, J9RASdumpContext *context, J9RASdumpAgent *agent)
 
    char *crashedThreadName = getOMRVMThreadName(crashedThread->omrVMThread);
    j9nls_printf(PORTLIB, J9NLS_INFO | J9NLS_STDERR, J9NLS_DMP_OCCURRED_THREAD_NAME_ID, "JIT", crashedThreadName, crashedThread);
+   releaseOMRVMThreadName(crashedThread->omrVMThread);
 
    J9JITConfig *jitConfig = crashedThread->javaVM->jitConfig;
    if (NULL == jitConfig)
@@ -317,7 +318,7 @@ runJitdump(char *label, J9RASdumpContext *context, J9RASdumpAgent *agent)
 
    compInfo->getPersistentInfo()->setDisableFurtherCompilation(true);
 
-   TR::CompilationInfoPerThread *recompilationThreadInfo = compInfo->getCompilationInfoForDumpThread();
+   TR::CompilationInfoPerThread *recompilationThreadInfo = compInfo->getCompilationInfoForDiagnosticThread();
    if (NULL == recompilationThreadInfo)
       {
       j9nls_printf(PORTLIB, J9NLS_ERROR | J9NLS_STDERR, J9NLS_DMP_ERROR_IN_DUMP_STR, "JIT", "Could not locate the diagnostic thread info");
@@ -428,35 +429,38 @@ runJitdump(char *label, J9RASdumpContext *context, J9RASdumpAgent *agent)
          {
          // We are an application thread
 
-         // The stack walker may not be able to walk the stack if the crash did not happen on a transition frame. In
-         // such cases the stack walker will resume walking the stack on the last known valid point, which will be a
-         // transition frame further up the stack (ex. INT -> JIT transition frame). This will result in the stack
-         // walker potentially skipping some JIT methods on the backtrace. This is not desirable. Chances are high
-         // that the crash happen because of a miscompilation in the first JIT compiled method on the stack, so it
-         // is imperative that we recompile the method we actually crashed in.
-         const char *name;
-         void *value;
-         U_32 infoType = j9sig_info(crashedThread->gpInfo, J9PORT_SIG_CONTROL, J9PORT_SIG_CONTROL_PC, &name, &value);
-
-         if (J9PORT_SIG_VALUE_ADDRESS == infoType)
+         if (NULL != crashedThread->gpInfo)
             {
-            J9JITExceptionTable *metadata = jitConfig->jitGetExceptionTableFromPC(crashedThread, *reinterpret_cast<UDATA*>(value));
-            if (NULL != metadata)
+            // The stack walker may not be able to walk the stack if the crash did not happen on a transition frame. In
+            // such cases the stack walker will resume walking the stack on the last known valid point, which will be a
+            // transition frame further up the stack (ex. INT -> JIT transition frame). This will result in the stack
+            // walker potentially skipping some JIT methods on the backtrace. This is not desirable. Chances are high
+            // that the crash happen because of a miscompilation in the first JIT compiled method on the stack, so it
+            // is imperative that we recompile the method we actually crashed in.
+            const char *name;
+            void *value;
+            U_32 infoType = j9sig_info(crashedThread->gpInfo, J9PORT_SIG_CONTROL, J9PORT_SIG_CONTROL_PC, &name, &value);
+
+            if (J9PORT_SIG_VALUE_ADDRESS == infoType)
                {
-               auto *bodyInfo = reinterpret_cast<TR_PersistentJittedBodyInfo *>(metadata->bodyInfo);
-               if (NULL != bodyInfo)
+               J9JITExceptionTable *metadata = jitConfig->jitGetExceptionTableFromPC(crashedThread, *reinterpret_cast<UDATA*>(value));
+               if (NULL != metadata)
                   {
-                  jitDumpRecompileWithTracing(
-                     crashedThread,
-                     metadata->ramMethod,
-                     compInfo,
-                     bodyInfo->getHotness(),
-                     bodyInfo->getIsProfilingBody(),
-                     NULL,
-                     bodyInfo->getIsAotedBody(),
-                     bodyInfo->getStartPCAfterPreviousCompile(),
-                     jitdumpFile
-                  );
+                  auto *bodyInfo = reinterpret_cast<TR_PersistentJittedBodyInfo *>(metadata->bodyInfo);
+                  if (NULL != bodyInfo)
+                     {
+                     jitDumpRecompileWithTracing(
+                        crashedThread,
+                        metadata->ramMethod,
+                        compInfo,
+                        bodyInfo->getHotness(),
+                        bodyInfo->getIsProfilingBody(),
+                        NULL,
+                        bodyInfo->getIsAotedBody(),
+                        bodyInfo->getStartPCAfterPreviousCompile(),
+                        jitdumpFile
+                     );
+                     }
                   }
                }
             }
@@ -547,6 +551,8 @@ runJitdump(char *label, J9RASdumpContext *context, J9RASdumpAgent *agent)
    trfflush(jitdumpFile);
    trfclose(jitdumpFile);
 
+   recompilationThreadInfo->suspendCompilationThread();
+
    compInfo->getPersistentInfo()->setDisableFurtherCompilation(false);
 
    return OMR_ERROR_NONE;