Remove preventing EH and entering EE at shutdown #100293
Conversation
Historically, the runtime was trying to block handling exceptions, entering EE and other stuff during shutdown of the runtime. We have already removed most of that in the past, since it has been a deadlock farm. While debugging a customer issue, I have found that we are still preventing some things to happen during shutdown. We stop handling exceptions at all and we also prevent entering EE at few places. This change removes these, as the plan we've been on is to keep the runtime running until the OS itself stops the process. In the customer's case, the problem was that during a machine shutdown, .NET runtime was generating watson dump due to an unhandled exception stemming from a network socket related code that would otherwise be handled.
|
@jkotas I am planning to remove the |
|
Assume this will work with both current and legacy exception handling? |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
|
||
| // No longer process exceptions | ||
| g_fNoExceptions = true; | ||
|
|
||
| // <TODO>@TODO: This does things which shouldn't occur in part 2. Namely, | ||
| // calling managed dll main callbacks (AppDomain::SignalProcessDetach), and | ||
| // RemoveAppDomainFromIPC. |
There was a problem hiding this comment.
I am looking at what we do below and there seems to be more instances of questionable cleanup:
- CLRRemoveVectoredHandlers
- StubManager::TerminateStubManagers - it should just flush the debug-only log, but it should not be destroying the Crsts
- Interpreter::Terminate
There was a problem hiding this comment.
The Disassembler::StaticClose(); frees the disassembler shared library, it also sounds like something we should not do.
There was a problem hiding this comment.
StubManager::TerminateStubManagers - it actually doesn't do anything interesting. The whole log is just a SString instance that is never accessed in any way - there is a StubManager::DbgGetLog method, but nothing calls it.
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/backport to release/8.0-staging |
|
Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/8607836627 |
|
@janvorli backporting to release/8.0-staging failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Remove preventing EH and entering EE at shutdown
Using index info to reconstruct a base tree...
M src/coreclr/vm/ceemain.cpp
M src/coreclr/vm/crst.h
M src/coreclr/vm/eepolicy.cpp
M src/coreclr/vm/excep.cpp
M src/coreclr/vm/i386/excepx86.cpp
M src/coreclr/vm/runtimecallablewrapper.cpp
M src/coreclr/vm/threads.cpp
M src/coreclr/vm/vars.cpp
M src/coreclr/vm/vars.hpp
Falling back to patching base and 3-way merge...
Auto-merging src/coreclr/vm/vars.hpp
Auto-merging src/coreclr/vm/vars.cpp
Auto-merging src/coreclr/vm/threads.cpp
Auto-merging src/coreclr/vm/runtimecallablewrapper.cpp
Auto-merging src/coreclr/vm/i386/excepx86.cpp
Auto-merging src/coreclr/vm/excep.cpp
CONFLICT (content): Merge conflict in src/coreclr/vm/excep.cpp
Auto-merging src/coreclr/vm/eepolicy.cpp
Auto-merging src/coreclr/vm/crst.h
Auto-merging src/coreclr/vm/ceemain.cpp
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Remove preventing EH and entering EE at shutdown
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
|
@janvorli an error occurred while backporting to release/8.0-staging, please check the run log for details! Error: git am failed, most likely due to a merge conflict. |
Backport of dotnet#100293 to release/8.0
Partial backport of dotnet#100293 to release/8.0
Partial backport of #100293 to release/8.0
Historically, the runtime was trying to block handling exceptions, entering EE and other stuff during shutdown of the runtime. We have already removed most of that in the past, since it has been a deadlock farm. While debugging a customer issue, I have found that we are still preventing some things to happen during shutdown. We stop handling exceptions at all and we also prevent entering EE at few places. This change removes these, as the plan we've been on is to keep the runtime running until the OS itself stops the process. In the customer's case, the problem was that during a machine shutdown, .NET runtime was generating watson dump due to an unhandled exception stemming from a network socket related code that would otherwise be handled.