From 2f249ef6dde5602d68587666fb2e6b40f5cc8985 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sun, 9 Jul 2023 11:15:49 +0200 Subject: [PATCH] src: return startup data for both BaseObject slots We previously only return startup data for the first slot for BaseObjects because we can already serialize all the necessary information in one go, but slots that do not get special startup data would be serialized verbatim which means that the pointer addresses are going to be part of the snapshot blob, resulting in indeterminism. This patch updates the serialization routines and capture information for both of the two slots - the first slot with type information and memory management type (which we can use in the future for cppgc-managed objects) and the second slot with data about the object itself. This way the embeedder slots can be serialized in a reproducible manner in the snapshot. --- src/encoding_binding.cc | 4 +- src/env.cc | 3 +- src/node_blob.cc | 4 +- src/node_file.cc | 4 +- src/node_process_methods.cc | 4 +- src/node_snapshotable.cc | 80 ++++++++++++++++++++++++++----------- src/node_snapshotable.h | 10 +++++ src/node_url.cc | 4 +- src/node_util.cc | 4 +- src/node_v8.cc | 4 +- src/timers.cc | 4 +- 11 files changed, 84 insertions(+), 41 deletions(-) diff --git a/src/encoding_binding.cc b/src/encoding_binding.cc index b65a4f868e2b26..c67af5319c8ff5 100644 --- a/src/encoding_binding.cc +++ b/src/encoding_binding.cc @@ -62,7 +62,7 @@ bool BindingData::PrepareForSerialization(Local context, } InternalFieldInfoBase* BindingData::Serialize(int index) { - DCHECK_EQ(index, BaseObject::kEmbedderType); + DCHECK_IS_SNAPSHOT_SLOT(index); InternalFieldInfo* info = internal_field_info_; internal_field_info_ = nullptr; return info; @@ -72,7 +72,7 @@ void BindingData::Deserialize(Local context, Local holder, int index, InternalFieldInfoBase* info) { - DCHECK_EQ(index, BaseObject::kEmbedderType); + DCHECK_IS_SNAPSHOT_SLOT(index); v8::HandleScope scope(context->GetIsolate()); Realm* realm = Realm::GetCurrent(context); // Recreate the buffer in the constructor. diff --git a/src/env.cc b/src/env.cc index cc0c7b2642c106..a3fc1be73c8f52 100644 --- a/src/env.cc +++ b/src/env.cc @@ -12,6 +12,7 @@ #include "node_options-inl.h" #include "node_process-inl.h" #include "node_shadow_realm.h" +#include "node_snapshotable.h" #include "node_v8_platform-inl.h" #include "node_worker.h" #include "req_wrap-inl.h" @@ -1735,7 +1736,7 @@ void Environment::EnqueueDeserializeRequest(DeserializeRequestCallback cb, Local holder, int index, InternalFieldInfoBase* info) { - DCHECK_EQ(index, BaseObject::kEmbedderType); + DCHECK_IS_SNAPSHOT_SLOT(index); DeserializeRequest request{cb, {isolate(), holder}, index, info}; deserialize_requests_.push_back(std::move(request)); } diff --git a/src/node_blob.cc b/src/node_blob.cc index c92bfb12c6b063..eef9fde18e0e5e 100644 --- a/src/node_blob.cc +++ b/src/node_blob.cc @@ -532,7 +532,7 @@ void BlobBindingData::Deserialize(Local context, Local holder, int index, InternalFieldInfoBase* info) { - DCHECK_EQ(index, BaseObject::kEmbedderType); + DCHECK_IS_SNAPSHOT_SLOT(index); HandleScope scope(context->GetIsolate()); Realm* realm = Realm::GetCurrent(context); BlobBindingData* binding = realm->AddBindingData(holder); @@ -548,7 +548,7 @@ bool BlobBindingData::PrepareForSerialization(Local context, } InternalFieldInfoBase* BlobBindingData::Serialize(int index) { - DCHECK_EQ(index, BaseObject::kEmbedderType); + DCHECK_IS_SNAPSHOT_SLOT(index); InternalFieldInfo* info = InternalFieldInfoBase::New(type()); return info; diff --git a/src/node_file.cc b/src/node_file.cc index 26c0754d010ee0..5fbb5fc1ac7394 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -3113,7 +3113,7 @@ void BindingData::Deserialize(Local context, Local holder, int index, InternalFieldInfoBase* info) { - DCHECK_EQ(index, BaseObject::kEmbedderType); + DCHECK_IS_SNAPSHOT_SLOT(index); HandleScope scope(context->GetIsolate()); Realm* realm = Realm::GetCurrent(context); InternalFieldInfo* casted_info = static_cast(info); @@ -3141,7 +3141,7 @@ bool BindingData::PrepareForSerialization(Local context, } InternalFieldInfoBase* BindingData::Serialize(int index) { - DCHECK_EQ(index, BaseObject::kEmbedderType); + DCHECK_IS_SNAPSHOT_SLOT(index); InternalFieldInfo* info = internal_field_info_; internal_field_info_ = nullptr; return info; diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc index 1b68207f3e3ba6..34d3c3af4c3e10 100644 --- a/src/node_process_methods.cc +++ b/src/node_process_methods.cc @@ -552,7 +552,7 @@ bool BindingData::PrepareForSerialization(Local context, } InternalFieldInfoBase* BindingData::Serialize(int index) { - DCHECK_EQ(index, BaseObject::kEmbedderType); + DCHECK_IS_SNAPSHOT_SLOT(index); InternalFieldInfo* info = InternalFieldInfoBase::New(type()); return info; @@ -562,7 +562,7 @@ void BindingData::Deserialize(Local context, Local holder, int index, InternalFieldInfoBase* info) { - DCHECK_EQ(index, BaseObject::kEmbedderType); + DCHECK_IS_SNAPSHOT_SLOT(index); v8::HandleScope scope(context->GetIsolate()); Realm* realm = Realm::GetCurrent(context); // Recreate the buffer in the constructor. diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index e2d98eb3139740..63c1d615fbc234 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -1107,25 +1107,33 @@ std::string SnapshotableObject::GetTypeName() const { void DeserializeNodeInternalFields(Local holder, int index, StartupData payload, - void* env) { + void* callback_data) { if (payload.raw_size == 0) { - holder->SetAlignedPointerInInternalField(index, nullptr); return; } + per_process::Debug(DebugCategory::MKSNAPSHOT, "Deserialize internal field %d of %p, size=%d\n", static_cast(index), (*holder), static_cast(payload.raw_size)); - if (payload.raw_size == 0) { - holder->SetAlignedPointerInInternalField(index, nullptr); + Environment* env = static_cast(callback_data); + + // To deserialize the first field, check the type and re-tag the object. + if (index == BaseObject::kEmbedderType) { + int size = sizeof(EmbedderTypeInfo); + DCHECK_EQ(payload.raw_size, size); + EmbedderTypeInfo read_data; + memcpy(&read_data, payload.data, size); + // For now we only support non-cppgc objects. + CHECK_EQ(read_data.mode, EmbedderTypeInfo::MemoryMode::kBaseObject); + BaseObject::TagBaseObject(env->isolate_data(), holder); return; } - DCHECK_EQ(index, BaseObject::kEmbedderType); - - Environment* env_ptr = static_cast(env); + // To deserialize the second field, enqueue a deserialize request. + DCHECK_IS_SNAPSHOT_SLOT(index); const InternalFieldInfoBase* info = reinterpret_cast(payload.data); // TODO(joyeecheung): we can add a constant kNodeEmbedderId to the @@ -1138,7 +1146,7 @@ void DeserializeNodeInternalFields(Local holder, "Object %p is %s\n", \ (*holder), \ #NativeTypeName); \ - env_ptr->EnqueueDeserializeRequest( \ + env->EnqueueDeserializeRequest( \ NativeTypeName::Deserialize, \ holder, \ index, \ @@ -1164,28 +1172,52 @@ void DeserializeNodeInternalFields(Local holder, StartupData SerializeNodeContextInternalFields(Local holder, int index, void* callback_data) { - // We only do one serialization for the kEmbedderType slot, the result - // contains everything necessary for deserializing the entire object, - // including the fields whose index is bigger than kEmbedderType - // (most importantly, BaseObject::kSlot). - // For Node.js this design is enough for all the native binding that are - // serializable. + // For the moment we do not set any internal fields in ArrayBuffer + // or ArrayBufferViews, so just return nullptr. + if (holder->IsArrayBuffer() || holder->IsArrayBufferView()) { + CHECK_NULL(holder->GetAlignedPointerFromInternalField(index)); + return StartupData{nullptr, 0}; + } + + // Use the V8 convention and serialize unknown objects verbatim. Environment* env = static_cast(callback_data); - if (index != BaseObject::kEmbedderType || - !BaseObject::IsBaseObject(env->isolate_data(), holder)) { + if (!BaseObject::IsBaseObject(env->isolate_data(), holder)) { + per_process::Debug(DebugCategory::MKSNAPSHOT, + "Serialize unknown object, index=%d, holder=%p\n", + static_cast(index), + *holder); return StartupData{nullptr, 0}; } per_process::Debug(DebugCategory::MKSNAPSHOT, - "Serialize internal field, index=%d, holder=%p\n", + "Serialize BaseObject, index=%d, holder=%p\n", static_cast(index), *holder); - void* native_ptr = - holder->GetAlignedPointerFromInternalField(BaseObject::kSlot); - per_process::Debug(DebugCategory::MKSNAPSHOT, "native = %p\n", native_ptr); - DCHECK(static_cast(native_ptr)->is_snapshotable()); - SnapshotableObject* obj = static_cast(native_ptr); + BaseObject* object_ptr = static_cast( + holder->GetAlignedPointerFromInternalField(BaseObject::kSlot)); + // If the native object is already set to null, ignore it. + if (object_ptr == nullptr) { + return StartupData{nullptr, 0}; + } + + DCHECK(object_ptr->is_snapshotable()); + SnapshotableObject* obj = static_cast(object_ptr); + + // To serialize the type field, save data in a EmbedderTypeInfo. + if (index == BaseObject::kEmbedderType) { + int size = sizeof(EmbedderTypeInfo); + char* data = new char[size]; + // We need to use placement new because V8 calls delete[] on the returned + // data. + // TODO(joyeecheung): support cppgc objects. + new (data) EmbedderTypeInfo(obj->type(), + EmbedderTypeInfo::MemoryMode::kBaseObject); + return StartupData{data, size}; + } + + // To serialize the slot field, invoke Serialize() method on the object. + DCHECK_IS_SNAPSHOT_SLOT(index); per_process::Debug(DebugCategory::MKSNAPSHOT, "Object %p is %s, ", @@ -1341,7 +1373,7 @@ bool BindingData::PrepareForSerialization(Local context, } InternalFieldInfoBase* BindingData::Serialize(int index) { - DCHECK_EQ(index, BaseObject::kEmbedderType); + DCHECK_IS_SNAPSHOT_SLOT(index); InternalFieldInfo* info = internal_field_info_; internal_field_info_ = nullptr; return info; @@ -1351,7 +1383,7 @@ void BindingData::Deserialize(Local context, Local holder, int index, InternalFieldInfoBase* info) { - DCHECK_EQ(index, BaseObject::kEmbedderType); + DCHECK_IS_SNAPSHOT_SLOT(index); v8::HandleScope scope(context->GetIsolate()); Realm* realm = Realm::GetCurrent(context); // Recreate the buffer in the constructor. diff --git a/src/node_snapshotable.h b/src/node_snapshotable.h index eed572beef3a0c..d1f28ecf154d9b 100644 --- a/src/node_snapshotable.h +++ b/src/node_snapshotable.h @@ -68,6 +68,14 @@ struct InternalFieldInfoBase { InternalFieldInfoBase() = default; }; +struct EmbedderTypeInfo { + enum class MemoryMode : uint8_t { kBaseObject, kCppGC }; + EmbedderTypeInfo(EmbedderObjectType t, MemoryMode m) : type(t), mode(m) {} + EmbedderTypeInfo() = default; + EmbedderObjectType type; + MemoryMode mode; +}; + // An interface for snapshotable native objects to inherit from. // Use the SERIALIZABLE_OBJECT_METHODS() macro in the class to define // the following methods to implement: @@ -123,6 +131,8 @@ void SerializeSnapshotableObjects(Realm* realm, v8::SnapshotCreator* creator, RealmSerializeInfo* info); +#define DCHECK_IS_SNAPSHOT_SLOT(index) DCHECK_EQ(index, BaseObject::kSlot) + namespace mksnapshot { class BindingData : public SnapshotableObject { public: diff --git a/src/node_url.cc b/src/node_url.cc index 35a7855035a2f1..5fcaa15917f331 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -54,7 +54,7 @@ bool BindingData::PrepareForSerialization(v8::Local context, } InternalFieldInfoBase* BindingData::Serialize(int index) { - DCHECK_EQ(index, BaseObject::kEmbedderType); + DCHECK_IS_SNAPSHOT_SLOT(index); InternalFieldInfo* info = InternalFieldInfoBase::New(type()); return info; @@ -64,7 +64,7 @@ void BindingData::Deserialize(v8::Local context, v8::Local holder, int index, InternalFieldInfoBase* info) { - DCHECK_EQ(index, BaseObject::kEmbedderType); + DCHECK_IS_SNAPSHOT_SLOT(index); v8::HandleScope scope(context->GetIsolate()); Realm* realm = Realm::GetCurrent(context); BindingData* binding = realm->AddBindingData(holder); diff --git a/src/node_util.cc b/src/node_util.cc index 2af3b5a7276aae..b8fd4a25359c2a 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -226,7 +226,7 @@ bool WeakReference::PrepareForSerialization(Local context, } InternalFieldInfoBase* WeakReference::Serialize(int index) { - DCHECK_EQ(index, BaseObject::kEmbedderType); + DCHECK_IS_SNAPSHOT_SLOT(index); InternalFieldInfo* info = InternalFieldInfoBase::New(type()); info->target = target_index_; @@ -238,7 +238,7 @@ void WeakReference::Deserialize(Local context, Local holder, int index, InternalFieldInfoBase* info) { - DCHECK_EQ(index, BaseObject::kEmbedderType); + DCHECK_IS_SNAPSHOT_SLOT(index); HandleScope scope(context->GetIsolate()); InternalFieldInfo* weak_info = reinterpret_cast(info); diff --git a/src/node_v8.cc b/src/node_v8.cc index a5e91f5b8ca624..814efe3d69651c 100644 --- a/src/node_v8.cc +++ b/src/node_v8.cc @@ -152,7 +152,7 @@ void BindingData::Deserialize(Local context, Local holder, int index, InternalFieldInfoBase* info) { - DCHECK_EQ(index, BaseObject::kEmbedderType); + DCHECK_IS_SNAPSHOT_SLOT(index); HandleScope scope(context->GetIsolate()); Realm* realm = Realm::GetCurrent(context); // Recreate the buffer in the constructor. @@ -163,7 +163,7 @@ void BindingData::Deserialize(Local context, } InternalFieldInfoBase* BindingData::Serialize(int index) { - DCHECK_EQ(index, BaseObject::kEmbedderType); + DCHECK_IS_SNAPSHOT_SLOT(index); InternalFieldInfo* info = internal_field_info_; internal_field_info_ = nullptr; return info; diff --git a/src/timers.cc b/src/timers.cc index 27fa18ec4d3f86..127806fbcdfd3e 100644 --- a/src/timers.cc +++ b/src/timers.cc @@ -94,7 +94,7 @@ bool BindingData::PrepareForSerialization(Local context, } InternalFieldInfoBase* BindingData::Serialize(int index) { - DCHECK_EQ(index, BaseObject::kEmbedderType); + DCHECK_IS_SNAPSHOT_SLOT(index); InternalFieldInfo* info = InternalFieldInfoBase::New(type()); return info; @@ -104,7 +104,7 @@ void BindingData::Deserialize(Local context, Local holder, int index, InternalFieldInfoBase* info) { - DCHECK_EQ(index, BaseObject::kEmbedderType); + DCHECK_IS_SNAPSHOT_SLOT(index); v8::HandleScope scope(context->GetIsolate()); Realm* realm = Realm::GetCurrent(context); // Recreate the buffer in the constructor.