Skip to content

Commit 89dd093

Browse files
joyeecheungjoyeecheung
authored
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: #48996 Refs: nodejs/build#3043 Reviewed-By: Rafael Gonzaga <[email protected]>
1 parent 9a2e6bc commit 89dd093

File tree

11 files changed

+84
-41
lines changed

11 files changed

+84
-41
lines changed

‎src/encoding_binding.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ bool BindingData::PrepareForSerialization(Local<Context> context,
6262
}
6363

6464
InternalFieldInfoBase* BindingData::Serialize(int index) {
65-
DCHECK_EQ(index, BaseObject::kEmbedderType);
65+
DCHECK_IS_SNAPSHOT_SLOT(index);
6666
InternalFieldInfo* info = internal_field_info_;
6767
internal_field_info_ = nullptr;
6868
return info;
@@ -72,7 +72,7 @@ void BindingData::Deserialize(Local<Context> context,
7272
Local<Object> holder,
7373
int index,
7474
InternalFieldInfoBase* info) {
75-
DCHECK_EQ(index, BaseObject::kEmbedderType);
75+
DCHECK_IS_SNAPSHOT_SLOT(index);
7676
v8::HandleScope scope(context->GetIsolate());
7777
Realm* realm = Realm::GetCurrent(context);
7878
// Recreate the buffer in the constructor.

‎src/env.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "node_options-inl.h"
1313
#include "node_process-inl.h"
1414
#include "node_shadow_realm.h"
15+
#include "node_snapshotable.h"
1516
#include "node_v8_platform-inl.h"
1617
#include "node_worker.h"
1718
#include "req_wrap-inl.h"
@@ -1761,7 +1762,7 @@ void Environment::EnqueueDeserializeRequest(DeserializeRequestCallback cb,
17611762
Local<Object> holder,
17621763
int index,
17631764
InternalFieldInfoBase* info) {
1764-
DCHECK_EQ(index, BaseObject::kEmbedderType);
1765+
DCHECK_IS_SNAPSHOT_SLOT(index);
17651766
DeserializeRequest request{cb, {isolate(), holder}, index, info};
17661767
deserialize_requests_.push_back(std::move(request));
17671768
}

‎src/node_blob.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,7 @@ void BlobBindingData::Deserialize(Local<Context> context,
532532
Local<Object> holder,
533533
int index,
534534
InternalFieldInfoBase* info) {
535-
DCHECK_EQ(index, BaseObject::kEmbedderType);
535+
DCHECK_IS_SNAPSHOT_SLOT(index);
536536
HandleScope scope(context->GetIsolate());
537537
Realm* realm = Realm::GetCurrent(context);
538538
BlobBindingData* binding = realm->AddBindingData<BlobBindingData>(holder);
@@ -548,7 +548,7 @@ bool BlobBindingData::PrepareForSerialization(Local<Context> context,
548548
}
549549

550550
InternalFieldInfoBase* BlobBindingData::Serialize(int index) {
551-
DCHECK_EQ(index, BaseObject::kEmbedderType);
551+
DCHECK_IS_SNAPSHOT_SLOT(index);
552552
InternalFieldInfo* info =
553553
InternalFieldInfoBase::New<InternalFieldInfo>(type());
554554
return info;

‎src/node_file.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3116,7 +3116,7 @@ void BindingData::Deserialize(Local<Context> context,
31163116
Local<Object> holder,
31173117
int index,
31183118
InternalFieldInfoBase* info) {
3119-
DCHECK_EQ(index, BaseObject::kEmbedderType);
3119+
DCHECK_IS_SNAPSHOT_SLOT(index);
31203120
HandleScope scope(context->GetIsolate());
31213121
Realm* realm = Realm::GetCurrent(context);
31223122
InternalFieldInfo* casted_info = static_cast<InternalFieldInfo*>(info);
@@ -3144,7 +3144,7 @@ bool BindingData::PrepareForSerialization(Local<Context> context,
31443144
}
31453145

31463146
InternalFieldInfoBase* BindingData::Serialize(int index) {
3147-
DCHECK_EQ(index, BaseObject::kEmbedderType);
3147+
DCHECK_IS_SNAPSHOT_SLOT(index);
31483148
InternalFieldInfo* info = internal_field_info_;
31493149
internal_field_info_ = nullptr;
31503150
return info;

‎src/node_process_methods.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -552,7 +552,7 @@ bool BindingData::PrepareForSerialization(Local<Context> context,
552552
}
553553

554554
InternalFieldInfoBase* BindingData::Serialize(int index) {
555-
DCHECK_EQ(index, BaseObject::kEmbedderType);
555+
DCHECK_IS_SNAPSHOT_SLOT(index);
556556
InternalFieldInfo* info =
557557
InternalFieldInfoBase::New<InternalFieldInfo>(type());
558558
return info;
@@ -562,7 +562,7 @@ void BindingData::Deserialize(Local<Context> context,
562562
Local<Object> holder,
563563
int index,
564564
InternalFieldInfoBase* info) {
565-
DCHECK_EQ(index, BaseObject::kEmbedderType);
565+
DCHECK_IS_SNAPSHOT_SLOT(index);
566566
v8::HandleScope scope(context->GetIsolate());
567567
Realm* realm = Realm::GetCurrent(context);
568568
// Recreate the buffer in the constructor.

‎src/node_snapshotable.cc‎

Lines changed: 56 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1146,25 +1146,33 @@ std::string SnapshotableObject::GetTypeName() const {
11461146
void DeserializeNodeInternalFields(Local<Object> holder,
11471147
int index,
11481148
StartupData payload,
1149-
void* env) {
1149+
void* callback_data) {
11501150
if (payload.raw_size == 0) {
1151-
holder->SetAlignedPointerInInternalField(index, nullptr);
11521151
return;
11531152
}
1153+
11541154
per_process::Debug(DebugCategory::MKSNAPSHOT,
11551155
"Deserialize internal field %d of %p, size=%d\n",
11561156
static_cast<int>(index),
11571157
(*holder),
11581158
static_cast<int>(payload.raw_size));
11591159

1160-
if (payload.raw_size == 0) {
1161-
holder->SetAlignedPointerInInternalField(index, nullptr);
1160+
Environment* env = static_cast<Environment*>(callback_data);
1161+
1162+
// To deserialize the first field, check the type and re-tag the object.
1163+
if (index == BaseObject::kEmbedderType) {
1164+
int size = sizeof(EmbedderTypeInfo);
1165+
DCHECK_EQ(payload.raw_size, size);
1166+
EmbedderTypeInfo read_data;
1167+
memcpy(&read_data, payload.data, size);
1168+
// For now we only support non-cppgc objects.
1169+
CHECK_EQ(read_data.mode, EmbedderTypeInfo::MemoryMode::kBaseObject);
1170+
BaseObject::TagBaseObject(env->isolate_data(), holder);
11621171
return;
11631172
}
11641173

1165-
DCHECK_EQ(index, BaseObject::kEmbedderType);
1166-
1167-
Environment* env_ptr = static_cast<Environment*>(env);
1174+
// To deserialize the second field, enqueue a deserialize request.
1175+
DCHECK_IS_SNAPSHOT_SLOT(index);
11681176
const InternalFieldInfoBase* info =
11691177
reinterpret_cast<const InternalFieldInfoBase*>(payload.data);
11701178
// TODO(joyeecheung): we can add a constant kNodeEmbedderId to the
@@ -1177,7 +1185,7 @@ void DeserializeNodeInternalFields(Local<Object> holder,
11771185
"Object %p is %s\n", \
11781186
(*holder), \
11791187
#NativeTypeName); \
1180-
env_ptr->EnqueueDeserializeRequest( \
1188+
env->EnqueueDeserializeRequest( \
11811189
NativeTypeName::Deserialize, \
11821190
holder, \
11831191
index, \
@@ -1203,28 +1211,52 @@ void DeserializeNodeInternalFields(Local<Object> holder,
12031211
StartupData SerializeNodeContextInternalFields(Local<Object> holder,
12041212
int index,
12051213
void* callback_data) {
1206-
// We only do one serialization for the kEmbedderType slot, the result
1207-
// contains everything necessary for deserializing the entire object,
1208-
// including the fields whose index is bigger than kEmbedderType
1209-
// (most importantly, BaseObject::kSlot).
1210-
// For Node.js this design is enough for all the native binding that are
1211-
// serializable.
1214+
// For the moment we do not set any internal fields in ArrayBuffer
1215+
// or ArrayBufferViews, so just return nullptr.
1216+
if (holder->IsArrayBuffer() || holder->IsArrayBufferView()) {
1217+
CHECK_NULL(holder->GetAlignedPointerFromInternalField(index));
1218+
return StartupData{nullptr, 0};
1219+
}
1220+
1221+
// Use the V8 convention and serialize unknown objects verbatim.
12121222
Environment* env = static_cast<Environment*>(callback_data);
1213-
if (index != BaseObject::kEmbedderType ||
1214-
!BaseObject::IsBaseObject(env->isolate_data(), holder)) {
1223+
if (!BaseObject::IsBaseObject(env->isolate_data(), holder)) {
1224+
per_process::Debug(DebugCategory::MKSNAPSHOT,
1225+
"Serialize unknown object, index=%d, holder=%p\n",
1226+
static_cast<int>(index),
1227+
*holder);
12151228
return StartupData{nullptr, 0};
12161229
}
12171230

12181231
per_process::Debug(DebugCategory::MKSNAPSHOT,
1219-
"Serialize internal field, index=%d, holder=%p\n",
1232+
"Serialize BaseObject, index=%d, holder=%p\n",
12201233
static_cast<int>(index),
12211234
*holder);
12221235

1223-
void* native_ptr =
1224-
holder->GetAlignedPointerFromInternalField(BaseObject::kSlot);
1225-
per_process::Debug(DebugCategory::MKSNAPSHOT, "native = %p\n", native_ptr);
1226-
DCHECK(static_cast<BaseObject*>(native_ptr)->is_snapshotable());
1227-
SnapshotableObject* obj = static_cast<SnapshotableObject*>(native_ptr);
1236+
BaseObject* object_ptr = static_cast<BaseObject*>(
1237+
holder->GetAlignedPointerFromInternalField(BaseObject::kSlot));
1238+
// If the native object is already set to null, ignore it.
1239+
if (object_ptr == nullptr) {
1240+
return StartupData{nullptr, 0};
1241+
}
1242+
1243+
DCHECK(object_ptr->is_snapshotable());
1244+
SnapshotableObject* obj = static_cast<SnapshotableObject*>(object_ptr);
1245+
1246+
// To serialize the type field, save data in a EmbedderTypeInfo.
1247+
if (index == BaseObject::kEmbedderType) {
1248+
int size = sizeof(EmbedderTypeInfo);
1249+
char* data = new char[size];
1250+
// We need to use placement new because V8 calls delete[] on the returned
1251+
// data.
1252+
// TODO(joyeecheung): support cppgc objects.
1253+
new (data) EmbedderTypeInfo(obj->type(),
1254+
EmbedderTypeInfo::MemoryMode::kBaseObject);
1255+
return StartupData{data, size};
1256+
}
1257+
1258+
// To serialize the slot field, invoke Serialize() method on the object.
1259+
DCHECK_IS_SNAPSHOT_SLOT(index);
12281260

12291261
per_process::Debug(DebugCategory::MKSNAPSHOT,
12301262
"Object %p is %s, ",
@@ -1380,7 +1412,7 @@ bool BindingData::PrepareForSerialization(Local<Context> context,
13801412
}
13811413

13821414
InternalFieldInfoBase* BindingData::Serialize(int index) {
1383-
DCHECK_EQ(index, BaseObject::kEmbedderType);
1415+
DCHECK_IS_SNAPSHOT_SLOT(index);
13841416
InternalFieldInfo* info = internal_field_info_;
13851417
internal_field_info_ = nullptr;
13861418
return info;
@@ -1390,7 +1422,7 @@ void BindingData::Deserialize(Local<Context> context,
13901422
Local<Object> holder,
13911423
int index,
13921424
InternalFieldInfoBase* info) {
1393-
DCHECK_EQ(index, BaseObject::kEmbedderType);
1425+
DCHECK_IS_SNAPSHOT_SLOT(index);
13941426
v8::HandleScope scope(context->GetIsolate());
13951427
Realm* realm = Realm::GetCurrent(context);
13961428
// Recreate the buffer in the constructor.

‎src/node_snapshotable.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,14 @@ struct InternalFieldInfoBase {
6868
InternalFieldInfoBase() = default;
6969
};
7070

71+
struct EmbedderTypeInfo {
72+
enum class MemoryMode : uint8_t { kBaseObject, kCppGC };
73+
EmbedderTypeInfo(EmbedderObjectType t, MemoryMode m) : type(t), mode(m) {}
74+
EmbedderTypeInfo() = default;
75+
EmbedderObjectType type;
76+
MemoryMode mode;
77+
};
78+
7179
// An interface for snapshotable native objects to inherit from.
7280
// Use the SERIALIZABLE_OBJECT_METHODS() macro in the class to define
7381
// the following methods to implement:
@@ -123,6 +131,8 @@ void SerializeSnapshotableObjects(Realm* realm,
123131
v8::SnapshotCreator* creator,
124132
RealmSerializeInfo* info);
125133

134+
#define DCHECK_IS_SNAPSHOT_SLOT(index) DCHECK_EQ(index, BaseObject::kSlot)
135+
126136
namespace mksnapshot {
127137
class BindingData : public SnapshotableObject {
128138
public:

‎src/node_url.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ bool BindingData::PrepareForSerialization(v8::Local<v8::Context> context,
5454
}
5555

5656
InternalFieldInfoBase* BindingData::Serialize(int index) {
57-
DCHECK_EQ(index, BaseObject::kEmbedderType);
57+
DCHECK_IS_SNAPSHOT_SLOT(index);
5858
InternalFieldInfo* info =
5959
InternalFieldInfoBase::New<InternalFieldInfo>(type());
6060
return info;
@@ -64,7 +64,7 @@ void BindingData::Deserialize(v8::Local<v8::Context> context,
6464
v8::Local<v8::Object> holder,
6565
int index,
6666
InternalFieldInfoBase* info) {
67-
DCHECK_EQ(index, BaseObject::kEmbedderType);
67+
DCHECK_IS_SNAPSHOT_SLOT(index);
6868
v8::HandleScope scope(context->GetIsolate());
6969
Realm* realm = Realm::GetCurrent(context);
7070
BindingData* binding = realm->AddBindingData<BindingData>(holder);

‎src/node_util.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ bool WeakReference::PrepareForSerialization(Local<Context> context,
246246
}
247247

248248
InternalFieldInfoBase* WeakReference::Serialize(int index) {
249-
DCHECK_EQ(index, BaseObject::kEmbedderType);
249+
DCHECK_IS_SNAPSHOT_SLOT(index);
250250
InternalFieldInfo* info =
251251
InternalFieldInfoBase::New<InternalFieldInfo>(type());
252252
info->target = target_index_;
@@ -258,7 +258,7 @@ void WeakReference::Deserialize(Local<Context> context,
258258
Local<Object> holder,
259259
int index,
260260
InternalFieldInfoBase* info) {
261-
DCHECK_EQ(index, BaseObject::kEmbedderType);
261+
DCHECK_IS_SNAPSHOT_SLOT(index);
262262
HandleScope scope(context->GetIsolate());
263263

264264
InternalFieldInfo* weak_info = reinterpret_cast<InternalFieldInfo*>(info);

‎src/node_v8.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ void BindingData::Deserialize(Local<Context> context,
152152
Local<Object> holder,
153153
int index,
154154
InternalFieldInfoBase* info) {
155-
DCHECK_EQ(index, BaseObject::kEmbedderType);
155+
DCHECK_IS_SNAPSHOT_SLOT(index);
156156
HandleScope scope(context->GetIsolate());
157157
Realm* realm = Realm::GetCurrent(context);
158158
// Recreate the buffer in the constructor.
@@ -163,7 +163,7 @@ void BindingData::Deserialize(Local<Context> context,
163163
}
164164

165165
InternalFieldInfoBase* BindingData::Serialize(int index) {
166-
DCHECK_EQ(index, BaseObject::kEmbedderType);
166+
DCHECK_IS_SNAPSHOT_SLOT(index);
167167
InternalFieldInfo* info = internal_field_info_;
168168
internal_field_info_ = nullptr;
169169
return info;

0 commit comments

Comments
 (0)