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

Migration from .NET Framework 4.7.2 to .NET 8 results in StackOverflowException due to reduced stack size #96347

Open
LunarWhisper opened this issue Dec 28, 2023 · 19 comments
Labels
area-VM-coreclr needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration tenet-performance Performance related issue
Milestone

Comments

@LunarWhisper
Copy link

LunarWhisper commented Dec 28, 2023

Description

In the process of migrating a huge Enterprise solution to .NET8, we encountered an unexpected regression:
The default stack size for the main and secondary threads has been reduced from 4 MB (x64) to 1.5 MB.

This resulted in a StackOverflowException in a RestAPI service using AutoMapper, custom serialization logic, and building dynamic methods.

The DefaultStackSize environment variable is internal and cannot be configured from an external .json config file.

RETAIL_CONFIG_DWORD_INFO(INTERNAL_DefaultStackSize, W("DefaultStackSize"), 0, "Stack size to use for new VM threads when thread is created with default stack size (dwStackSize == 0).")

Configuration

  • .NET 8 (SDK 8.0.100)
  • Windows 10 and Ubuntu 22 (64-bit)
  • x64 and AnyCPU (Prefer 32-bit is disabled)

Regression?

Yes, this is regression from .NET Framework where the default stack size was 4 MB for 64-bit applications.
I expect 4 MB and this to work the same for the .NET Framework and .NET8+ on any OS, when building for x64 and AnyCPU platforms.

Data

// This line of code will fail if you try to run .NET8 (AnyCPU/x64) application but will work for .NET Framework (x64)
var numbers = stackalloc byte[2 * 1024 * 1024];

Analysis

An article about internals of threads:
https://learn.microsoft.com/en-us/archive/blogs/markrussinovich/pushing-the-limits-of-windows-processes-and-threads

Current .NET stack sizes:
image

@LunarWhisper LunarWhisper added the tenet-performance Performance related issue label Dec 28, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Dec 28, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Dec 28, 2023
@EgorBo EgorBo added area-VM-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Dec 28, 2023
@EgorBo
Copy link
Member

EgorBo commented Dec 28, 2023

cc @janvorli

@MichalPetryka
Copy link
Contributor

AFAIR .NET 8 mostly defaults to the OS default stack sizes today, but I recall there being some talks in the past about having a guaranteed size for all platforms.

If you control the thread creation, you can change the stack limit in the managed code with the Thread constructor.

@EgorBo
Copy link
Member

EgorBo commented Dec 28, 2023

AFAIR .NET 8 mostly defaults to the OS default stack sizes today, but I recall there being some talks in the past about having a guaranteed size for all platforms.

If you control the thread creation, you can change the stack limit in the managed code with the Thread constructor.

1.5MB is not a Windows OS default and you can't control it for the main thread (only via env vars).

@Clockwork-Muse
Copy link
Contributor

// This line of code will fail if you try to run .NET8 (AnyCPU/x64) application but will work for .NET Framework (x64)
var numbers = stackalloc byte[2 * 1024 * 1024];

Sure, it might, but I hope you're not really trying to stackalloc a 2MB chunk? If you need a chunk of memory that large, a memory pool is likely a better option. Otherwise, it's trivial to find a number that would cause even the .NetF stack to overflow.

@EgorBo
Copy link
Member

EgorBo commented Dec 28, 2023

Sure, it might, but I hope you're not really trying to stackalloc a 2MB chunk?

it's not the only way to get an SO. We still have an open issue somwhere where Roslyn runs out of stack compiling a C# program. I think bumping main thread's default stack size AND/OR exposing DefaultStackSize option as a runtime-config/msbuild property does make sense.

@janvorli
Copy link
Member

@jkotas do you happen to remember why we have opted for 1.5MB large main thread stack size for both 32 and 64 bit code instead of the 1MB / 4MB that .NET framework was using?

@janvorli
Copy link
Member

exposing DefaultStackSize option as a runtime-config/msbuild property

That would not work for the main thread - the main thread gets its stack size from the app .exe file header (Windows OS reads it from the PE file and sets it accordingly) and it is embedded in that header when we compile the dotnet.exe / apphost.exe.

@EgorBo
Copy link
Member

EgorBo commented Dec 28, 2023

exposing DefaultStackSize option as a runtime-config/msbuild property

That would not work for the main thread - the main thread gets its stack size from the app .exe file header (Windows OS reads it from the PE file and sets it accordingly) and it is embedded in that header when we compile the dotnet.exe / apphost.exe.

Ah, good point

@MichalPetryka
Copy link
Contributor

I still think that .NET should share a minimum of like 2MB or 4MB for all platforms.

@jkotas
Copy link
Member

jkotas commented Dec 28, 2023

@jkotas do you happen to remember why we have opted for 1.5MB large main thread stack size for both 32 and 64 bit code

git blame says that the 1.5MB value was introduced in dotnet/coreclr#1144 as a workaround for stackoverflows in debug builds of the runtime. It looks like completely accidental change. Prior to this change, we have been using the Windows native toolset default before that is 1MB.

@stephentoub
Copy link
Member

It looks like completely accidental change

