Skip to content
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

Garbage collection method is not manageable by environment variables in .net 6 #61674

Closed
erexo opened this issue Nov 16, 2021 · 11 comments · Fixed by #61950
Closed

Garbage collection method is not manageable by environment variables in .net 6 #61674

erexo opened this issue Nov 16, 2021 · 11 comments · Fixed by #61950
Assignees
Milestone

Comments

@erexo
Copy link

erexo commented Nov 16, 2021

Describe the bug

Documentation mentions that user is able to change the default Server garbage collection to Workstation garbage collection via either COMPlus_gcServer (pre .net6) or DOTNET_gcServer (in .net6) Environment variable.
It was working (the COMPlus env) in .net5, but after project upgrade to .net6, both methods seems to be doing nothing, gcServer is always set to true.
MSBuild property is still functioning, we can add /p:ServerGarbageCollection=false to the build parameters and we will achieve exactly that.
The problem seems to be asp.net related, Console application (with default gc set to Workstation) can be overridden by both of those env variables to Server.

To Reproduce

  • Create webapi template dotnet new webapi in .net6
  • Run that project with any of given env variables (COMPlus_gcServer=0 or DOTNET_gcServer=0)
  • Check used garbage collection method (ie. print System.Runtime.GCSettings.IsServerGC on startup)

Further technical details

  • ASP.NET Core version: 6.0
  • The IDE: VS Code 1.62.2-1636665017_amd64
dotnet --info Output
NET SDK (reflecting any global.json):
 Version:   6.0.100
 Commit:    9e8b04bbff

Runtime Environment:
 OS Name:     ubuntu
 OS Version:  20.04
 OS Platform: Linux
 RID:         ubuntu.20.04-x64
 Base Path:   /usr/share/dotnet/sdk/6.0.100/

Host (useful for support):
  Version: 6.0.0
  Commit:  4822e3c3aa
@javiercn javiercn transferred this issue from dotnet/aspnetcore Nov 16, 2021
@javiercn javiercn transferred this issue from dotnet/core Nov 16, 2021
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Nov 16, 2021
@ghost
Copy link

ghost commented Nov 16, 2021

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

Issue Details

Describe the bug

Documentation mentions that user is able to change the default Server garbage collection to Workstation garbage collection via either COMPlus_gcServer (pre .net6) or DOTNET_gcServer (in .net6) Environment variable.
It was working (the COMPlus env) in .net5, but after project upgrade to .net6, both methods seems to be doing nothing, gcServer is always set to true.
MSBuild property is still functioning, we can add /p:ServerGarbageCollection=false to the build parameters and we will achieve exactly that.
The problem seems to be asp.net related, Console application (with default gc set to Workstation) can be overridden by both of those env variables to Server.

To Reproduce

  • Create webapi template dotnet new webapi in .net6
  • Run that project with any of given env variables (COMPlus_gcServer=0 or DOTNET_gcServer=0)
  • Check used garbage collection method (ie. print System.Runtime.GCSettings.IsServerGC on startup)

Further technical details

  • ASP.NET Core version: 6.0
  • The IDE: VS Code 1.62.2-1636665017_amd64
dotnet --info Output
NET SDK (reflecting any global.json):
 Version:   6.0.100
 Commit:    9e8b04bbff

Runtime Environment:
 OS Name:     ubuntu
 OS Version:  20.04
 OS Platform: Linux
 RID:         ubuntu.20.04-x64
 Base Path:   /usr/share/dotnet/sdk/6.0.100/

Host (useful for support):
  Version: 6.0.0
  Commit:  4822e3c3aa
Author: Erexo
Assignees: -
Labels:

area-GC-coreclr, untriaged

Milestone: -

@davidfowl
Copy link
Member

I can reproduce this one. I'm wondering what happens if you have both the environment variable and the value set in runtimeconfig.json

@mangod9
Copy link
Member

mangod9 commented Nov 17, 2021

so are you observing that the value in runtimeconfig.json is now overriding what is set in the env. variable?

@cshung
Copy link
Member

cshung commented Nov 17, 2021

I can reproduce this one. I'm wondering what happens if you have both the environment variable and the value set in runtimeconfig.json

In general, the design is that the environment variable should override the settings in runtimeconfig.json.

