-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
stackSize override is not correct value format #60762
Comments
Output from memory dump --- Usage Summary ---------------- RgnCount ----------- Total Size -------- %ofBusy %ofTotal !address -f:MEM_RESERVE,Stack .... This particular dump did result in oom in a bad place, starting new thread. 0:000> !pe 176c8030 0:000> !threadpool CPU utilization: 9% |
Yup, we should fix this. This is the relevant code in runtime where you can see that it expects a hex value: https://github.com/dotnet/runtime/blob/3d275542626ac6420269168ecbebe2bcb8e6ae54/src/coreclr/vm/threads.cpp#L2051C8-L2051C42 Good catch! |
Are you interested in opening a PR for this @clintwar? |
I can if no one else wants to :) |
This should be a relatively straightforward fix (minor code change, and a doc update here: https://github.com/dotnet/AspNetCore.Docs/blob/main/aspnetcore/host-and-deploy/iis/advanced.md), so if you're interested, we'd love to have you take it on. Let me know if you need any pointers or guidance along the way. Otherwise, if you'd prefer not to, I can take care of it--just let me know what works best for you. |
We should also separately update the comment in https://github.com/dotnet/runtime/blob/016b59002f2a2e276a8223a6bae91222b733d65f/src/coreclr/inc/clrconfigvalues.h#L484 to mention base16 |
[ASP.NET Core incorrectly assumed the `DefaultStackSize` value was parsed as an integer](dotnet/aspnetcore#60762) (most values are? But that's not a discussion for this PR). But the value is parsed as a base16 number. Just updating the comment here to explicitly mention that.
PR for fix is here |
Is there an existing issue for this?
Describe the bug
When aspnetcore for in process hosting overrides the default stack size for new threads on the threadpool (https://learn.microsoft.com/en-us/aspnet/core/host-and-deploy/iis/advanced?view=aspnetcore-9.0) the value is incorrectly defaulted.
aspnetcore/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/InProcessOptions.cpp
Line 68 in e05d6f6
3be11f6
The internals of coreclr is looking for the value as base16. The default value of 1048576 is treating as base16. This is not 1 MB. This is 17 MB.
If you take a memory dump of an inprocess hosted .net core 8 app and view !address -f:MEM_RESERVE,Stack you will notice reserved memory for stack around this 0x1048576 size. In 32bit world this is a bad situation as you will run out of memory if your process needs to create many threads.
In the learning document it indicates how to set it to 2MB however this actually would set it to 34~ MB.
Expected Behavior
Actually set stack to a reasonable size similar to what .net core does by default 1.5 MB for 32bit app and 4MB for x64.
Steps To Reproduce
If you take a memory dump of an inprocess hosted .net core 8 app and view !address -f:MEM_RESERVE,Stack you will notice reserved memory for stack around this 0x1048576 size. In 32bit world this is a bad situation as you will run out of memory if your process needs to create many threads.
Exceptions (if any)
No response
.NET Version
No response
Anything else?
No response
The text was updated successfully, but these errors were encountered: