Skip to content

Commit

Permalink
Protect against a rare invalid lock acquision attempt during etw proc…
Browse files Browse the repository at this point in the history
…essing 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: dotnet#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.
  • Loading branch information
kouvel committed Oct 16, 2019
1 parent ecfe3bc commit 16a4c45
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 6 deletions.
9 changes: 4 additions & 5 deletions src/vm/ceemain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1311,6 +1311,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();
}

Expand All @@ -1327,11 +1331,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.
Expand Down
8 changes: 7 additions & 1 deletion src/vm/prestub.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1197,7 +1197,13 @@ PrepareCodeConfig::JitOptimizationTier PrepareCodeConfig::GetJitOptimizationTier
_ASSERTE(methodDesc != nullptr);
_ASSERTE(config == nullptr || methodDesc == config->GetMethodDesc());

if (config != nullptr)
// The code inside this block 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 (config != nullptr && !g_fProcessDetach)
{
if (config->JitSwitchedToMinOpt())
{
Expand Down

0 comments on commit 16a4c45

Please sign in to comment.