src: iterate over base objects to prepare for snapshot

src/env-inl.h

@@ -835,17 +835,6 @@ void Environment::ForEachBaseObject(T&& iterator) {
   }
 }
 
-template <typename T>
-void Environment::ForEachBindingData(T&& iterator) {
-  BindingDataStore* map = static_cast<BindingDataStore*>(
-      context()->GetAlignedPointerFromEmbedderData(
-          ContextEmbedderIndex::kBindingListIndex));
-  DCHECK_NOT_NULL(map);
-  for (auto& it : *map) {
-    iterator(it.first, it.second);
-  }
-}
-
 void Environment::modify_base_object_count(int64_t delta) {
   base_object_count_ += delta;
 }

src/env.cc

@@ -1654,7 +1654,6 @@ EnvSerializeInfo Environment::Serialize(SnapshotCreator* creator) {
   EnvSerializeInfo info;
   Local<Context> ctx = context();
 
-  SerializeBindingData(this, creator, &info);
   // Currently all modules are compiled without cache in builtin snapshot
   // builder.
   info.builtins = std::vector<std::string>(builtins_without_cache.begin(),
@@ -1682,6 +1681,10 @@ EnvSerializeInfo Environment::Serialize(SnapshotCreator* creator) {
   ENVIRONMENT_STRONG_PERSISTENT_VALUES(V)
 #undef V
 
+  // Do this after other creator->AddData() calls so that Snapshotable objects
+  // can use 0 to indicate that a SnapshotIndex is invalid.
+  SerializeSnapshotableObjects(this, creator, &info);
+
   info.context = creator->AddData(ctx, context());
   return info;
 }
@@ -1714,9 +1717,9 @@ std::ostream& operator<<(std::ostream& output,
 
 std::ostream& operator<<(std::ostream& output, const EnvSerializeInfo& i) {
   output << "{\n"
-         << "// -- bindings begins --\n"
-         << i.bindings << ",\n"
-         << "// -- bindings ends --\n"
+         << "// -- native_objects begins --\n"
+         << i.native_objects << ",\n"
+         << "// -- native_objects ends --\n"
          << "// -- builtins begins --\n"
          << i.builtins << ",\n"
          << "// -- builtins ends --\n"
@@ -1743,7 +1746,7 @@ std::ostream& operator<<(std::ostream& output, const EnvSerializeInfo& i) {
 void Environment::EnqueueDeserializeRequest(DeserializeRequestCallback cb,
                                             Local<Object> holder,
                                             int index,
-                                            InternalFieldInfo* info) {
+                                            InternalFieldInfoBase* info) {
   DCHECK_EQ(index, BaseObject::kEmbedderType);
   DeserializeRequest request{cb, {isolate(), holder}, index, info};
   deserialize_requests_.push_back(std::move(request));

src/env.h

@@ -956,19 +956,19 @@ class CleanupHookCallback {
 typedef void (*DeserializeRequestCallback)(v8::Local<v8::Context> context,
                                            v8::Local<v8::Object> holder,
                                            int index,
-                                           InternalFieldInfo* info);
+                                           InternalFieldInfoBase* info);
 struct DeserializeRequest {
   DeserializeRequestCallback cb;
   v8::Global<v8::Object> holder;
   int index;
-  InternalFieldInfo* info = nullptr;  // Owned by the request
+  InternalFieldInfoBase* info = nullptr;  // Owned by the request
 
   // Move constructor
   DeserializeRequest(DeserializeRequest&& other) = default;
 };
 
 struct EnvSerializeInfo {
-  std::vector<PropInfo> bindings;
+  std::vector<PropInfo> native_objects;
   std::vector<std::string> builtins;
   AsyncHooks::SerializeInfo async_hooks;
   TickInfo::SerializeInfo tick_info;
@@ -1044,7 +1044,7 @@ class Environment : public MemoryRetainer {
   void EnqueueDeserializeRequest(DeserializeRequestCallback cb,
                                  v8::Local<v8::Object> holder,
                                  int index,
-                                 InternalFieldInfo* info);
+                                 InternalFieldInfoBase* info);
   void RunDeserializeRequests();
   // Should be called before InitializeInspector()
   void InitializeDiagnostics();
@@ -1467,7 +1467,7 @@ class Environment : public MemoryRetainer {
   void RemoveUnmanagedFd(int fd);
 
   template <typename T>
-  void ForEachBindingData(T&& iterator);
+  void ForEachBaseObject(T&& iterator);
 
  private:
   inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
@@ -1625,9 +1625,6 @@ class Environment : public MemoryRetainer {
   std::function<void(Environment*, int)> process_exit_handler_ {
       DefaultProcessExitHandler };
 
-  template <typename T>
-  void ForEachBaseObject(T&& iterator);
-
 #define V(PropertyName, TypeName) v8::Global<TypeName> PropertyName ## _;
   ENVIRONMENT_STRONG_PERSISTENT_VALUES(V)
 #undef V

src/node_blob.cc

@@ -462,11 +462,10 @@ BlobBindingData::StoredDataObject BlobBindingData::get_data_object(
   return entry->second;
 }
 
-void BlobBindingData::Deserialize(
-    Local<Context> context,
-    Local<Object> holder,
-    int index,
-    InternalFieldInfo* info) {
+void BlobBindingData::Deserialize(Local<Context> context,
+                                  Local<Object> holder,
+                                  int index,
+                                  InternalFieldInfoBase* info) {
   DCHECK_EQ(index, BaseObject::kEmbedderType);
   HandleScope scope(context->GetIsolate());
   Environment* env = Environment::GetCurrent(context);
@@ -475,15 +474,18 @@ void BlobBindingData::Deserialize(
   CHECK_NOT_NULL(binding);
 }
 
-void BlobBindingData::PrepareForSerialization(
-    Local<Context> context,
-    v8::SnapshotCreator* creator) {
+bool BlobBindingData::PrepareForSerialization(Local<Context> context,
+                                              v8::SnapshotCreator* creator) {
   // Stored blob objects are not actually persisted.
+  // Return true because we need to maintain the reference to the binding from
+  // JS land.
+  return true;
 }
 
-InternalFieldInfo* BlobBindingData::Serialize(int index) {
+InternalFieldInfoBase* BlobBindingData::Serialize(int index) {
   DCHECK_EQ(index, BaseObject::kEmbedderType);
-  InternalFieldInfo* info = InternalFieldInfo::New(type());
+  InternalFieldInfo* info =
+      InternalFieldInfoBase::New<InternalFieldInfo>(type());
   return info;
 }
 

src/node_blob.h

@@ -146,6 +146,8 @@ class BlobBindingData : public SnapshotableObject {
  public:
   explicit BlobBindingData(Environment* env, v8::Local<v8::Object> wrap);
 
+  using InternalFieldInfo = InternalFieldInfoBase;
+
   SERIALIZABLE_OBJECT_METHODS()
 
   static constexpr FastStringKey type_name{"node::BlobBindingData"};

src/node_file.cc

@@ -2400,26 +2400,30 @@ BindingData::BindingData(Environment* env, v8::Local<v8::Object> wrap)
 void BindingData::Deserialize(Local<Context> context,
                               Local<Object> holder,
                               int index,
-                              InternalFieldInfo* info) {
+                              InternalFieldInfoBase* info) {
   DCHECK_EQ(index, BaseObject::kEmbedderType);
   HandleScope scope(context->GetIsolate());
   Environment* env = Environment::GetCurrent(context);
   BindingData* binding = env->AddBindingData<BindingData>(context, holder);
   CHECK_NOT_NULL(binding);
 }
 
-void BindingData::PrepareForSerialization(Local<Context> context,
+bool BindingData::PrepareForSerialization(Local<Context> context,
                                           v8::SnapshotCreator* creator) {
   CHECK(file_handle_read_wrap_freelist.empty());
   // We'll just re-initialize the buffers in the constructor since their
   // contents can be thrown away once consumed in the previous call.
   stats_field_array.Release();
   stats_field_bigint_array.Release();
+  // Return true because we need to maintain the reference to the binding from
+  // JS land.
+  return true;
 }
 
-InternalFieldInfo* BindingData::Serialize(int index) {
+InternalFieldInfoBase* BindingData::Serialize(int index) {
   DCHECK_EQ(index, BaseObject::kEmbedderType);
-  InternalFieldInfo* info = InternalFieldInfo::New(type());
+  InternalFieldInfo* info =
+      InternalFieldInfoBase::New<InternalFieldInfo>(type());
   return info;
 }
 

src/node_file.h

@@ -23,6 +23,7 @@ class BindingData : public SnapshotableObject {
   std::vector<BaseObjectPtr<FileHandleReadWrap>>
       file_handle_read_wrap_freelist;
 
+  using InternalFieldInfo = InternalFieldInfoBase;
   SERIALIZABLE_OBJECT_METHODS()
   static constexpr FastStringKey type_name{"node::fs::BindingData"};
   static constexpr EmbedderObjectType type_int =

src/node_process.h

@@ -50,6 +50,8 @@ class BindingData : public SnapshotableObject {
   void AddMethods();
   static void RegisterExternalReferences(ExternalReferenceRegistry* registry);
 
+  using InternalFieldInfo = InternalFieldInfoBase;
+
   SERIALIZABLE_OBJECT_METHODS()
   static constexpr FastStringKey type_name{"node::process::BindingData"};
   static constexpr EmbedderObjectType type_int =

src/node_process_methods.cc

@@ -524,23 +524,27 @@ void BindingData::SlowNumber(const v8::FunctionCallbackInfo<v8::Value>& args) {
   NumberImpl(FromJSObject<BindingData>(args.Holder()));
 }
 
-void BindingData::PrepareForSerialization(Local<Context> context,
+bool BindingData::PrepareForSerialization(Local<Context> context,
                                           v8::SnapshotCreator* creator) {
   // It's not worth keeping.
   // Release it, we will recreate it when the instance is dehydrated.
   array_buffer_.Reset();
+  // Return true because we need to maintain the reference to the binding from
+  // JS land.
+  return true;
 }
 
-InternalFieldInfo* BindingData::Serialize(int index) {
+InternalFieldInfoBase* BindingData::Serialize(int index) {
   DCHECK_EQ(index, BaseObject::kEmbedderType);
-  InternalFieldInfo* info = InternalFieldInfo::New(type());
+  InternalFieldInfo* info =
+      InternalFieldInfoBase::New<InternalFieldInfo>(type());
   return info;
 }
 
 void BindingData::Deserialize(Local<Context> context,
                               Local<Object> holder,
                               int index,
-                              InternalFieldInfo* info) {
+                              InternalFieldInfoBase* info) {
   DCHECK_EQ(index, BaseObject::kEmbedderType);
   v8::HandleScope scope(context->GetIsolate());
   Environment* env = Environment::GetCurrent(context);

src/node_snapshotable.cc

@@ -638,7 +638,7 @@ template <>
 EnvSerializeInfo FileReader::Read() {
   per_process::Debug(DebugCategory::MKSNAPSHOT, "Read<EnvSerializeInfo>()\n");
   EnvSerializeInfo result;
-  result.bindings = ReadVector<PropInfo>();
+  result.native_objects = ReadVector<PropInfo>();
   result.builtins = ReadVector<std::string>();
   result.async_hooks = Read<AsyncHooks::SerializeInfo>();
   result.tick_info = Read<TickInfo::SerializeInfo>();
@@ -661,7 +661,7 @@ size_t FileWriter::Write(const EnvSerializeInfo& data) {
   }
 
   // Use += here to ensure order of evaluation.
-  size_t written_total = WriteVector<PropInfo>(data.bindings);
+  size_t written_total = WriteVector<PropInfo>(data.native_objects);
   written_total += WriteVector<std::string>(data.builtins);
   written_total += Write<AsyncHooks::SerializeInfo>(data.async_hooks);
   written_total += Write<TickInfo::SerializeInfo>(data.tick_info);
@@ -1065,17 +1065,6 @@ const char* SnapshotableObject::GetTypeNameChars() const {
   }
 }
 
-bool IsSnapshotableType(FastStringKey key) {
-#define V(PropertyName, NativeTypeName)                                        \
-  if (key == NativeTypeName::type_name) {                                      \
-    return true;                                                               \
-  }
-  SERIALIZABLE_OBJECT_TYPES(V)
-#undef V
-
-  return false;
-}
-
 void DeserializeNodeInternalFields(Local<Object> holder,
                                    int index,
                                    StartupData payload,
@@ -1098,10 +1087,10 @@ void DeserializeNodeInternalFields(Local<Object> holder,
   DCHECK_EQ(index, BaseObject::kEmbedderType);
 
   Environment* env_ptr = static_cast<Environment*>(env);
-  const InternalFieldInfo* info =
-      reinterpret_cast<const InternalFieldInfo*>(payload.data);
+  const InternalFieldInfoBase* info =
+      reinterpret_cast<const InternalFieldInfoBase*>(payload.data);
   // TODO(joyeecheung): we can add a constant kNodeEmbedderId to the
-  // beginning of every InternalFieldInfo to ensure that we don't
+  // beginning of every InternalFieldInfoBase to ensure that we don't
   // step on payloads that were not serialized by Node.js.
   switch (info->type) {
 #define V(PropertyName, NativeTypeName)                                        \
@@ -1111,12 +1100,25 @@ void DeserializeNodeInternalFields(Local<Object> holder,
                        (*holder),                                              \
                        NativeTypeName::type_name.c_str());                     \
     env_ptr->EnqueueDeserializeRequest(                                        \
-        NativeTypeName::Deserialize, holder, index, info->Copy());             \
+        NativeTypeName::Deserialize,                                           \
+        holder,                                                                \
+        index,                                                                 \
+        info->Copy<NativeTypeName::InternalFieldInfo>());                      \
     break;                                                                     \
   }
     SERIALIZABLE_OBJECT_TYPES(V)
 #undef V
-    default: { UNREACHABLE(); }
+    default: {
+      // This should only be reachable during development when trying to
+      // deserialize a snapshot blob built by a version of Node.js that
+      // has more recognizable EmbedderObjectTypes than the deserializing
+      // Node.js binary.
+      fprintf(stderr,
+              "Unknown embedder object type %" PRIu8 ", possibly caused by "
+              "mismatched Node.js versions\n",
+              static_cast<uint8_t>(info->type));
+      ABORT();
+    }
   }
 }
 
@@ -1149,17 +1151,17 @@ StartupData SerializeNodeContextInternalFields(Local<Object> holder,
                      static_cast<int>(index),
                      *holder);
 
-  void* binding_ptr =
+  void* native_ptr =
       holder->GetAlignedPointerFromInternalField(BaseObject::kSlot);
-  per_process::Debug(DebugCategory::MKSNAPSHOT, "binding = %p\n", binding_ptr);
-  DCHECK(static_cast<BaseObject*>(binding_ptr)->is_snapshotable());
-  SnapshotableObject* obj = static_cast<SnapshotableObject*>(binding_ptr);
+  per_process::Debug(DebugCategory::MKSNAPSHOT, "native = %p\n", native_ptr);
+  DCHECK(static_cast<BaseObject*>(native_ptr)->is_snapshotable());
+  SnapshotableObject* obj = static_cast<SnapshotableObject*>(native_ptr);
 
   per_process::Debug(DebugCategory::MKSNAPSHOT,
                      "Object %p is %s, ",
                      *holder,
                      obj->GetTypeNameChars());
-  InternalFieldInfo* info = obj->Serialize(index);
+  InternalFieldInfoBase* info = obj->Serialize(index);
 
   per_process::Debug(DebugCategory::MKSNAPSHOT,
                      "payload size=%d\n",
@@ -1168,31 +1170,35 @@ StartupData SerializeNodeContextInternalFields(Local<Object> holder,
                      static_cast<int>(info->length)};
 }
 
-void SerializeBindingData(Environment* env,
-                          SnapshotCreator* creator,
-                          EnvSerializeInfo* info) {
+void SerializeSnapshotableObjects(Environment* env,
+                                  SnapshotCreator* creator,
+                                  EnvSerializeInfo* info) {
   uint32_t i = 0;
-  env->ForEachBindingData([&](FastStringKey key,
-                              BaseObjectPtr<BaseObject> binding) {
+  env->ForEachBaseObject([&](BaseObject* obj) {
+    // If there are any BaseObjects that are not snapshotable left
+    // during context serialization, V8 would crash due to unregistered
+    // global handles and print detailed information about them.
+    if (!obj->is_snapshotable()) {
+      return;
+    }
+    SnapshotableObject* ptr = static_cast<SnapshotableObject*>(obj);
+
+    const char* type_name = ptr->GetTypeNameChars();
     per_process::Debug(DebugCategory::MKSNAPSHOT,
-                       "Serialize binding %i (%p), object=%p, type=%s\n",
+                       "Serialize snapshotable object %i (%p), "
+                       "object=%p, type=%s\n",
                        static_cast<int>(i),
-                       binding.get(),
-                       *(binding->object()),
-                       key.c_str());
+                       ptr,
+                       *(ptr->object()),
+                       type_name);
 
-    if (IsSnapshotableType(key)) {
-      SnapshotIndex index = creator->AddData(env->context(), binding->object());
+    if (ptr->PrepareForSerialization(env->context(), creator)) {
+      SnapshotIndex index = creator->AddData(env->context(), obj->object());
       per_process::Debug(DebugCategory::MKSNAPSHOT,
                          "Serialized with index=%d\n",
                          static_cast<int>(index));
-      info->bindings.push_back({key.c_str(), i, index});
-      SnapshotableObject* ptr = static_cast<SnapshotableObject*>(binding.get());
-      ptr->PrepareForSerialization(env->context(), creator);
-    } else {
-      UNREACHABLE();
+      info->native_objects.push_back({type_name, i, index});
     }
-
     i++;
   });
 }