From 152fce68e9a7e016f8f89896a1b97df21e7e34fa Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sun, 9 Jul 2023 11:15:49 +0200 Subject: [PATCH] src: serialize 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 b65a4f868e2b..c67af5319c8f 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 cc0c7b2642c1..a3fc1be73c8f 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 c92bfb12c6b0..eef9fde18e0e 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 26c0754d010e..5fbb5fc1ac73 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 1b68207f3e3b..34d3c3af4c3e 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 e2d98eb31397..63c1d615fbc2 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 eed572beef3a..d1f28ecf154d 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 35a7855035a2..5fcaa15917f3 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 2af3b5a7276a..b8fd4a25359c 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 a5e91f5b8ca6..814efe3d6965 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 27fa18ec4d3f..127806fbcdfd 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.