Skip to content

Commit

Permalink
Fix a potential shutdown hang with Environment.Exit() on Windows de…
Browse files Browse the repository at this point in the history
…pending on timing with GCs

- On Windows when a thread calls `ExitProcess`, the `TlsDestructionMonitor` for the thread appears to be destructed after all other threads in the process are torn down. It's possible for a GC to be in progress during that time, and the thread cleanup code in `TlsDestructionMonitor` tries to enter cooperative GC mode to fix the frame pointer, leading to a hang. Fixed by deactivating the `TlsDestructionMonitor` for the thread before calling `ExitProcess`.
- Also disabled the relevant test due to a different issue dotnet#83658 occurring in the same test on multiple platforms/architectures that is not understood yet.

Fixes dotnet#84006
  • Loading branch information
kouvel committed Sep 7, 2023
1 parent 617f2e5 commit 4eaf6f7
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 3 deletions.
10 changes: 10 additions & 0 deletions src/coreclr/vm/ceemain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1727,6 +1727,11 @@ struct TlsDestructionMonitor
m_activated = true;
}

void Deactivate()
{
m_activated = false;
}

~TlsDestructionMonitor()
{
if (m_activated)
Expand Down Expand Up @@ -1768,6 +1773,11 @@ void EnsureTlsDestructionMonitor()
tls_destructionMonitor.Activate();
}

void DeactivateTlsDestructionMonitor()
{
tls_destructionMonitor.Deactivate();
}

#ifdef DEBUGGING_SUPPORTED
//
// InitializeDebugger initialized the Runtime-side COM+ Debugging Services
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/vm/ceemain.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ void ForceEEShutdown(ShutdownCompleteAction sca = SCA_ExitProcessWhenShutdownCom
void ThreadDetaching();

void EnsureTlsDestructionMonitor();
void DeactivateTlsDestructionMonitor();

void SetLatchedExitCode (INT32 code);
INT32 GetLatchedExitCode (void);
Expand Down
7 changes: 7 additions & 0 deletions src/coreclr/vm/eepolicy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,13 @@ void SafeExitProcess(UINT exitCode, ShutdownCompleteAction sca = SCA_ExitProcess
// disabled because if we fault in this code path we will trigger our Watson code
CONTRACT_VIOLATION(ThrowsViolation);

// The TlsDestructionMonitor for this thread would likely be destructed at some point after ExitProcess is called. On
// Windows, this happens after all other threads in the process are torn down, and may occur while a GC is in progress.
// The thread cleanup code in TlsDestructionMonitor may try to enter cooperative GC mode to fix the frame pointer and
// wait for the pending GC to complete, leading to a hang. Since the process is being exited, deactivate the
// TlsDestructionMonitor for this thread before calling ExitProcess.
DeactivateTlsDestructionMonitor();

ExitProcess(exitCode);
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/tests/issues.targets
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@
<ExcludeList Include="$(XunitTestBinBase)/baseservices/mono/runningmono/*">
<Issue>This test is to verify we are running mono, and therefore only makes sense on mono.</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/baseservices/threading/regressions/2164/foreground-shutdown/*">
<Issue>https://github.com/dotnet/runtime/issues/83658</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/GC/Coverage/271010/**">
<Issue>https://github.com/dotnet/runtime/issues/5933</Issue>
</ExcludeList>
Expand Down Expand Up @@ -251,9 +254,6 @@

<!-- Windows all architecture excludes -->
<ItemGroup Condition="'$(XunitTestBinBase)' != '' and '$(TargetsWindows)' == 'true' and '$(RuntimeFlavor)' == 'coreclr'">
<ExcludeList Include="$(XunitTestBinBase)/baseservices/threading/regressions/2164/foreground-shutdown/*">
<Issue>https://github.com/dotnet/runtime/issues/84006</Issue>
</ExcludeList>
</ItemGroup>

<!-- Windows x64 specific excludes -->
Expand Down

0 comments on commit 4eaf6f7

Please sign in to comment.