Skip to content

Commit e994688

Browse files
joyeecheungUlisesGascon
authored andcommitted
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 <rafael.nunu@hotmail.com>
1 parent 30f26a9 commit e994688

11 files changed

+84
-41
lines changed

src/encoding_binding.cc

+2-2
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

+2-1
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"
@@ -1760,7 +1761,7 @@ void Environment::EnqueueDeserializeRequest(DeserializeRequestCallback cb,
17601761
Local<Object> holder,
17611762
int index,
17621763
InternalFieldInfoBase* info) {
1763-
DCHECK_EQ(index, BaseObject::kEmbedderType);
1764+
DCHECK_IS_SNAPSHOT_SLOT(index);
17641765
DeserializeRequest request{cb, {isolate(), holder}, index, info};
17651766
deserialize_requests_.push_back(std::move(request));
17661767
}

src/node_blob.cc

+2-2
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

+2-2
Original file line numberDiff line numberDiff line change
@@ -3151,7 +3151,7 @@ void BindingData::Deserialize(Local<Context> context,
31513151
Local<Object> holder,
31523152
int index,
31533153
InternalFieldInfoBase* info) {
3154-
DCHECK_EQ(index, BaseObject::kEmbedderType);
3154+
DCHECK_IS_SNAPSHOT_SLOT(index);
31553155
HandleScope scope(context->GetIsolate());
31563156
Realm* realm = Realm::GetCurrent(context);
31573157
InternalFieldInfo* casted_info = static_cast<InternalFieldInfo*>(info);
@@ -3179,7 +3179,7 @@ bool BindingData::PrepareForSerialization(Local<Context> context,
31793179
}
31803180

31813181
InternalFieldInfoBase* BindingData::Serialize(int index) {
3182-
DCHECK_EQ(index, BaseObject::kEmbedderType);
3182+
DCHECK_IS_SNAPSHOT_SLOT(index);
31833183
InternalFieldInfo* info = internal_field_info_;
31843184
internal_field_info_ = nullptr;
31853185
return info;

src/node_process_methods.cc

+2-2
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

+56-24
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

+10
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

+2-2
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

+2-2
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

+2-2
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;

src/timers.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ bool BindingData::PrepareForSerialization(Local<Context> context,
9494
}
9595

9696
InternalFieldInfoBase* BindingData::Serialize(int index) {
97-
DCHECK_EQ(index, BaseObject::kEmbedderType);
97+
DCHECK_IS_SNAPSHOT_SLOT(index);
9898
InternalFieldInfo* info =
9999
InternalFieldInfoBase::New<InternalFieldInfo>(type());
100100
return info;
@@ -104,7 +104,7 @@ void BindingData::Deserialize(Local<Context> context,
104104
Local<Object> holder,
105105
int index,
106106
InternalFieldInfoBase* info) {
107-
DCHECK_EQ(index, BaseObject::kEmbedderType);
107+
DCHECK_IS_SNAPSHOT_SLOT(index);
108108
v8::HandleScope scope(context->GetIsolate());
109109
Realm* realm = Realm::GetCurrent(context);
110110
// Recreate the buffer in the constructor.

0 commit comments

Comments
 (0)