The GCToEEInterface::GetBooleanConfigValue function do just that.

Unfortunately, the gcServer is a special case because we need to read that configuration before we initialize the heap, so the reading of the flag is outside of this function. The g_heap_type is set in InitializeStartupFlags in ceemain.cpp.

The startupFlags are set in SetStartupFlags which is setting a value computed earlier in InitializeStartupFlags in unixinterface.cpp, that function uses Configuration::GetKnobBooleanValue which also make sure we read environment variable first.

Not sure what is going wrong - @davidfowl, can you help us to debug what is happening in these two functions? Maybe we can find the bug there.

@mangod9
Copy link
Member

mangod9 commented Nov 17, 2021

The startupFlags are set in SetStartupFlags which is setting a value computed earlier in InitializeStartupFlags in unixinterface.cpp

So does this only repro on linux or windows too?

@davidfowl
Copy link
Member

Windows as well.

@cshung
Copy link
Member

cshung commented Nov 18, 2021

I have found the culprit of this bug, it appears to be this line:

// Ignore the default value even if it's set explicitly.

So first thing first, while confusing, unixinterface.cpp is actually involved in Windows as well. We really should rename it.

Following the analysis above, when the runtime startup, it uses Configuration::GetKnobBooleanValue to determine whether or not to use server GC, it is obvious that the intent of that function is that the environment variable can override the runtimeconfig.json

bool Configuration::GetKnobBooleanValue(LPCWSTR name, const CLRConfig::ConfigDWORDInfo& dwordInfo)
{
bool returnedDefaultValue;
DWORD legacyValue = CLRConfig::GetConfigValue(dwordInfo, &returnedDefaultValue);
if (!returnedDefaultValue)
{
return (legacyValue != 0);
}
LPCWSTR knobValue = GetConfigurationValue(name);
if (knobValue != nullptr)
{
return (wcscmp(knobValue, W("true")) == 0);
}

In the majority of the cases, the code is fine, because there is really no point for someone to specify an environment variable with value that is exactly the default value (for the runtime).

Unfortunately, if the user wanted to disable server GC in WebAPI, then the user would set the value to be 0, which also happened to be the default value for the runtime. Now the culprit line of CLRConfig::GetConfigValue would determine while the environment variable is already set, but it is setting to a default value, so ignoring the fact that it is set and set *isDefault to true anyway. As such, Configuration::GetKnobBooleanValue would move on and read the runtimeconfig.json anyway, even when the environment variable is set.

Apparently, that behavior is not what we want in this issue.

Given the analysis, it isn't difficult to fix this one, but I wonder why one put in that specific ignore there. In particular, I don't want to regress something else. @AaronRobinsonMSFT, do you know why we want to specifically ignore the cases where the user explicitly specified the environment variable with exactly the default value?

@AaronRobinsonMSFT
Copy link
Member

@AaronRobinsonMSFT, do you know why we want to specifically ignore the cases where the user explicitly specified the environment variable with exactly the default value?

If I recall that logic was previously there from long back https://github.com/dotnet/coreclr/blob/1d483901530c2d5b8a943fab932e8c184d2c5f36/src/utilcode/clrconfig.cpp#L295-L300

I tried to maintain the identical behavior but it seems there was a branch in the GC case that I missed. The old logic was so intertwined with registry access I'm not sure which part has changed though.

@AaronRobinsonMSFT
Copy link
Member

I would fix this outside of the clrconfig.cpp file. One can use the following to query if it is set.

BOOL CLRConfig::IsConfigOptionSpecified(LPCWSTR name)

@mangod9
Copy link
Member

mangod9 commented Nov 18, 2021

Thanks for investigating @cshung. We should make a fix in 7 and will possibly need to port to 6. Is the workaround to just remove the setting from runtimeconfig.json then?

@AaronRobinsonMSFT AaronRobinsonMSFT self-assigned this Nov 22, 2021
@AaronRobinsonMSFT AaronRobinsonMSFT removed the untriaged New issue has not been triaged by the area owner label Nov 22, 2021
@AaronRobinsonMSFT AaronRobinsonMSFT added this to the .NET 7.0 milestone Nov 22, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Nov 23, 2021
@karelz karelz modified the milestones: .NET 7.0, 7.0.0 Nov 23, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Nov 23, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Dec 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants