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: #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>

Author: codebytere

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) {

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<PerIsolateOptions> options) {
-  const SnapshotData* snapshot_data =
-      SnapshotData::FromEmbedderWrapper(embedder_snapshot_data);
   if (options == nullptr) {
     options = per_process::cli_options->per_isolate->Clone();
   }

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<PerIsolateOptions> options = nullptr);
   ~IsolateData();
 
@@ -662,7 +662,6 @@ struct SnapshotData {
   static bool FromBlob(SnapshotData* out, std::string_view in);
   static const SnapshotData* FromEmbedderWrapper(
       const EmbedderSnapshotData* data);
-  EmbedderSnapshotData::Pointer AsEmbedderWrapper() const;
 
   ~SnapshotData();
 };

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();

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));
 }

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_);

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<v8::Context> context = v8::Context::New(isolate_);