Skip to content

src: do not enable wasm trap handler if there's not enough vmem - #62132

Closed
joyeecheung wants to merge 2 commits into
nodejs:mainfrom
joyeecheung:wasm-handler-2
Closed

src: do not enable wasm trap handler if there's not enough vmem#62132
joyeecheung wants to merge 2 commits into
nodejs:mainfrom
joyeecheung:wasm-handler-2

Conversation

@joyeecheung

@joyeecheungjoyeecheung commented Mar 6, 2026

Copy link
Copy Markdown
Member

The first commit is a backport of https://chromium-review.googlesource.com/c/v8/v8/+/7638233

This complements --disable-wasm-trap-handler and allows users to keep wasm functional in an environment with virtual memory restrictions (e.g. in the case of VS code server run for remote access, the user does not necessary always have the permission on the server to configure NODE_OPTIONS or --disable-wasm-trap-handler - in fact they don't even necessarily know this has anything to do with WASM on Node.js since they shouldn't be aware of what VS code server is implemented with).

Supersedes #60788 - this keeps the estimation logic in Node.js instead, and also keeps it in POSIX because on other systems the issue that this is solving is not practical (e.g. on Windows, one cannot bound the virtual memory available without bounding the physical memory, and it does not make a difference whether trap based bound checks are enabled or not when physical memory limit is reached)

  1. When the amount of virtual memory available does not allow more than 1 wasm cage at startup, Node.js automatically disables trap-based bound checks for wasm - in this case, no wasm can run at all if we keep the trap-based bound checks, so there's no point enabling the optimization. This should remove a good enough of reproductions of Out of memory: Cannot allocate Wasm memory for new instance (after update from 1.100.3 to 1.101) microsoft/vscode#251777 (which is triggered by the usage of wasm in undici)
  2. When the amount of virtual memory allows more than 1 cage, but still less than the usual 128TB on 64-bit systems, the optimization is still optimistically enabled. In this case some initial wasm cage may be allocated but may fail later when there's not enough virtual memory left. This maintains the current behavior when we unconditionally enable the optimzation.
  3. --disable-wasm-trap-handler is kept as an escape hatch for 2.

We can tweak the threshold of estimation of needed wasm cages in 1 in Node.js to reduce the possibility of 2, this PR currently sets the threshold to 1, since even within Node.js core there's already undici that would use one cage whenever fetch() i sused.

Refs: microsoft/vscode#251777
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/7638233

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/security-wg
  • @nodejs/startup
  • @nodejs/v8-update

@nodejs-github-botnodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Mar 6, 2026
@joyeecheung
joyeecheung marked this pull request as draft March 6, 2026 14:51
@codecov

codecovBot commented Mar 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.67%. Comparing base (0d7e4b1) to head (322e31d).
⚠️ Report is 69 commits behind head on main.

Files with missing linesPatch %Lines
src/node.cc94.44%0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #62132 +/- ##
==========================================
- Coverage 89.68% 89.67% -0.02% 
==========================================
Files 676 676 Lines 206738 206755 +17 Branches 39594 39601 +7 ==========================================
- Hits 185414 185407 -7 - Misses 13467 13497 +30 + Partials 7857 7851 -6 
Files with missing linesCoverage Δ
src/debug_utils.h80.00% <ø> (ø)
src/node.cc76.78% <94.44%> (+0.39%)⬆️

... and 34 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

hubot pushed a commit to v8/v8 that referenced this pull request Mar 10, 2026
When the system does not have enough virtual memory for the wasm
cage, installing the trap handler would cause any code allocating
wasm memory to throw. Therefore it's useful for the embedder to
know when the system doesn't have enough virtual address space
to allocate enough wasm cages and in that case, skip the
trap handler installation so that wasm code can at least work
(even not at the maximal performance).
Node.js previously has a command line option
--disable-wasm-trap-handler to fully disable trap-based bound checks,
this new API would allow it to adapt automatically while keeping the
optimization in the happy path, since it's not always possible for
end users to opt-into disabling trap-based bound checks (for example,
when a VS Code Server is loaded in a remote server for debugging).
Refs: nodejs/node#62132
Refs: microsoft/vscode#251777
Change-Id: I345c076af2b2b47700e5716b49c3133fdf8a0981
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7638233
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#105702}
@nodejs-github-bot

This comment was marked as outdated.

