Skip to content

[v24.x backport] src: do not enable wasm trap handler if there's not enough vmem - #64338

Closed
joyeecheung wants to merge 121 commits into
nodejs:v24.x-stagingfrom
joyeecheung:backport-62132-to-24
Closed

[v24.x backport] src: do not enable wasm trap handler if there's not enough vmem#64338
joyeecheung wants to merge 121 commits into
nodejs:v24.x-stagingfrom
joyeecheung:backport-62132-to-24

Conversation

@joyeecheung

Copy link
Copy Markdown
Member

This is a backport of #62132 which would help closing microsoft/vscode#251777

panva and others added 30 commits June 25, 2026 15:30
Replace the O(n) case-insensitive algorithm-name scan
with an O(1) SafeMap lookup. The map is pre-built at
module init alongside kSupportedAlgorithms.

Hoist the opts object literal used in normalizeAlgorithm
to module level to avoid allocating identical
{ prefix, context } objects on every call.

Pre-compute ObjectKeys() for simpleAlgorithmDictionaries
entries at module init to avoid allocating a new keys
array on every normalizeAlgorithm call.

Signed-off-by: Filip Skokan <[email protected]>
PR-URL: nodejs#62756
Reviewed-By: Yagiz Nizipli <[email protected]>
Replace object spread in nested WebIDL conversion
options with stable-shape ordinary objects. This keeps
hot dictionary and sequence conversion paths from
allocating null-prototype spread results.

Apply the same pattern to Web Crypto converter wrappers
that override allowResizable or enable [EnforceRange].

Signed-off-by: Filip Skokan <[email protected]>
PR-URL: nodejs#62756
Reviewed-By: Yagiz Nizipli <[email protected]>
Signed-off-by: Matteo Collina <[email protected]>
PR-URL: nodejs#62562
Reviewed-By: James M Snell <[email protected]>
Signed-off-by: Nad Alaba <[email protected]>
PR-URL: nodejs#63117
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Signed-off-by: avivkeller <[email protected]>
PR-URL: nodejs#63493
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: René <[email protected]>
Rename WebCrypto operation parameters and local variables to match the
spec terminology.

Use operation, algorithm, and key names consistently between the docs
and implementation.

Signed-off-by: Filip Skokan <[email protected]>
PR-URL: nodejs#63518
Reviewed-By: René <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Drive the regression test from the WebCrypto algorithm registry so all
supported algorithms and operations must add explicit coverage
regardless of whether they are native-job backed or js-based.

Signed-off-by: Filip Skokan <[email protected]>
PR-URL: nodejs#63520
Reviewed-By: René <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Refs: nodejs#62761
Signed-off-by: StefanStojanovic <[email protected]>
PR-URL: nodejs#63413
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
Signed-off-by: Mohamed Sayed <[email protected]>
PR-URL: nodejs#63529
Fixes: nodejs#63514
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Signed-off-by: Filip Skokan <[email protected]>
PR-URL: nodejs#63556
Reviewed-By: Jordan Harband <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Signed-off-by: Filip Skokan <[email protected]>
PR-URL: nodejs#63556
Reviewed-By: Jordan Harband <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Signed-off-by: Filip Skokan <[email protected]>
PR-URL: nodejs#63556
Reviewed-By: Jordan Harband <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Signed-off-by: Filip Skokan <[email protected]>
PR-URL: nodejs#63556
Reviewed-By: Jordan Harband <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Signed-off-by: Mert Can Altin <[email protected]>
PR-URL: nodejs#63385
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Gürgün Dayıoğlu <[email protected]>
Fix build warnings by casting the number literals to the size_t
constants they are compared against.

Signed-off-by: Maya Lekova <[email protected]>
PR-URL: nodejs#63490
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: René <[email protected]>
PR-URL: nodejs#63668
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#63669
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Signed-off-by: Mohamed Sayed <[email protected]>
PR-URL: nodejs#63601
Fixes: nodejs#63513
Reviewed-By: LiviaMedeiros <[email protected]>
Reviewed-By: Matthew Aitken <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Signed-off-by: RafaelGSS <[email protected]>
PR-URL: nodejs#63293
Reviewed-By: Antoine du Hamel <[email protected]>
This function call can fail with `Z_VERSION_ERROR` if the compiled
library vs loaded library mismatched in version number or in
stream structure size.
In those cases, zlib doesn't initialize the `strm_.msg` field to
null. Therefore, when a `CompressionError` object is created via
`ErrorForMessage()`, it can read a stale or uninitialized `strm_.msg`
pointer that will cause a crash.

