From e9946885f9be4767168294333c3d128b8835166d Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 15 Aug 2023 20:32:14 +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. PR-URL: https://github.com/nodejs/node/pull/48996 Refs: https://github.com/nodejs/build/issues/3043 Reviewed-By: Rafael Gonzaga --- 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 7e3d3aca2d5f96..20b404996ccd0f 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" @@ -1760,7 +1761,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 e4a3b2fe8b0f98..8dc81a6f15e867 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 4c21cc7467ccf2..bde29316888042 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -3151,7 +3151,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); @@ -3179,7 +3179,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 1f066c7d5bb9ff..af85ea10a94163 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -1146,25 +1146,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 @@ -1177,7 +1185,7 @@ void DeserializeNodeInternalFields(Local holder, "Object %p is %s\n", \ (*holder), \ #NativeTypeName); \ - env_ptr->EnqueueDeserializeRequest( \ + env->EnqueueDeserializeRequest( \ NativeTypeName::Deserialize, \ holder, \ index, \ @@ -1203,28 +1211,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, ", @@ -1380,7 +1412,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; @@ -1390,7 +1422,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 f055acd51c323c..da8790c1d1843e 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 e8cb28969621fb..ec637dcbd78861 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -246,7 +246,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_; @@ -258,7 +258,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.