@joyeecheung
joyeecheungforce-pushed the wasm-handler-2 branch 2 times, most recently from de51d96 to 37b5c46CompareMarch 11, 2026 00:22
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@joyeecheung
joyeecheung marked this pull request as ready for review March 12, 2026 02:27
@joyeecheungjoyeecheung changed the title [WIP] src: do not enable wasm trap handler if there's not enough vmemsrc: do not enable wasm trap handler if there's not enough vmemMar 12, 2026
@joyeecheungjoyeecheung added the review wanted PRs that need reviews. label Mar 12, 2026
@joyeecheung

Copy link
Copy Markdown
MemberAuthor

cc @nodejs/cpp-reviewers @nodejs/v8 PTAL, this should address microsoft/vscode#251777 (there are also a bunch of similarly confused users on the Internet affected by this issue, like https://www.reddit.com/r/vscode/comments/1ld6fp7/downloading_vs_code_server_loop_when_connecting/ and https://stackoverflow.com/questions/65055955/vscode-ssh-extension-stuck-on-the-installation-step)

@joyeecheungjoyeecheung added wasm Issues and PRs related to WebAssembly. memory Issues and PRs related to the memory management or memory footprint. lts-watch-v22.x PRs that may need to be released in v22.x lts-watch-v24.x PRs that may need to be released in v24.x labels Mar 12, 2026
@Aditi-1400

Aditi-1400 commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

Also, we should update --disable-wasm-trap-handler documentation in cli.md

@joyeecheung

Copy link
Copy Markdown
MemberAuthor

@Aditi-1400 Updated the docs, can you take another look? Thanks

@joyeecheung
joyeecheungforce-pushed the wasm-handler-2 branch 2 times, most recently from bab88bb to 6219acaCompareMarch 12, 2026 17:32
Comment threadsrc/node.cc Outdated
Comment threadtest/wasm-allocation/wasm-allocation.status Outdated
RafaelGSS pushed a commit that referenced this pull request Apr 27, 2026
RafaelGSS pushed a commit that referenced this pull request Apr 28, 2026
@aduh95aduh95 added the backported-to-v26.x PRs backported to the v26.x-staging branch. label May 5, 2026
@aduh95

Copy link
Copy Markdown
Contributor

Doesn't land cleanly on v24.x-staging, probably needs to be backported alongside #60678