Example ASAN report:
```
AddressSanitizer: SEGV on unknown address
    #0 __strlen_avx2
        string/../sysdeps/x86_64/multiarch/strlen-avx2.S:76
    #1 strlen (/work/node/out/Debug/node+0x1a42ab7)
    #2 v8::(anonymous namespace)::StringLength(char const*)
        /work/node/out/../deps/v8/src/api/api.cc:7581:16
    #3 v8::(anonymous namespace)::StringLength(unsigned char const*)
        /work/node/out/../deps/v8/src/api/api.cc:7587:10
    #4 v8::String::NewFromOneByte(v8::Isolate*,
        unsigned char const*, v8::NewStringType, int)
        /work/node/out/../deps/v8/src/api/api.cc:7677:3
    #5 node::OneByteString(v8::Isolate*,
        char const*, int, v8::NewStringType)
        /work/node/out/../src/util-inl.h:166:10
    #6 node::(anonymous namespace)::CompressionStream<
        node::(anonymous namespace)::ZlibContext>
        ::EmitError(node::(anonymous namespace)
        ::CompressionError const&)
        /work/node/out/../src/node_zlib.cc:565:7
    #7 node::(anonymous namespace)::CompressionStream<
        node::(anonymous namespace)::ZlibContext>
        ::CheckError()
        /work/node/out/../src/node_zlib.cc:519:5
    #8 node::(anonymous namespace)::CompressionStream<
        node::(anonymous namespace)::ZlibContext>
        ::AfterThreadPoolWork(int)
        /work/node/out/../src/node_zlib.cc:543:10
    #9 node::ThreadPoolWork::ScheduleWork()
        ::'lambda'(uv_work_s*, int)
        ::operator()(uv_work_s*, int) const
        /work/node/out/../src/threadpoolwork-inl.h:57:15
    #10 node::ThreadPoolWork::ScheduleWork()
        ::'lambda'(uv_work_s*, int)
        ::__invoke(uv_work_s*, int)
        /work/node/out/../src/threadpoolwork-inl.h:48:7
    nodejs#11 uv__work_done /work/libuv-1.51.0/src/threadpool.c:330:5
    nodejs#12 uv__async_io.part.0
        /work/libuv-1.51.0/src/unix/async.c:208:5
```

Signed-off-by: ndossche <[email protected]>
PR-URL: nodejs#63476
Reviewed-By: Anna Henningsen <[email protected]>
Signed-off-by: arbinger <[email protected]>
PR-URL: nodejs#63050
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ethan Arrowood <[email protected]>
PR-URL: nodejs#63670
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Signed-off-by: Tobias Nießen <[email protected]>
PR-URL: nodejs#63644
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Update the setRawMode documentation to specify that raw mode disables
special processing of input characters only. Output processing, such as
newline translation on Unix terminals, is not affected.

Fixes: nodejs#63059
Signed-off-by: zeeshan56656 <[email protected]>
PR-URL: nodejs#63438
Reviewed-By: Anna Henningsen <[email protected]>
A recursively watched directory can be removed after a parent watcher
observes it but before the non-native recursive watcher scans it.

Ignore ENOENT from the directory scan so this deletion race does not
emit an unhandled watcher error.

Signed-off-by: Kamat, Trivikram <[email protected]>
Assisted-by: openai:gpt-5.5
PR-URL: nodejs#63686
Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-06-01.md#jstest-failure
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Use common.nodeProcessAborted() for the intentional abort path so
platform-specific abort signals such as SIGILL are accepted.

Signed-off-by: Kamat, Trivikram <[email protected]>
Assisted-by: openai:gpt-5.5
PR-URL: nodejs#63687
Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-06-01.md#jstest-failure
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Signed-off-by: Herrtian <[email protected]>
PR-URL: nodejs#63542
Fixes: nodejs#62774
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Signed-off-by: Daijiro Wachi <[email protected]>
PR-URL: nodejs#63700
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Tierney Cyren <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Signed-off-by: Antoine du Hamel <[email protected]>
PR-URL: nodejs#63707
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@codecov

