-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
In conservative mode RedirectedThreadFrame should report registers to GC. #48923
Conversation
Tagging subscribers to this area: @dotnet/gc Issue DetailsWhen thread is redirected, its registers are in a context structure that is not on the stack.
|
was this found via a stress test or a CR? |
If you run We do not generally test conservative mode. That is mostly for platform bring-up purposes. Or when investigating bugs, it can be enabled to rule out issues with gc root tracking (since it does not use any of that). It is supposed to work though. I guess - who uses it, fixes it. |
src/coreclr/gc/gcconfig.h
Outdated
@@ -65,7 +65,7 @@ class GCConfigStringHolder | |||
#define GC_CONFIGURATION_KEYS \ | |||
BOOL_CONFIG (ServerGC, "gcServer", NULL, false, "Whether we should be using Server GC") \ | |||
BOOL_CONFIG (ConcurrentGC, "gcConcurrent", NULL, true, "Whether we should be using Concurrent GC") \ | |||
BOOL_CONFIG (ConservativeGC, "gcConservative", NULL, false, "Enables/Disables conservative GC") \ | |||
BOOL_CONFIG (ConservativeGC, "gcConservative", NULL, true, "Enables/Disables conservative GC") \ |
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.
I temporarily enabled conservative GC by default - for a CI run. That is not a part of the fix, just for test coverage. I will undo config changes.
The CI run with conservative mode enabled by default has some failures. There are no random crashes anymore though. I also ran |
@jkotas @davidwrighton - a fairly small change. Could you take a look? The failures in tests are expected. They basically show that conservative mode works as expected. |
Thanks!! |
Hello @VSadov! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
When a thread is redirected, its registers are stored in a context structure that is not on the stack.
The registers may contain managed roots and not reporting them to GC can cause crashes when running in conservative mode.