src: disable V8 external memory reasonable size check - #65589
Open
bitpshr wants to merge 1 commit into
Open
Conversation
V8 aborts the process when external memory grows by more than --external-memory-max-reasonable-size gigabytes in a single step. The 32 GB default is a Chromium-oriented sanity check; allocating a buffer larger than that is legitimate in Node and should raise a RangeError rather than terminate the process. Default the flag to 0 unless the user supplied a value, alongside the other V8 defaults Node sets explicitly. Fixes: nodejs#65534 Signed-off-by: Paul Bouchon <mail@bitpshr.net>
Collaborator
|
Review requested:
|
nodejs-github-bot
added
c++
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes: #65534
V8 aborts the process when external memory grows by more than
--external-memory-max-reasonable-sizegigabytes in a single step, which defaults to 32. Allocating a buffer larger than that is a legitimate thing to do in Node, and it should raise aRangeErrorrather than terminate the process:This defaults the flag to 0 unless the user supplied a value, next to the other V8 defaults Node already sets explicitly. Per @Renegade334 on the issue, the 32 GB limit is a Chromium oriented sanity check rather than something Node wants to inherit.
The guard checks both the dashed and underscored spellings, since V8 accepts either, and appends only when neither is present so an explicit
--external-memory-max-reasonable-size=Nstill wins.On testing: reproducing the real thing needs more than 32 GB of RAM, which I do not have, so I verified against a lowered threshold instead.
--external-memory-max-reasonable-size=1plus a 2 GB allocation produces the identical fatal check, and the same allocation succeeds once the flag defaults to 0. The test covers the override path rather than the new default, because asserting the default is gone would need a >32 GB allocation in CI. I confirmed the test fails if the appended flag clobbers a user supplied value, so it does catch the case it is there for.