-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Add a config switch to fully disable hot-reload #123744
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
src/libraries/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a configuration switch to fully disable hot-reload by introducing a new "none" value for the DOTNET_MODIFIABLE_ASSEMBLIES environment variable. This addresses issue #78540 where hot-reload couldn't be disabled when it was causing bugs, particularly when a debugger was attached.
Changes:
- Introduced a tri-state enum (UNSET, NONE, DEBUG) to distinguish between "not configured", "explicitly disabled", and "explicitly enabled"
- Updated both Mono and CoreCLR implementations to recognize the "none" value
- Added test coverage for the new disable functionality
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/mono/mono/metadata/metadata-update.h | Updated enum to add UNSET/NONE/DEBUG states with new numbering |
| src/mono/mono/component/hot_reload.c | Added "none" value handling and updated logic (contains critical bug) |
| src/mono/mono/component/hot_reload-stub.c | Updated stub to use UNSET instead of NONE for uninitialized state |
| src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs | Minor whitespace fix |
| src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs | Added constant for disabled value |
| src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs | Added test for disable functionality |
| src/coreclr/vm/eeconfig.h | Updated enum definition and method signatures for new tri-state |
| src/coreclr/vm/eeconfig.cpp | Added parsing logic for "none" value |
| src/coreclr/vm/ceeload.cpp | Updated condition to use new enum |
| src/coreclr/vm/assemblynative.cpp | Updated logic to check for explicit disable |
src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs
Outdated
Show resolved
Hide resolved
|
Could you please check that this env variable can be used to fully disable hot reload under VS? (#78218 (comment) is the motivating feedback.) |
|
cc @tmat |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
I don't think there's a high bar. It probably would have been clearer if I had said I don't think what @barosiak posted in VS was a big deal. It would require someone in VS to make changes. |
It works, I was setting it incorrectly. To sum up, there are two main ways to disable hot-reload when debugging in VS. Using UI:
Using launchSettings.json file:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/mono/mono/component/hot_reload.c:361
- The documentation comment should be updated to reflect the new "none" value. It currently states "all other values are ignored and metadata updates are disabled" but "none" is now a recognized value that explicitly disables metadata updates. Consider updating to: ""debug" means debuggable assemblies are modifiable, "none" explicitly disables metadata updates, and all other values are ignored and metadata updates are disabled."
hot_reload_update_enabled (int *modifiable_assemblies_out)
{
static gboolean inited = FALSE;
static int modifiable = MONO_MODIFIABLE_ASSM_UNSET;
gboolean result = FALSE;
if (!inited) {
modifiable = hot_reload_update_enabled_slow_check (NULL);
inited = TRUE;
result = (modifiable == MONO_MODIFIABLE_ASSM_DEBUG);
if (result) {
mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Metadata update enabled for debuggable assemblies");
}
}
if (modifiable_assemblies_out)
*modifiable_assemblies_out = modifiable;
return result;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
src/mono/mono/component/hot_reload.c:360
hot_reload_update_enabledreturnsresult, butresultis only computed on the first call (insideif (!inited)). After initialization, subsequent calls will always returnFALSEeven ifmodifiableisMONO_MODIFIABLE_ASSM_DEBUG, which can incorrectly disable hot-reload behavior (e.g., callers likemono_metadata_update_enabled/inlining checks). Compute the return value from the cachedmodifiableon every call (or cache a separate static boolean).
hot_reload_update_enabled (int *modifiable_assemblies_out)
{
static gboolean inited = FALSE;
static int modifiable = MONO_MODIFIABLE_ASSM_UNSET;
gboolean result = FALSE;
if (!inited) {
modifiable = hot_reload_update_enabled_slow_check (NULL);
inited = TRUE;
result = (modifiable == MONO_MODIFIABLE_ASSM_DEBUG);
if (result) {
mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Metadata update enabled for debuggable assemblies");
}
}
if (modifiable_assemblies_out)
*modifiable_assemblies_out = modifiable;
return result;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Adds a config switch that unconditionally disables metadata updater.
Fixes #78540