From c55177fad71a8cfe8c090fb5ce5d0a79b20dcef6 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Wed, 2 Sep 2026 23:42:58 +0000 Subject: [PATCH] src: fix null pointer call when running without a startup snapshot 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: https://github.com/nodejs/node/pull/47731 Signed-off-by: Shelley Vohr --- src/api/environment.cc | 6 +++++- src/env.cc | 4 +--- src/env.h | 3 +-- src/node_main_instance.cc | 10 +++++----- src/node_snapshotable.cc | 4 ---- src/node_worker.cc | 14 +++++++------- test/cctest/test_environment.cc | 12 ++++++++++++ 7 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index 40aef133596e..30ec537bf5d1 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -391,7 +391,11 @@ IsolateData* CreateIsolateData( ArrayBufferAllocator* allocator, const EmbedderSnapshotData* embedder_snapshot_data) { return IsolateData::CreateIsolateData( - isolate, loop, platform, allocator, embedder_snapshot_data); + isolate, + loop, + platform, + allocator, + SnapshotData::FromEmbedderWrapper(embedder_snapshot_data)); } void FreeIsolateData(IsolateData* isolate_data) { diff --git a/src/env.cc b/src/env.cc index 5626b2a7e10e..2f18e354ad30 100644 --- a/src/env.cc +++ b/src/env.cc @@ -600,10 +600,8 @@ IsolateData* IsolateData::CreateIsolateData( uv_loop_t* loop, MultiIsolatePlatform* platform, ArrayBufferAllocator* allocator, - const EmbedderSnapshotData* embedder_snapshot_data, + const SnapshotData* snapshot_data, std::shared_ptr options) { - const SnapshotData* snapshot_data = - SnapshotData::FromEmbedderWrapper(embedder_snapshot_data); if (options == nullptr) { options = per_process::cli_options->per_isolate->Clone(); } diff --git a/src/env.h b/src/env.h index 4410ccb3e101..ff5d062d2f6a 100644 --- a/src/env.h +++ b/src/env.h @@ -143,7 +143,7 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer { uv_loop_t* event_loop, MultiIsolatePlatform* platform = nullptr, ArrayBufferAllocator* node_allocator = nullptr, - const EmbedderSnapshotData* embedder_snapshot_data = nullptr, + const SnapshotData* snapshot_data = nullptr, std::shared_ptr options = nullptr); ~IsolateData(); @@ -605,7 +605,6 @@ struct SnapshotData { static bool FromBlob(SnapshotData* out, std::string_view in); static const SnapshotData* FromEmbedderWrapper( const EmbedderSnapshotData* data); - EmbedderSnapshotData::Pointer AsEmbedderWrapper() const; ~SnapshotData(); }; diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc index 6f674df3ed0d..bc07d6aaf426 100644 --- a/src/node_main_instance.cc +++ b/src/node_main_instance.cc @@ -51,11 +51,11 @@ NodeMainInstance::NodeMainInstance(const SnapshotData* snapshot_data, // If the indexes are not nullptr, we are not deserializing isolate_data_.reset( - CreateIsolateData(isolate_, - event_loop, - platform, - array_buffer_allocator_.get(), - snapshot_data->AsEmbedderWrapper().get())); + IsolateData::CreateIsolateData(isolate_, + event_loop, + platform, + array_buffer_allocator_.get(), + snapshot_data)); isolate_data_->max_young_gen_size = isolate_params_->constraints.max_young_generation_size_in_bytes(); diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index e861e499534c..37123682dd16 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -628,10 +628,6 @@ const SnapshotData* SnapshotData::FromEmbedderWrapper( return data != nullptr ? data->impl_ : nullptr; } -EmbedderSnapshotData::Pointer SnapshotData::AsEmbedderWrapper() const { - return EmbedderSnapshotData::Pointer{new EmbedderSnapshotData(this, false)}; -} - bool SnapshotData::FromFile(SnapshotData* out, FILE* in) { return FromBlob(out, ReadFileSync(in)); } diff --git a/src/node_worker.cc b/src/node_worker.cc index 1a12946593ab..951da2e1e0f5 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -204,13 +204,13 @@ class WorkerThreadData { isolate->SetStackLimit(w->stack_base_); HandleScope handle_scope(isolate); - isolate_data_.reset(IsolateData::CreateIsolateData( - isolate, - &loop_, - w_->platform_, - allocator.get(), - w->snapshot_data()->AsEmbedderWrapper().get(), - std::move(w_->per_isolate_opts_))); + isolate_data_.reset( + IsolateData::CreateIsolateData(isolate, + &loop_, + w_->platform_, + allocator.get(), + w->snapshot_data(), + std::move(w_->per_isolate_opts_))); CHECK(isolate_data_); CHECK(!isolate_data_->is_building_snapshot()); isolate_data_->set_worker_context(w_); diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc index 449d7d38750b..4f52198c11de 100644 --- a/test/cctest/test_environment.cc +++ b/test/cctest/test_environment.cc @@ -355,6 +355,18 @@ TEST_F(EnvironmentTest, MultipleEnvironmentsPerIsolate) { EXPECT_TRUE(called_cb_2); } +TEST_F(EnvironmentTest, WorkerInEnvironmentWithoutSnapshot) { + const v8::HandleScope handle_scope(isolate_); + const Argv argv; + Env env{handle_scope, argv}; + CHECK_NULL(isolate_data_->snapshot_data()); + node::LoadEnvironment(*env, + "const { Worker } = require('worker_threads');" + "new Worker('process.exit(0)', { eval: true });") + .ToLocalChecked(); + EXPECT_EQ(node::SpinEventLoop(*env).FromJust(), 0); +} + TEST_F(EnvironmentTest, NoEnvironmentSanity) { const v8::HandleScope handle_scope(isolate_); v8::Local context = v8::Context::New(isolate_);