codecov Bot commented Jul 7, 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.87%. Comparing base (b881658) to head (5159d50).
⚠️ Report is 371 commits behind head on v24.x-staging.

Files with missing lines Patch % Lines
src/node.cc 94.44% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@                Coverage Diff                @@
##           v24.x-staging   #64338      +/-   ##
=================================================
- Coverage          89.92%   89.87%   -0.05%     
=================================================
  Files                686      694       +8     
  Lines             208389   212085    +3696     
  Branches           40077    40686     +609     
=================================================
+ Hits              187387   190616    +3229     
- Misses             13238    13578     +340     
- Partials            7764     7891     +127     
Files with missing lines Coverage Δ
src/debug_utils.h 80.00% <ø> (ø)
src/node.cc 74.87% <94.44%> (+0.43%) ⬆️

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

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/74688/

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/74778/

@joyeecheung

Copy link
Copy Markdown
Member Author

CI is green. @nodejs/releasers can we include this in the next v24 release please? This would help address a class of bugs that basically manifests as "vscode remote ssh mysteriously getting stuck when connecting to a server".

@joyeecheung joyeecheung added commit-queue PRs queued for automated landing through the Commit Queue. commit-queue-rebase PRs the Commit Queue should land as multiple self-contained commits. labels Jul 17, 2026
@joyeecheung
joyeecheung force-pushed the backport-62132-to-24 branch from 0eeb4f5 to 3048bdc Compare July 17, 2026 21:34
@joyeecheung
joyeecheung force-pushed the backport-62132-to-24 branch from 3048bdc to 5159d50 Compare July 17, 2026 21:38
@joyeecheung

Copy link
Copy Markdown
Member Author

CI was green. Force pushed to sign the commits, but it seems I cannot land it myself - maybe it needs to be someone from @nodejs/releasers ?

@aduh95 aduh95 removed commit-queue PRs queued for automated landing through the Commit Queue. commit-queue-rebase PRs the Commit Queue should land as multiple self-contained commits. labels Jul 21, 2026
@aduh95
aduh95 force-pushed the backport-62132-to-24 branch from 5159d50 to 0eeb4f5 Compare July 21, 2026 14:47
@aduh95 aduh95 changed the title [v24.x] src: do not enable wasm trap handler if there's not enough vmem [v24.x backport] src: do not enable wasm trap handler if there's not enough vmem Jul 21, 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 <[email protected]>
    Reviewed-by: Michael Lippautz <[email protected]>
    Commit-Queue: Joyee Cheung <[email protected]>
    Reviewed-by: Clemens Backes <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#105702}

Refs: v8/v8@bef0d9c
Co-authored-by: Joyee Cheung <[email protected]>
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 <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
aduh95 pushed a commit that referenced this pull request Jul 21, 2026
@aduh95 aduh95 added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Jul 21, 2026
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 <[email protected]>
    Reviewed-by: Michael Lippautz <[email protected]>
    Commit-Queue: Joyee Cheung <[email protected]>
    Reviewed-by: Clemens Backes <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#105702}

Refs: v8/v8@bef0d9c
Co-authored-by: Joyee Cheung <[email protected]>
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 <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
aduh95 pushed a commit that referenced this pull request Jul 22, 2026
aduh95 pushed a commit that referenced this pull request Jul 30, 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 <[email protected]>
    Reviewed-by: Michael Lippautz <[email protected]>
    Commit-Queue: Joyee Cheung <[email protected]>
    Reviewed-by: Clemens Backes <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#105702}

Refs: v8/v8@bef0d9c
Co-authored-by: Joyee Cheung <[email protected]>
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 <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
aduh95 pushed a commit that referenced this pull request Jul 30, 2026
@aduh95

aduh95 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Landed in 6e2f7e6...818b43d

@aduh95 aduh95 closed this Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. v24.x Issues that can be reproduced on v24.x or PRs targeting the v24.x-staging branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.