bootstrap: put is_building_snapshot state in IsolateData

Previously we modify the CLI options store to indicate whether the isolate is created for building snapshot, which is a bit hacky and makes it difficult to tell whether the snapshot is built from the command line or through other APIs. This patch adds is_building_snapshot to IsolateData and use this instead when we need to know whether the isolate is created for building snapshot (but do not care about where that request is coming from). PR-URL: nodejs#47887 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
Ceres6 · Aug 14, 2023 · 51ef15c · 51ef15c
1 parent 249f6a7
commit 51ef15c
Show file tree

Hide file tree

Showing 10 changed files with 131 additions and 18 deletions.
diff --git a/lib/internal/v8/startup_snapshot.js b/lib/internal/v8/startup_snapshot.js
@@ -14,11 +14,11 @@ const {
   setSerializeCallback,
   setDeserializeCallback,
   setDeserializeMainFunction: _setDeserializeMainFunction,
+  isBuildingSnapshotBuffer
 } = internalBinding('mksnapshot');
 
 function isBuildingSnapshot() {
-  // For now this is the only way to build a snapshot.
-  return require('internal/options').getOptionValue('--build-snapshot');
+  return isBuildingSnapshotBuffer[0];
 }
 
 function throwIfNotBuildingSnapshot() {

diff --git a/src/api/embed_helpers.cc b/src/api/embed_helpers.cc
@@ -142,8 +142,8 @@ CommonEnvironmentSetup::CommonEnvironmentSetup(
 
     impl_->isolate_data.reset(CreateIsolateData(
         isolate, loop, platform, impl_->allocator.get(), snapshot_data));
-    impl_->isolate_data->options()->build_snapshot =
-        impl_->snapshot_creator.has_value();
+    impl_->isolate_data->set_is_building_snapshot(
+        impl_->snapshot_creator.has_value());
 
     if (snapshot_data) {
       impl_->env.reset(make_env(this));

diff --git a/src/base_object_types.h b/src/base_object_types.h
@@ -12,6 +12,7 @@ namespace node {
 #define SERIALIZABLE_BINDING_TYPES(V)                                          \
   V(encoding_binding_data, encoding_binding::BindingData)                      \
   V(fs_binding_data, fs::BindingData)                                          \
+  V(mksnapshot_binding_data, mksnapshot::BindingData)                          \
   V(v8_binding_data, v8_utils::BindingData)                                    \
   V(blob_binding_data, BlobBindingData)                                        \
   V(process_binding_data, process::BindingData)                                \

diff --git a/src/env.h b/src/env.h
@@ -136,6 +136,9 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer {
   void MemoryInfo(MemoryTracker* tracker) const override;
   IsolateDataSerializeInfo Serialize(v8::SnapshotCreator* creator);
 
+  bool is_building_snapshot() const { return is_building_snapshot_; }
+  void set_is_building_snapshot(bool value) { is_building_snapshot_ = value; }
+
   inline uv_loop_t* event_loop() const;
   inline MultiIsolatePlatform* platform() const;
   inline const SnapshotData* snapshot_data() const;
@@ -219,6 +222,7 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer {
   const SnapshotData* snapshot_data_;
   std::shared_ptr<PerIsolateOptions> options_;
   worker::Worker* worker_context_ = nullptr;
+  bool is_building_snapshot_ = false;
 };
 
 struct ContextInfo {

diff --git a/src/node.cc b/src/node.cc
@@ -283,7 +283,7 @@ MaybeLocal<Value> StartExecution(Environment* env, StartExecutionCallback cb) {
     auto reset_entry_point =
         OnScopeLeave([&]() { env->set_embedder_entry_point({}); });
 
-    const char* entry = env->isolate_data()->options()->build_snapshot
+    const char* entry = env->isolate_data()->is_building_snapshot()
                             ? "internal/main/mksnapshot"
                             : "internal/main/embedding";
 
@@ -311,7 +311,7 @@ MaybeLocal<Value> StartExecution(Environment* env, StartExecutionCallback cb) {
     return StartExecution(env, "internal/main/inspect");
   }
 
-  if (env->isolate_data()->options()->build_snapshot) {
+  if (env->isolate_data()->is_building_snapshot()) {
     return StartExecution(env, "internal/main/mksnapshot");
   }
 

diff --git a/src/node_binding.h b/src/node_binding.h
@@ -37,6 +37,7 @@ static_assert(static_cast<int>(NM_F_LINKED) ==
   V(contextify)                                                                \
   V(encoding_binding)                                                          \
   V(fs)                                                                        \
+  V(mksnapshot)                                                                \
   V(timers)                                                                    \
   V(process_methods)                                                           \
   V(performance)                                                               \

diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc
@@ -56,6 +56,8 @@ NodeMainInstance::NodeMainInstance(const SnapshotData* snapshot_data,
                         platform,
                         array_buffer_allocator_.get(),
                         snapshot_data->AsEmbedderWrapper().get()));
+  isolate_data_->set_is_building_snapshot(
+      per_process::cli_options->per_isolate->build_snapshot);
 
   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
@@ -3,6 +3,7 @@
 #include <iostream>
 #include <sstream>
 #include <vector>
+#include "aliased_buffer-inl.h"
 #include "base_object-inl.h"
 #include "blob_serializer_deserializer-inl.h"
 #include "debug_utils-inl.h"
@@ -34,6 +35,7 @@ namespace node {
 using v8::Context;
 using v8::Function;
 using v8::FunctionCallbackInfo;
+using v8::FunctionTemplate;
 using v8::HandleScope;
 using v8::Isolate;
 using v8::Local;
@@ -1177,8 +1179,6 @@ void SerializeSnapshotableObjects(Realm* realm,
   });
 }
 
-namespace mksnapshot {
-
 // NB: This is also used by the regular embedding codepath.
 void GetEmbedderEntryFunction(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
@@ -1251,16 +1251,89 @@ void SetDeserializeMainFunction(const FunctionCallbackInfo<Value>& args) {
   env->set_snapshot_deserialize_main(args[0].As<Function>());
 }
 
-void Initialize(Local<Object> target,
-                Local<Value> unused,
-                Local<Context> context,
-                void* priv) {
+namespace mksnapshot {
+
+BindingData::BindingData(Realm* realm,
+                         v8::Local<v8::Object> object,
+                         InternalFieldInfo* info)
+    : SnapshotableObject(realm, object, type_int),
+      is_building_snapshot_buffer_(
+          realm->isolate(),
+          1,
+          MAYBE_FIELD_PTR(info, is_building_snapshot_buffer)) {
+  if (info == nullptr) {
+    object
+        ->Set(
+            realm->context(),
+            FIXED_ONE_BYTE_STRING(realm->isolate(), "isBuildingSnapshotBuffer"),
+            is_building_snapshot_buffer_.GetJSArray())
+        .Check();
+  } else {
+    is_building_snapshot_buffer_.Deserialize(realm->context());
+  }
+  // Reset the status according to the current state of the realm.
+  bool is_building_snapshot = realm->isolate_data()->is_building_snapshot();
+  DCHECK_IMPLIES(is_building_snapshot,
+                 realm->isolate_data()->snapshot_data() == nullptr);
+  is_building_snapshot_buffer_[0] = is_building_snapshot ? 1 : 0;
+  is_building_snapshot_buffer_.MakeWeak();
+}
+
+bool BindingData::PrepareForSerialization(Local<Context> context,
+                                          v8::SnapshotCreator* creator) {
+  DCHECK_NULL(internal_field_info_);
+  internal_field_info_ = InternalFieldInfoBase::New<InternalFieldInfo>(type());
+  internal_field_info_->is_building_snapshot_buffer =
+      is_building_snapshot_buffer_.Serialize(context, creator);
+  // Return true because we need to maintain the reference to the binding from
+  // JS land.
+  return true;
+}
+
+InternalFieldInfoBase* BindingData::Serialize(int index) {
+  DCHECK_EQ(index, BaseObject::kEmbedderType);
+  InternalFieldInfo* info = internal_field_info_;
+  internal_field_info_ = nullptr;
+  return info;
+}
+
+void BindingData::Deserialize(Local<Context> context,
+                              Local<Object> holder,
+                              int index,
+                              InternalFieldInfoBase* info) {
+  DCHECK_EQ(index, BaseObject::kEmbedderType);
+  v8::HandleScope scope(context->GetIsolate());
+  Realm* realm = Realm::GetCurrent(context);
+  // Recreate the buffer in the constructor.
+  InternalFieldInfo* casted_info = static_cast<InternalFieldInfo*>(info);
+  BindingData* binding =
+      realm->AddBindingData<BindingData>(context, holder, casted_info);
+  CHECK_NOT_NULL(binding);
+}
+
+void BindingData::MemoryInfo(MemoryTracker* tracker) const {
+  tracker->TrackField("is_building_snapshot_buffer",
+                      is_building_snapshot_buffer_);
+}
+
+void CreatePerContextProperties(Local<Object> target,
+                                Local<Value> unused,
+                                Local<Context> context,
+                                void* priv) {
+  Realm* realm = Realm::GetCurrent(context);
+  realm->AddBindingData<BindingData>(context, target);
+}
+
+void CreatePerIsolateProperties(IsolateData* isolate_data,
+                                Local<FunctionTemplate> ctor) {
+  Isolate* isolate = isolate_data->isolate();
+  Local<ObjectTemplate> target = ctor->PrototypeTemplate();
   SetMethod(
-      context, target, "getEmbedderEntryFunction", GetEmbedderEntryFunction);
-  SetMethod(context, target, "compileSerializeMain", CompileSerializeMain);
-  SetMethod(context, target, "setSerializeCallback", SetSerializeCallback);
-  SetMethod(context, target, "setDeserializeCallback", SetDeserializeCallback);
-  SetMethod(context,
+      isolate, target, "getEmbedderEntryFunction", GetEmbedderEntryFunction);
+  SetMethod(isolate, target, "compileSerializeMain", CompileSerializeMain);
+  SetMethod(isolate, target, "setSerializeCallback", SetSerializeCallback);
+  SetMethod(isolate, target, "setDeserializeCallback", SetDeserializeCallback);
+  SetMethod(isolate,
             target,
             "setDeserializeMainFunction",
             SetDeserializeMainFunction);
@@ -1274,8 +1347,12 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(SetDeserializeMainFunction);
 }
 }  // namespace mksnapshot
+
 }  // namespace node
 
-NODE_BINDING_CONTEXT_AWARE_INTERNAL(mksnapshot, node::mksnapshot::Initialize)
+NODE_BINDING_CONTEXT_AWARE_INTERNAL(
+    mksnapshot, node::mksnapshot::CreatePerContextProperties)
+NODE_BINDING_PER_ISOLATE_INIT(mksnapshot,
+                              node::mksnapshot::CreatePerIsolateProperties)
 NODE_BINDING_EXTERNAL_REFERENCE(mksnapshot,
                                 node::mksnapshot::RegisterExternalReferences)
diff --git a/src/node_snapshotable.h b/src/node_snapshotable.h
@@ -4,6 +4,7 @@
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
+#include "aliased_buffer.h"
 #include "base_object.h"
 #include "util.h"
 
@@ -121,6 +122,31 @@ void DeserializeNodeInternalFields(v8::Local<v8::Object> holder,
 void SerializeSnapshotableObjects(Realm* realm,
                                   v8::SnapshotCreator* creator,
                                   RealmSerializeInfo* info);
+
+namespace mksnapshot {
+class BindingData : public SnapshotableObject {
+ public:
+  struct InternalFieldInfo : public node::InternalFieldInfoBase {
+    AliasedBufferIndex is_building_snapshot_buffer;
+  };
+
+  BindingData(Realm* realm,
+              v8::Local<v8::Object> obj,
+              InternalFieldInfo* info = nullptr);
+  SET_BINDING_ID(mksnapshot_binding_data)
+  SERIALIZABLE_OBJECT_METHODS()
+
+  void MemoryInfo(MemoryTracker* tracker) const override;
+  SET_SELF_SIZE(BindingData)
+  SET_MEMORY_INFO_NAME(BindingData)
+
+ private:
+  AliasedUint8Array is_building_snapshot_buffer_;
+  InternalFieldInfo* internal_field_info_ = nullptr;
+};
+
+}  // namespace mksnapshot
+
 }  // namespace node
 
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

diff --git a/src/node_worker.cc b/src/node_worker.cc
@@ -186,6 +186,7 @@ class WorkerThreadData {
                             allocator.get(),
                             w->snapshot_data()->AsEmbedderWrapper().get()));
       CHECK(isolate_data_);
+      CHECK(!isolate_data_->is_building_snapshot());
       if (w_->per_isolate_opts_)
         isolate_data_->set_options(std::move(w_->per_isolate_opts_));
       isolate_data_->set_worker_context(w_);
@@ -481,6 +482,7 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
     THROW_ERR_MISSING_PLATFORM_FOR_WORKER(env);
     return;
   }
+  CHECK(!env->isolate_data()->is_building_snapshot());
 
   std::string url;
   std::string name;