Seems like it was the result of this suggestion:
dotnet/coreclr#1144 (comment)

@LunarWhisper
Copy link
Author

@janvorli any news?

The max stack size is a perf trade-off. The larger max stack size, the more (unused) memory is potentially committed to stacks. Growing max stack size is potentially replacing stack overflow with out of memory.

If there is no silver bullet, please give us the option to configure this via app.config/runtime.config/etc. per application because now this is the regression and there is no way to fix it in prod. :(

@jkotas
Copy link
Member

jkotas commented Feb 17, 2024

You can workaround this for now by running editbin.exe yourapp.exe /STACK:0x400000 on your app.

editbin is part of C++ workload in Visual Studio. It is on a path like C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.39.33519\bin\Hostx64\x64\editbin.exe.

@LunarWhisper
Copy link
Author

editbin.exe

@jkotas unfortunately, this works in a very limited way:

Win: editbin.exe yourapp.exe /STACK:4194304  // 4mb
Win: editbin.exe yourapp.dll /STACK:4194304  // 1.5mb
Lin: editbin.exe yourapp.dll /STACK:4194304  // 8mb

This isn't quite the stack size control we expect. :)

The max stack size is a perf trade-off.
Also, when developing multi-platform applications, we expect them to behave the same on each platform.
You don't need performance if your application crashes. Otherwise, this significantly increases the cost of testing.

Therefore, the ability to explicitly specify the stack size of the main thread and threads in the thread pool is really necessary so that the application works the same, and we can use the AnyCPU platform (at least for now it is the default for all created projects in the IDE).

@jkotas
Copy link
Member

jkotas commented Mar 6, 2024

If we enable the main thread stack size to be configurable, it will require your app to be launched via platform-specific apphost (ie yourapp.exe on Windows, yourapp on Linux). dotnet yourapp.dll won't work. This is OS limitation, outside of .NET runtime control. In other words, it would behave just like your test with editbin.

at least for now it is the default for all created projects in the IDE

The current default for most projects is Framework-dependent Executable. The build will produce the yourapp.exe on Windows, the Visual Studio debugger is going to use yourapp.exe to start you app, etc. (There is still yourapp.dll in the bin directory, but it is not meant to be the primary entrypoint for your app.)

@janvorli
Copy link
Member

janvorli commented Mar 6, 2024

It is also worth noting that the editbin works for Windows binaries only. On Linux, you can influence the main thread stack size using the "ulimit -s" command before launching your executable.

@LunarWhisper
Copy link
Author

dotnet yourapp.dll won't work

In this case, I hope that you can return the 4MB stack and make it the default for all configurations in Windows.
It seems to me that this is the minimum necessary to avoid catching errors in runtime after migrating from .NET Framework to .NET in enterprise solutions.

It is also worth noting that the editbin works for Windows binaries only. On Linux, you can influence the main thread stack size using the "ulimit -s" command before launching your executable.

Well, we haven't had to reduce the stack size yet, and 8 MB on Linux is definitely enough for us.
The main point of this Issue is that the behavior has changed relative to the .NET Framework, and this is very unexpected and leads to unpleasant bugs.

@jkotas
Copy link
Member

jkotas commented Mar 6, 2024

I hope that you can return the 4MB stack and make it the default for all configurations in Windows.

Maintaining bug-for-bug .NET Framework compatibility is not a strong argument at this point. There are number of scenarios where the stack consumption can be very different from how .NET Framework behaved. Also, we have 1.5MB default in .NET Core/5+ on Windows for close to 10 years and it worked fine so far.

Could you please tell us more about what your app does that it is hitting the 1.5MB limit?

Why is the launching your app via platform specific yourapp.exe not an option for you? In general, launching your app via platform specific apphost (e.g. yourapp.exe )gives you better control over the exact behavior, and it is even required for some types of GUI apps.

@mangod9 mangod9 added needs-author-action An issue or pull request that requires more info or actions from the author. and removed untriaged New issue has not been triaged by the area owner labels May 2, 2024
@mangod9 mangod9 added this to the Future milestone May 2, 2024
@LunarWhisper
Copy link
Author

LunarWhisper commented May 16, 2024

Could you please tell us more about what your app does that it is hitting the 1.5MB limit?

@jkotas, as I said at the beginning:

This resulted in a StackOverflowException in a RestAPI service using AutoMapper, custom serialization logic, and building dynamic methods.

We've stopped using AutoMapper, we've fixed the generation code, we've changed a few recursive methods, and we've fixed all issues we found. Unfortunately, this did not solve the problems that we are not aware of. Now all we have to do is intensively run regression tests and hope that the problems with which users come to support can be easily resolved.

  • or -

Create platform-specific wrappers with hard-wired limits to return the old behavior.

Expectations: after changing the old version of the framework to a new one, and fixing the compatibility problems mentioned in the guides, we get a solution that works no worse than before.

Now our expectations are different from reality. :(

@dotnet-policy-service dotnet-policy-service bot added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed needs-author-action An issue or pull request that requires more info or actions from the author. labels May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-VM-coreclr needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration tenet-performance Performance related issue
Projects
None yet
Development

No branches or pull requests

8 participants