@aduh95aduh95 added the backport-requested-v24.x PRs awaiting manual backport to the v24.x-staging branch. label May 7, 2026
@marco-ippolitomarco-ippolito added backport-requested-v22.x PRs awaiting manual backport to the v22.x-staging branch. and removed lts-watch-v22.x PRs that may need to be released in v22.x labels May 11, 2026
SalimTerryLi added a commit to SalimTerryLi/openRuyi that referenced this pull request May 27, 2026
SalimTerryLi added a commit to SalimTerryLi/openRuyi that referenced this pull request May 27, 2026
SalimTerryLi added a commit to SalimTerryLi/openRuyi that referenced this pull request May 27, 2026
SalimTerryLi added a commit to SalimTerryLi/openRuyi that referenced this pull request May 27, 2026
SalimTerryLi added a commit to SalimTerryLi/openRuyi that referenced this pull request May 27, 2026
SalimTerryLi added a commit to SalimTerryLi/openRuyi that referenced this pull request May 27, 2026
SalimTerryLi added a commit to SalimTerryLi/openRuyi that referenced this pull request May 27, 2026
SalimTerryLi added a commit to SalimTerryLi/openRuyi that referenced this pull request May 27, 2026
SalimTerryLi added a commit to SalimTerryLi/openRuyi that referenced this pull request Jun 3, 2026
SalimTerryLi added a commit to SalimTerryLi/openRuyi that referenced this pull request Jun 5, 2026
joyeecheung added a commit to joyeecheung/node that referenced this pull request Jul 7, 2026
Original commit message:
[api] Add V8::GetWasmMemoryReservationSizeInBytes()
When the system does not have enough virtual memory for the wasm
cage, installing the trap handler would cause any code allocating
wasm memory to throw. Therefore it's useful for the embedder to
know when the system doesn't have enough virtual address space
to allocate enough wasm cages and in that case, skip the
trap handler installation so that wasm code can at least work
(even not at the maximal performance).
Node.js previously has a command line option
--disable-wasm-trap-handler to fully disable trap-based bound checks,
this new API would allow it to adapt automatically while keeping the
optimization in the happy path, since it's not always possible for
end users to opt-into disabling trap-based bound checks (for example,
when a VS Code Server is loaded in a remote server for debugging).
Refs: nodejs#62132
Refs: microsoft/vscode#251777
Change-Id: I345c076af2b2b47700e5716b49c3133fdf8a0981
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7638233
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#105702}
Refs: v8/v8@bef0d9c
Co-authored-by: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: nodejs#62132
Refs: microsoft/vscode#251777
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/7638233
Reviewed-By: Aditi Singh <aditisingh1400@gmail.com>
Signed-off-by: Joyee Cheung <joyeec9h3@gmail.com>
joyeecheung added a commit to joyeecheung/node that referenced this pull request Jul 7, 2026
PR-URL: nodejs#62132
Refs: microsoft/vscode#251777
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/7638233
Reviewed-By: Aditi Singh <aditisingh1400@gmail.com>
Signed-off-by: Joyee Cheung <joyeec9h3@gmail.com>
@Renegade334Renegade334 added backport-open-v24.x Indicate that the PR has an open backport and removed backport-requested-v24.x PRs awaiting manual backport to the v24.x-staging branch. labels Jul 7, 2026
aduh95 pushed a commit that referenced this pull request Jul 21, 2026
Original commit message:
[api] Add V8::GetWasmMemoryReservationSizeInBytes()
When the system does not have enough virtual memory for the wasm
cage, installing the trap handler would cause any code allocating
wasm memory to throw. Therefore it's useful for the embedder to
know when the system doesn't have enough virtual address space
to allocate enough wasm cages and in that case, skip the
trap handler installation so that wasm code can at least work
(even not at the maximal performance).
Node.js previously has a command line option
--disable-wasm-trap-handler to fully disable trap-based bound checks,
this new API would allow it to adapt automatically while keeping the
optimization in the happy path, since it's not always possible for
end users to opt-into disabling trap-based bound checks (for example,
when a VS Code Server is loaded in a remote server for debugging).
Refs: #62132
Refs: microsoft/vscode#251777
Change-Id: I345c076af2b2b47700e5716b49c3133fdf8a0981
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7638233
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#105702}
Refs: v8/v8@bef0d9c
Co-authored-by: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: #62132
Backport-PR-URL: #64338
Refs: microsoft/vscode#251777
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/7638233
Signed-off-by: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
aduh95 pushed a commit that referenced this pull request Jul 21, 2026
PR-URL: #62132
Backport-PR-URL: #64338
Refs: microsoft/vscode#251777
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/7638233
Signed-off-by: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
aduh95 pushed a commit that referenced this pull request Jul 22, 2026
Original commit message:
[api] Add V8::GetWasmMemoryReservationSizeInBytes()
When the system does not have enough virtual memory for the wasm
cage, installing the trap handler would cause any code allocating
wasm memory to throw. Therefore it's useful for the embedder to
know when the system doesn't have enough virtual address space
to allocate enough wasm cages and in that case, skip the
trap handler installation so that wasm code can at least work
(even not at the maximal performance).
Node.js previously has a command line option
--disable-wasm-trap-handler to fully disable trap-based bound checks,
this new API would allow it to adapt automatically while keeping the
optimization in the happy path, since it's not always possible for
end users to opt-into disabling trap-based bound checks (for example,
when a VS Code Server is loaded in a remote server for debugging).
Refs: #62132
Refs: microsoft/vscode#251777
Change-Id: I345c076af2b2b47700e5716b49c3133fdf8a0981
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7638233
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#105702}
Refs: v8/v8@bef0d9c
Co-authored-by: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: #62132
Backport-PR-URL: #64338
Refs: microsoft/vscode#251777
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/7638233
Signed-off-by: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
aduh95 pushed a commit that referenced this pull request Jul 22, 2026
PR-URL: #62132
Backport-PR-URL: #64338
Refs: microsoft/vscode#251777
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/7638233
Signed-off-by: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-open-v24.xIndicate that the PR has an open backportbackport-requested-v22.xPRs awaiting manual backport to the v22.x-staging branch.backported-to-v26.xPRs backported to the v26.x-staging branch.commit-queue-rebaseAdd this label to allow the Commit Queue to land a PR in several commits.lib / srcIssues and PRs related to general changes in the lib or src directory.memoryIssues and PRs related to the memory management or memory footprint.needs-ciPRs that need a full CI run.review wantedPRs that need reviews.wasmIssues and PRs related to WebAssembly.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@joyeecheung@nodejs-github-bot@Aditi-1400@ChALkeR@kxxt@aduh95@Renegade334@marco-ippolito