Enable new exception handling by default #98570
Conversation
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
src/coreclr/inc/clrconfigvalues.h
Outdated
| @@ -259,7 +259,7 @@ RETAIL_CONFIG_DWORD_INFO(UNSUPPORTED_legacyCorruptedStateExceptionsPolicy, W("le | |||
| CONFIG_DWORD_INFO(INTERNAL_SuppressLostExceptionTypeAssert, W("SuppressLostExceptionTypeAssert"), 0, "") | |||
| RETAIL_CONFIG_DWORD_INFO(INTERNAL_UseEntryPointFilter, W("UseEntryPointFilter"), 0, "") | |||
| RETAIL_CONFIG_DWORD_INFO(INTERNAL_Corhost_Swallow_Uncaught_Exceptions, W("Corhost_Swallow_Uncaught_Exceptions"), 0, "") | |||
| RETAIL_CONFIG_DWORD_INFO(EXTERNAL_EnableNewExceptionHandling, W("EnableNewExceptionHandling"), 0, "Enable new exception handling."); | |||
| RETAIL_CONFIG_DWORD_INFO(EXTERNAL_EnableNewExceptionHandling, W("EnableNewExceptionHandling"), 1, "Enable new exception handling."); | |||
There was a problem hiding this comment.
is there a way to configure this via runtimeconfig as well? We had some asks for having even W^X configurable through that as well.
There was a problem hiding this comment.
No, let me figure out how to do that.
There was a problem hiding this comment.
@mangod9 I've just added a commit that adds "System.Runtime.EnableNewExceptionHandling" setting that can control that from runtime.config.
There was a problem hiding this comment.
Could we not call this "New" exception handling? I've worked on way too many features where the "New" thing was 15 years old and its name confused everyone at all times.
There was a problem hiding this comment.
Ah, I suppose thats the name throughout the product at this point. Oh well. I suppose New it is. Sigh.
There was a problem hiding this comment.
@davidwrighton yeah, I know. I couldn't come up with anything better over the time. If you have a better idea, I'd be happy to rename it.
There was a problem hiding this comment.
Just a quick idea - would using DOTNET_ExceptionHandlingMode and System.Runtime.ExceptionHandlingMode with a numeric value feel better? We would use 0 for the old EH, 1 for the new EH and if we ever moved it to something else, then we could keep incrementing it.
There was a problem hiding this comment.
I assume that this setting is temporary just in case somebody gets blocked and we are going to delete it at some point. Do we have an idea when we are going to delete it?
There was a problem hiding this comment.
I couldn't come up with anything better over the time.
A better name would be LegacyExceptionHandling with opposite meaning. It makes it clear that it is something that one should not use and that is likely going to disappear at some point.
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Looks like there are PALTest failures? |
Yes, break from another PAL refactoring, see #98574 |
|
I don't know what happened. Huge number of tests are failing. I had a test PR before with all the changes and all tests were passing. I must have merged something incorrectly. I need to investigate it. |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
janvorli
force-pushed
the
enable-new-eh-by-default
branch
from
5950a3b
to
3f53147
Compare
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This change just flips the config knob to enable the new exception handling by default.
From the EnableNewExceptionHandling to LegacyExceptionHandling
janvorli
force-pushed
the
enable-new-eh-by-default
branch
from
3f53147
to
918ff7c
Compare
This change just flips the config knob to enable the new exception handling by default.