Skip to content

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

Draft
joyeecheung wants to merge 2 commits intonodejs:mainfrom
joyeecheung:wasm-handler-2
Draft

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

Conversation

@joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Mar 6, 2026

Marked as WIP as it needs to wait for 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 running VS code server for remote debugging, the user does not necessary always have the permission to configure the NODE_OPTIONS to configure --disable-wasm-trap-handler, and they don't necessarily even 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
  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.

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

joyeecheung and others added 2 commits March 5, 2026 19:31
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 emebdder to
    know when the system doesn't have enough virtual address space
    to allocate any wasm cage 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 for this, this new API would allow
    it to adapt automatically without requiring the use of
    a command line flag, which is not always under end-user's control
    (for example, when a VS Code Server is loaded in a remote server
    for debugging).

    As a drive by this also makes the trap handlers a bit more robust
    by making them a no-op of trap handling is not enabled at all.

    Refs: nodejs#52766
    Refs: microsoft/vscode#251777

    Bug: 40644005
    Change-Id: Ie0608970daabe370db4616b875a8f098711c80e2

Refs: v8/v8@2733a08
Co-authored-by: Joyee Cheung <joyeec9h3@gmail.com>
@nodejs-github-bot
Copy link
Collaborator

Review requested:

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

@nodejs-github-bot nodejs-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
Copy link

codecov bot commented Mar 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.65%. Comparing base (b864049) to head (0c431d9).
⚠️ Report is 179 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #62132      +/-   ##
==========================================
- Coverage   89.74%   89.65%   -0.10%     
==========================================
  Files         675      676       +1     
  Lines      204601   206424    +1823     
  Branches    39325    39537     +212     
==========================================
+ Hits       183616   185065    +1449     
- Misses      13273    13489     +216     
- Partials     7712     7870     +158     
Files with missing lines Coverage Δ
src/debug_utils.h 80.00% <ø> (+1.42%) ⬆️
src/node.cc 76.75% <100.00%> (+0.46%) ⬆️

... and 197 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants