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

Increase thread stack size limit on Windows #43630

Closed
tniessen opened this issue Jun 30, 2022 · 3 comments · Fixed by #43632
Closed

Increase thread stack size limit on Windows #43630

tniessen opened this issue Jun 30, 2022 · 3 comments · Fixed by #43632
Labels
addons Issues and PRs related to native addons. build Issues and PRs related to build files or the CI. feature request Issues that request new features to be added to Node.js. windows Issues and PRs related to the Windows platform.

Comments

@tniessen
Copy link
Member

tniessen commented Jun 30, 2022

What is the problem this feature will solve?

Node.js uses a hard-coded default stack size limit on Windows, which is 1 MiB:

Dump of file C:\Program Files\nodejs\node.exe

...

OPTIONAL HEADER VALUES
             ... ...
          100000 size of stack reserve
            1000 size of stack commit

On other platforms, the limit is usually around 8 or 10 MiB and can be changed using ulimit -s.

V8 stack size

The default V8 stack size limit on 64-bit systems is 984 kBytes, which is below the thread stack size limit on all supported operating systems. On Linux, the V8 stack size can usually be increased almost arbitrarily by passing --stack-size=... if needed, and the maximum thread stack size can usually be changed without modifying the node executable (ulimit -s). On Windows, however, raising V8's stack size beyond the hard-coded thread stack size limit of 1 MiB will crash the process:

C:\Users\Tobias>node --stack-size=1100
Welcome to Node.js v18.3.0.
Type ".help" for more information.
> let n = 0; function stackExplode() { n++; return stackExplode(); }; stackExplode()

C:\Users\Tobias>echo %ERRORLEVEL%
-1073741571

Raising the hard-coded thread stack size limit will allow greater manual adjustments through --stack-size=... beyond the default value.

Native addons

Native addons are not necessarily limited by the V8 stack size limit, but they are limited by the hard-coded thread stack size limit (unless they create new threads or fibers). While the stack size limit of 1 MiB is often sufficient, some libraries perform stack allocations exceeding the limit. Such libraries work just fine when using Node.js on Linux but overflow the stack on Windows.

Raising the hard-coded thread stack size limit will simplify creating portable native addons that rely on large stack allocations (however questionable large stack allocations might be).

What is the feature you are proposing to solve the problem?

Increase the thread stack size limit beyond the default value of 1 MiB. See /STACK.

Only increase the reserve size, not the commit size, to avoid an increase in the actual memory usage.


For comparison, Chrome, Edge, and Firefox all use a hard-coded thread stack size limit of 8 MiB.

Dump of file C:\Program Files\Google\Chrome\Application\chrome.exe

...

OPTIONAL HEADER VALUES
             ... ...
          800000 size of stack reserve
            1000 size of stack commit
Dump of file C:\Program Files (x86)\Microsoft\Edge\Application\msedge.exe

...

OPTIONAL HEADER VALUES
             ... ...
          800000 size of stack reserve
            1000 size of stack commit
Dump of file C:\Program Files\Mozilla Firefox\firefox.exe

...

OPTIONAL HEADER VALUES
             ... ...
          800000 size of stack reserve
            1000 size of stack commit

What alternatives have you considered?

  • node-mceliece-nist and node-pqclean switch to a WebAssembly module instead of a native addon on Windows, to support cryptographic implementations that rely on large stack allocations (which are mapped to heap memory in WebAssembly).

  • editbin.exe can be used to change the hard-coded stack size of node.exe:

    editbin.exe /STACK:800000 node.exe
    
@tniessen tniessen added windows Issues and PRs related to the Windows platform. build Issues and PRs related to build files or the CI. feature request Issues that request new features to be added to Node.js. addons Issues and PRs related to native addons. labels Jun 30, 2022
@tniessen
Copy link
Member Author

cc @nodejs/build-files @nodejs/platform-windows

@targos
Copy link
Member

targos commented Jun 30, 2022

Can you please point to where it is hard-coded in the repo?

@tniessen
Copy link
Member Author

@targos I think the MSVC linker always embeds a hard-coded value and we just don't override the default of 1 MiB.

After looking into this more, it seems like it's as simple as adding 'StackReserveSize': 0x800000 to VCLinkerTool in msvs_settings. I am waiting for my local build to complete so I can confirm.

tniessen added a commit to tniessen/node that referenced this issue Jun 30, 2022
tniessen added a commit to tniessen/node that referenced this issue Jun 30, 2022
nodejs-github-bot pushed a commit that referenced this issue Jul 2, 2022
Fixes: #43630

PR-URL: #43632
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
targos pushed a commit that referenced this issue Jul 12, 2022
Fixes: #43630

PR-URL: #43632
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
targos pushed a commit that referenced this issue Jul 20, 2022
Fixes: #43630

PR-URL: #43632
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
targos pushed a commit that referenced this issue Jul 31, 2022
Fixes: #43630

PR-URL: #43632
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this issue Oct 10, 2022
Fixes: nodejs/node#43630

PR-URL: nodejs/node#43632
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addons Issues and PRs related to native addons. build Issues and PRs related to build files or the CI. feature request Issues that request new features to be added to Node.js. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants