src: fix null pointer call when running without a startup snapshot by codebytere · Pull Request #65820 · nodejs/node

codebytere · 2026-09-05T16:05:25Z

Without a startup snapshot (--no-node-snapshot, a --without-node-snapshot build, or an embedder Environment bootstrapped from scratch), starting a Worker and setting up the main instance both make a member call through a null SnapshotData*; it only works because the callee does not touch this, and UBSan reports it for every such Worker. The call existed because IsolateData::CreateIsolateData() took an EmbedderSnapshotData* that its internal callers had to wrap their possibly-null SnapshotData* into.

The internal function now takes the SnapshotData* itself and only the public CreateIsolateData() unwraps the embedder type; AsEmbedderWrapper() had no other users and is removed.

Tests: EnvironmentTest.WorkerInEnvironmentWithoutSnapshot (UBSan before); node --no-node-snapshot with a Worker and the worker/snapshot suites pass.

Refs: #47731

Disclosure: the code, test and this description were written by Claude Code, directed and reviewed by @codebytere.

nodejs-github-bot · 2026-09-05T16:05:31Z

Review requested:

@nodejs/startup

nodejs-github-bot · 2026-09-05T16:06:32Z

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

Without a startup snapshot (`--no-node-snapshot`, a `--without-node-snapshot` build, or an embedder Environment that was bootstrapped from scratch) starting a Worker made a member call through a null `SnapshotData*`, and so did `NodeMainInstance` while setting itself up. It only worked because the function called does not touch `this`; UBSan reports it for every such Worker. The call existed because `IsolateData::CreateIsolateData()` took an `EmbedderSnapshotData*` and unwrapped it straight away, so the two internal callers wrapped their possibly-null `SnapshotData*` with `AsEmbedderWrapper()` only for it to be unwrapped again. Let the internal function take the `SnapshotData*` itself, unwrap in the public `CreateIsolateData()` only, and drop `AsEmbedderWrapper()`, which has no other users. Refs: nodejs#47731 Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>

nodejs-github-bot · 2026-09-05T17:57:19Z

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

codecov · 2026-09-05T20:01:05Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.17%. Comparing base (b0d56c7) to head (c55177f).
⚠️ Report is 49 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #65820      +/-   ##
==========================================
- Coverage   90.19%   90.17%   -0.03%     
==========================================
  Files         770      770              
  Lines      264410   264410              
  Branches    50243    50232      -11     
==========================================
- Hits       238490   238437      -53     
- Misses      16924    16979      +55     
+ Partials     8996     8994       -2

Files with missing lines	Coverage Δ
src/api/environment.cc	`78.01% <100.00%> (+0.13%)`	⬆️
src/env.cc	`82.24% <ø> (-0.10%)`	⬇️
src/env.h	`98.36% <ø> (ø)`
src/node_main_instance.cc	`90.14% <100.00%> (ø)`
src/node_snapshotable.cc	`73.47% <ø> (-0.06%)`	⬇️
src/node_worker.cc	`82.10% <100.00%> (-0.21%)`	⬇️

... and 30 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 · 2026-09-06T09:46:21Z

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

nodejs-github-bot · 2026-09-07T21:33:01Z

Landed in 1e0ebef

Without a startup snapshot (`--no-node-snapshot`, a `--without-node-snapshot` build, or an embedder Environment that was bootstrapped from scratch) starting a Worker made a member call through a null `SnapshotData*`, and so did `NodeMainInstance` while setting itself up. It only worked because the function called does not touch `this`; UBSan reports it for every such Worker. The call existed because `IsolateData::CreateIsolateData()` took an `EmbedderSnapshotData*` and unwrapped it straight away, so the two internal callers wrapped their possibly-null `SnapshotData*` with `AsEmbedderWrapper()` only for it to be unwrapped again. Let the internal function take the `SnapshotData*` itself, unwrap in the public `CreateIsolateData()` only, and drop `AsEmbedderWrapper()`, which has no other users. Refs: #47731 Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65820 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

… snapshot SnapshotData::AsEmbedderWrapper() was removed. Both call sites wrap the embedded snapshot, so use EmbedderSnapshotData::BuiltinSnapshotData(). Ref: nodejs/node#65820

* chore: bump node in DEPS to v24.22.0 * fix(patch): enable the V8 sandbox in shared-cage builds Upstream now enables the V8 sandbox only for shared-cage builds. Keep Electron's behaviour of enabling it whenever pointer compression is on, matching Chromium's V8. The unofficial.gni x86 hunk landed upstream. Ref: nodejs/node#62237 Ref: nodejs/node#65491 * fix(patch): use names for asymmetric key algorithms Upstream dropped the BoringSSL guard in DhKeyGenTraits::Setup(), removed ReadASN1Element() from ncrypto, and no longer includes <openssl/evp.h> from env.h, so those hunks are no longer needed. Ref: nodejs/node#65966 Ref: nodejs/node#66108 Ref: nodejs/node#65484 * fix(patch): skip RSA-PSS keygen deprecation tests with BoringSSL The skip is now upstream. Ref: nodejs/node#65808 * fix(patch): detach cppgc wrappers from their Realm before it is freed Upstream's CppgcWrapperList::Cleanup() now pops nodes itself and the wrapper owns and frees its list node, so the local PopFront/delete hunk is dropped. Ref: nodejs/node#65778 * fix(patch): gate jspi flag on V8_ENABLE_WEBASSEMBLY Ref: nodejs/node#66082 * chore: remove upstreamed patches - nodejs/node#63985 - nodejs/node#65630 - nodejs/node#64453 - nodejs/node#65778 * chore: update patches (trivial only) * chore: update filenames.auto.gni * fix(patch): discover hashes from OpenSSL providers ncrypto's DigestCache::aliases() now has a function-local static, so ncrypto needs the no_exit_time_destructors config too. Ref: nodejs/node#65484 * fix(patch): read the peer certificate chain without consuming it sk_X509_num() returns size_t in BoringSSL. Ref: nodejs/node#65602 * fix(patch): fix shared buffer growability validation Ref: nodejs/node#65845 * chore: update patches (trivial only) * node#65820: src: fix null pointer call when running without a startup snapshot SnapshotData::AsEmbedderWrapper() was removed. Both call sites wrap the embedded snapshot, so use EmbedderSnapshotData::BuiltinSnapshotData(). Ref: nodejs/node#65820 * node#65407: src: apply IsolateSettings when using a snapshot node::CreateEnvironment() no longer installs the per-isolate message listener when it builds the environment from a snapshot, so stop compensating for that and let SetIsolateUpForNode() add it. Ref: nodejs/node#65407 * test: drop removed wasm rab integration flag from webidl buffer source test Chromium's V8 removed --experimental-wasm-rab-integration after shipping it; the new test-internal-webidl-buffer-source.js passes it. Ref: nodejs/node#65845 Ref: https://chromium-review.googlesource.com/c/v8/v8/+/7708297 * chore: remove obsolete TLS external-memory accounting patch Upstream moved the SecureContext external-memory Increase() into Init() and guards the Decrease() on ctx_, so the leak this patch fixed is gone. The patch's removal of that guard now double-decrements when v24.22.0's TLSSocket closes a single-use context before the wrapper is destroyed, tripping V8's ExternalMemoryAccounter DCHECK in every TLS test. Already dropped on main in #53524. * test: skip new rmSync permission error test when the Node suite runs as root test-fs-rmSync-special-char-additional-error.js is new in v24.22.0 and relies on a read-only directory blocking removal, which root bypasses; the Linux Node.js test job runs as root. Ref: nodejs/node#64453 --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Sam Attard <sattard@anthropic.com>

* chore: bump node in DEPS to v24.22.0 * fix(patch): enable the V8 sandbox in shared-cage builds Upstream now enables the V8 sandbox only for shared-cage builds. Keep Electron's behaviour of enabling it whenever pointer compression is on, matching Chromium's V8. The unofficial.gni x86 hunk landed upstream. Ref: nodejs/node#62237 Ref: nodejs/node#65491 * fix(patch): use names for asymmetric key algorithms Upstream dropped the BoringSSL guard in DhKeyGenTraits::Setup(), removed ReadASN1Element() from ncrypto, and no longer includes <openssl/evp.h> from env.h, so those hunks are no longer needed. Ref: nodejs/node#65966 Ref: nodejs/node#66108 Ref: nodejs/node#65484 * fix(patch): skip RSA-PSS keygen deprecation tests with BoringSSL The skip is now upstream. Ref: nodejs/node#65808 * fix(patch): detach cppgc wrappers from their Realm before it is freed Upstream's CppgcWrapperList::Cleanup() now pops nodes itself and the wrapper owns and frees its list node, so the local PopFront/delete hunk is dropped. Ref: nodejs/node#65778 * fix(patch): gate jspi flag on V8_ENABLE_WEBASSEMBLY Ref: nodejs/node#66082 * chore: remove upstreamed patches - nodejs/node#63985 - nodejs/node#65630 - nodejs/node#64453 - nodejs/node#65778 * chore: update patches (trivial only) * chore: update filenames.auto.gni * fix(patch): discover hashes from OpenSSL providers ncrypto's DigestCache::aliases() now has a function-local static, so ncrypto needs the no_exit_time_destructors config too. Ref: nodejs/node#65484 * fix(patch): read the peer certificate chain without consuming it sk_X509_num() returns size_t in BoringSSL. Ref: nodejs/node#65602 * fix(patch): fix shared buffer growability validation Ref: nodejs/node#65845 * chore: update patches (trivial only) * node#65820: src: fix null pointer call when running without a startup snapshot SnapshotData::AsEmbedderWrapper() was removed. Both call sites wrap the embedded snapshot, so use EmbedderSnapshotData::BuiltinSnapshotData(). Ref: nodejs/node#65820 * node#65407: src: apply IsolateSettings when using a snapshot node::CreateEnvironment() no longer installs the per-isolate message listener when it builds the environment from a snapshot, so stop compensating for that and let SetIsolateUpForNode() add it. Ref: nodejs/node#65407 * test: drop removed wasm rab integration flag from webidl buffer source test Chromium's V8 removed --experimental-wasm-rab-integration after shipping it; the new test-internal-webidl-buffer-source.js passes it. Ref: nodejs/node#65845 Ref: https://chromium-review.googlesource.com/c/v8/v8/+/7708297 * test: skip new rmSync permission error test when the Node suite runs as root test-fs-rmSync-special-char-additional-error.js is new in v24.22.0 and relies on a read-only directory blocking removal, which root bypasses; the Linux Node.js test job runs as root. Ref: nodejs/node#64453 --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Sam Attard <sattard@anthropic.com>

* chore: bump node in DEPS to v24.22.0 * fix(patch): enable the V8 sandbox in shared-cage builds Upstream now enables the V8 sandbox only for shared-cage builds. Keep Electron's behaviour of enabling it whenever pointer compression is on, matching Chromium's V8. The unofficial.gni x86 hunk landed upstream. Ref: nodejs/node#62237 Ref: nodejs/node#65491 * fix(patch): use names for asymmetric key algorithms Upstream dropped the BoringSSL guard in DhKeyGenTraits::Setup(), removed ReadASN1Element() from ncrypto, and no longer includes <openssl/evp.h> from env.h, so those hunks are no longer needed. Ref: nodejs/node#65966 Ref: nodejs/node#66108 Ref: nodejs/node#65484 * fix(patch): skip RSA-PSS keygen deprecation tests with BoringSSL The skip is now upstream. Ref: nodejs/node#65808 * fix(patch): detach cppgc wrappers from their Realm before it is freed Upstream's CppgcWrapperList::Cleanup() now pops nodes itself and the wrapper owns and frees its list node, so the local PopFront/delete hunk is dropped. Ref: nodejs/node#65778 * fix(patch): gate jspi flag on V8_ENABLE_WEBASSEMBLY Ref: nodejs/node#66082 * chore: remove upstreamed patches - nodejs/node#63985 - nodejs/node#65630 - nodejs/node#64453 - nodejs/node#65778 * chore: update patches (trivial only) * chore: update filenames.auto.gni * fix(patch): discover hashes from OpenSSL providers ncrypto's DigestCache::aliases() now has a function-local static, so ncrypto needs the no_exit_time_destructors config too. Ref: nodejs/node#65484 * fix(patch): read the peer certificate chain without consuming it sk_X509_num() returns size_t in BoringSSL. Ref: nodejs/node#65602 * chore: update patches (trivial only) * node#65820: src: fix null pointer call when running without a startup snapshot SnapshotData::AsEmbedderWrapper() was removed. Both call sites wrap the embedded snapshot, so use EmbedderSnapshotData::BuiltinSnapshotData(). Ref: nodejs/node#65820 * node#65407: src: apply IsolateSettings when using a snapshot node::CreateEnvironment() no longer installs the per-isolate message listener when it builds the environment from a snapshot, so stop compensating for that and let SetIsolateUpForNode() add it. Ref: nodejs/node#65407 * test: drop removed wasm rab integration flag from webidl buffer source test Chromium's V8 removed --experimental-wasm-rab-integration after shipping it; the new test-internal-webidl-buffer-source.js passes it. Ref: nodejs/node#65845 Ref: https://chromium-review.googlesource.com/c/v8/v8/+/7708297 * chore: remove obsolete TLS external-memory accounting patch Upstream moved the SecureContext external-memory Increase() into Init() and guards the Decrease() on ctx_, so the leak this patch fixed is gone. The patch's removal of that guard now double-decrements when v24.22.0's TLSSocket closes a single-use context before the wrapper is destroyed, tripping V8's ExternalMemoryAccounter DCHECK in every TLS test. Already dropped on main in #53524. * test: skip new rmSync permission error test when the Node suite runs as root test-fs-rmSync-special-char-additional-error.js is new in v24.22.0 and relies on a read-only directory blocking removal, which root bypasses; the Linux Node.js test job runs as root. Ref: nodejs/node#64453 --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Sam Attard <sattard@anthropic.com>

* chore: bump node in DEPS to v24.22.0 * fix(patch): enable the V8 sandbox in shared-cage builds Upstream now enables the V8 sandbox only for shared-cage builds. Keep Electron's behaviour of enabling it whenever pointer compression is on, matching Chromium's V8. The unofficial.gni x86 hunk landed upstream. Ref: nodejs/node#62237 Ref: nodejs/node#65491 * fix(patch): use names for asymmetric key algorithms Upstream dropped the BoringSSL guard in DhKeyGenTraits::Setup(), removed ReadASN1Element() from ncrypto, and no longer includes <openssl/evp.h> from env.h, so those hunks are no longer needed. Ref: nodejs/node#65966 Ref: nodejs/node#66108 Ref: nodejs/node#65484 * fix(patch): skip RSA-PSS keygen deprecation tests with BoringSSL The skip is now upstream. Ref: nodejs/node#65808 * fix(patch): detach cppgc wrappers from their Realm before it is freed Upstream's CppgcWrapperList::Cleanup() now pops nodes itself and the wrapper owns and frees its list node, so the local PopFront/delete hunk is dropped. Ref: nodejs/node#65778 * fix(patch): gate jspi flag on V8_ENABLE_WEBASSEMBLY Ref: nodejs/node#66082 * chore: remove upstreamed patches - nodejs/node#63985 - nodejs/node#65630 - nodejs/node#64453 - nodejs/node#65778 * chore: update patches (trivial only) * chore: update filenames.auto.gni * fix(patch): discover hashes from OpenSSL providers ncrypto's DigestCache::aliases() now has a function-local static, so ncrypto needs the no_exit_time_destructors config too. Ref: nodejs/node#65484 * fix(patch): read the peer certificate chain without consuming it sk_X509_num() returns size_t in BoringSSL. Ref: nodejs/node#65602 * fix(patch): fix shared buffer growability validation Ref: nodejs/node#65845 * chore: update patches (trivial only) * node#65820: src: fix null pointer call when running without a startup snapshot SnapshotData::AsEmbedderWrapper() was removed. Both call sites wrap the embedded snapshot, so use EmbedderSnapshotData::BuiltinSnapshotData(). Ref: nodejs/node#65820 * node#65407: src: apply IsolateSettings when using a snapshot node::CreateEnvironment() no longer installs the per-isolate message listener when it builds the environment from a snapshot, so stop compensating for that and let SetIsolateUpForNode() add it. Ref: nodejs/node#65407 * test: drop removed wasm rab integration flag from webidl buffer source test Chromium's V8 removed --experimental-wasm-rab-integration after shipping it; the new test-internal-webidl-buffer-source.js passes it. Ref: nodejs/node#65845 Ref: https://chromium-review.googlesource.com/c/v8/v8/+/7708297 * test: skip new rmSync permission error test when the Node suite runs as root test-fs-rmSync-special-char-additional-error.js is new in v24.22.0 and relies on a read-only directory blocking removal, which root bypasses; the Linux Node.js test job runs as root. Ref: nodejs/node#64453 --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Sam Attard <sattard@anthropic.com>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Sep 5, 2026

codebytere changed the title ~~src: pass SnapshotData to IsolateData::CreateIsolateData() directly~~ src: fix null pointer call when running without a startup snapshot Sep 5, 2026

codebytere force-pushed the fix/embedder-worker-null-snapshot-ub branch from a6a8713 to c55177f Compare September 5, 2026 17:56

jasnell approved these changes Sep 6, 2026

View reviewed changes

panva added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Sep 7, 2026

legendecas approved these changes Sep 7, 2026

View reviewed changes

legendecas added the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 7, 2026

nodejs-github-bot merged commit 1e0ebef into nodejs:main Sep 7, 2026
72 checks passed

nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 7, 2026

github-actions Bot mentioned this pull request Sep 20, 2026

2026-09-22, Version 26.10.0 (Current) #66163

Merged

github-actions Bot mentioned this pull request Oct 11, 2026

2026-10-14, Version 24.22.0 'Krypton' (LTS) #66667

Merged

Navigation Menu

Repository navigation

src: fix null pointer call when running without a startup snapshot - #65820

src: fix null pointer call when running without a startup snapshot#65820
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
codebytere:fix/embedder-worker-null-snapshot-ub

codebytere commented Sep 5, 2026 •

edited

Loading

Uh oh!

nodejs-github-bot commented Sep 5, 2026

Uh oh!

nodejs-github-bot commented Sep 5, 2026

Uh oh!

nodejs-github-bot commented Sep 5, 2026

Uh oh!

codecov Bot commented Sep 5, 2026 •

edited

Loading

Uh oh!

nodejs-github-bot commented Sep 6, 2026

Uh oh!

Uh oh!

nodejs-github-bot commented Sep 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

codebytere commented Sep 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Sep 5, 2026

Uh oh!

nodejs-github-bot commented Sep 5, 2026

Uh oh!

nodejs-github-bot commented Sep 5, 2026

Uh oh!

codecov Bot commented Sep 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nodejs-github-bot commented Sep 6, 2026

Uh oh!

Uh oh!

nodejs-github-bot commented Sep 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codebytere commented Sep 5, 2026 •

edited

Loading

codecov Bot commented Sep 5, 2026 •

edited

Loading