[3.1] Protect against a rare invalid lock acquision attempt during etw processing...
authorKoundinya Veluri <kouvel@users.noreply.github.com>
Thu, 24 Oct 2019 20:06:58 +0000 (13:06 -0700)
committerGitHub <noreply@github.com>
Thu, 24 Oct 2019 20:06:58 +0000 (13:06 -0700)
* Protect against a rare invalid lock acquision attempt during etw processing during abrupt shutdown

Targeted and partial fix for https://github.com/dotnet/coreclr/issues/27129
- This is not a generic fix for the issue above, it is only a very targeted fix for an issue seen (a new issue introduced in 3.x). For a generic fix and more details, see the fix in 5.0: https://github.com/dotnet/coreclr/pull/27238.
- This change avoids taking a lock during process detach - a point in time when all other threads have already been abruptly shut down by the OS and locks may have been orphaned.
- The issue leads to a hang during shutdown when ETW tracing is enabled and the .NET process being traced begins the shutdown sequence at an unfortunate time - this is a probably rare timing issue. It would take the shutdown sequence to begin at just the point when a thread holds a particular lock and is terminated by the OS while holding the lock, then the OS sends the process detach event to the CLR, work during which then tries to acquire the lock and cannot because it is orphaned.
- The generic fix has broader consequences and is unlikely to be a reasonable change to make so late in the cycle, such a change needs some bake time and feedback. Hence this targeted fix for 3.x.

* Report tier as unknown when it cannot be determined

* Return unknown only on process detach

src/vm/ceemain.cpp
src/vm/prestub.cpp

index 1c5d4f8..8a5bb4f 100644 (file)
@@ -1323,6 +1323,10 @@ void STDMETHODCALLTYPE EEShutDownHelper(BOOL fIsDllUnloading)
 
     if (fIsDllUnloading)
     {
+        // The process is detaching, so set the global state.
+        // This is used to get around FreeLibrary problems.
+        g_fProcessDetach = true;
+
         ETW::EnumerationLog::ProcessShutdown();
     }
 
@@ -1339,11 +1343,6 @@ void STDMETHODCALLTYPE EEShutDownHelper(BOOL fIsDllUnloading)
     Thread * pThisThread = GetThread();
 #endif
 
-    // If the process is detaching then set the global state.
-    // This is used to get around FreeLibrary problems.
-    if(fIsDllUnloading)
-        g_fProcessDetach = true;
-
     if (IsDbgHelperSpecialThread())
     {
         // Our debugger helper thread does not allow Thread object to be set up.
index b76a018..a8d3e42 100644 (file)
@@ -1197,6 +1197,17 @@ PrepareCodeConfig::JitOptimizationTier PrepareCodeConfig::GetJitOptimizationTier
     _ASSERTE(methodDesc != nullptr);
     _ASSERTE(config == nullptr || methodDesc == config->GetMethodDesc());
 
+    // The code below may take one or more locks. If process detach has already occurred, then other threads would have already
+    // been abruptly terminated, which may have left locks orphaned, so it would not be feasible to attempt taking a lock on
+    // process detach. This function is called from ETW-related code on shutdown where unfortunately it processes and sends
+    // rundown events upon process detach. If process detach has already occurred, then resort to best-effort behavior that may
+    // return inaccurate information. This is a temporary and targeted fix for a clear problem, and should not be used
+    // long-term.
+    if (g_fProcessDetach)
+    {
+        return JitOptimizationTier::Unknown;
+    }
+
     if (config != nullptr)
     {
         if (config->JitSwitchedToMinOpt())