doc: note resourceLimits termination is best-effort - #65019
Open
jcross wants to merge 1 commit into
Open
Conversation
The `resourceLimits` option is documented as terminating the `Worker` instance when a limit is reached, caveated only for a global out-of-memory situation. In practice Node.js terminates the worker when V8 reports that the heap is close to its limit, and a single allocation large enough to exceed the limit in one step aborts the whole process instead. That case is not a global out-of-memory situation, so the existing caveat does not describe it. This documents the behavior described in nodejs#47224, where it was closed as inactionable because it is a V8 limitation, and where worker termination was described as "a best effort attempt, it's not 100% reliable". No behavior change. Refs: nodejs#47224 Refs: nodejs#64155 Signed-off-by: James Cross <jms.cross@gmail.com>
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
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.
What the documentation says now
doc/api/worker_threads.mddescribes theresourceLimitsoption like this:The one caveat is for a global out-of-memory situation:
What happens
A worker with
maxOldGenerationSizeMb: 128on a machine with free memory is notin a global out-of-memory situation. It can still abort the whole process.
The result depends on the shape of the allocation, not on the total size. This
is Node.js v23.11.0 on Linux, with
maxOldGenerationSizeMb: 128in every run:ERR_WORKER_OUT_OF_MEMORYon'error', worker exit code 1ERR_WORKER_OUT_OF_MEMORYon'error', worker exit code 1new Array(1e8), then filled by indexFATAL ERROR: CALL_AND_RETRY_LAST Allocation failedpush()1e8 times onto one arrayFATAL ERROR: Reached heap limit Allocation failedThe first two rows are the documented behavior. The last two are not.
The fourth row is the reason for this pull request.
push()in a loop lookslike many small allocations, but the backing store doubles, so one growth step
becomes a single large allocation. Ordinary code reaches the fatal path.
Repro for the fourth row, with no dependencies:
The process aborts. Neither handler runs.
Why the existing caveat does not cover this
The caveat names a global out-of-memory situation. The host has memory to spare
here. What is reached is the limit that
resourceLimitssets, which is the casethe sentence above the caveat says is contained.
This is known and it is not a defect report
#47224 reports this behavior and
was closed as inactionable, because out-of-memory errors are not recoverable and
this is a V8 limitation. That closure is not in question here, and this pull
request asks for no change in behavior.
The reason to change the documentation is in that same thread:
That is an accurate description, and the documentation does not contain it. This
pull request copies it into the page.
#64155 is an open report of the
same class on v26.4.0, through a large inline source map, so the behavior is
current.
A note on the second hunk
ERR_WORKER_OUT_OF_MEMORYwas not referenced anywhere inworker_threads.mdbefore this change, so the link definition had to be added. Without it
lint-mdfails withno-undefined-references.How this was tested
The four rows above were measured, each in a separate process, reading the exit
code directly rather than through a pipe.
node tools/lint-md/lint-md.mjs doc/api/worker_threads.mdexits 0 on thisbranch. I also checked that the linter catches the missing link definition, by
removing it and confirming the
no-undefined-referenceswarning, so the pass isa real result and not an unrun check.
Please correct the wording as you prefer. You know how much detail belongs in an
option description better than I do, and a shorter version that keeps
"best-effort" would still fix the part that is wrong.