Skip to content

Commit 3359c2c

Browse files
joyeecheungdanielleadams
joyeecheung
authored and
danielleadams
committed
src: iterate over base objects to prepare for snapshot
Instead of iterating over the bindings, iterate over the base objects that are snapshottable. This allows us to snapshot base objects that are not bindings. In addition this refactors the InternalFieldInfo class to eliminate potential undefined behaviors, and renames it to InternalFieldInfoBase. The {de}serialize callbacks now expect a InternalFieldInfo struct nested in Snapshotable classes that can be used to carry serialization data around. This allows us to create structs inheriting from InternalFieldInfo for Snapshotable objects that need custom fields. PR-URL: #44192 Refs: #37476 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
1 parent 83a7ffe commit 3359c2c

13 files changed

+139
-116
lines changed

‎src/env-inl.h

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -835,17 +835,6 @@ void Environment::ForEachBaseObject(T&& iterator) {
835835
}
836836
}
837837

838-
template <typename T>
839-
void Environment::ForEachBindingData(T&& iterator) {
840-
BindingDataStore* map = static_cast<BindingDataStore*>(
841-
context()->GetAlignedPointerFromEmbedderData(
842-
ContextEmbedderIndex::kBindingListIndex));
843-
DCHECK_NOT_NULL(map);
844-
for (auto& it : *map) {
845-
iterator(it.first, it.second);
846-
}
847-
}
848-
849838
void Environment::modify_base_object_count(int64_t delta) {
850839
base_object_count_ += delta;
851840
}

‎src/env.cc

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1809,7 +1809,6 @@ EnvSerializeInfo Environment::Serialize(SnapshotCreator* creator) {
18091809
EnvSerializeInfo info;
18101810
Local<Context> ctx = context();
18111811

1812-
SerializeBindingData(this, creator, &info);
18131812
// Currently all modules are compiled without cache in builtin snapshot
18141813
// builder.
18151814
info.builtins = std::vector<std::string>(builtins_without_cache.begin(),
@@ -1837,6 +1836,10 @@ EnvSerializeInfo Environment::Serialize(SnapshotCreator* creator) {
18371836
ENVIRONMENT_STRONG_PERSISTENT_VALUES(V)
18381837
#undef V
18391838

1839+
// Do this after other creator->AddData() calls so that Snapshotable objects
1840+
// can use 0 to indicate that a SnapshotIndex is invalid.
1841+
SerializeSnapshotableObjects(this, creator, &info);
1842+
18401843
info.context = creator->AddData(ctx, context());
18411844
return info;
18421845
}
@@ -1869,9 +1872,9 @@ std::ostream& operator<<(std::ostream& output,
18691872

18701873
std::ostream& operator<<(std::ostream& output, const EnvSerializeInfo& i) {
18711874
output << "{\n"
1872-
<< "// -- bindings begins --\n"
1873-
<< i.bindings << ",\n"
1874-
<< "// -- bindings ends --\n"
1875+
<< "// -- native_objects begins --\n"
1876+
<< i.native_objects << ",\n"
1877+
<< "// -- native_objects ends --\n"
18751878
<< "// -- builtins begins --\n"
18761879
<< i.builtins << ",\n"
18771880
<< "// -- builtins ends --\n"
@@ -1898,7 +1901,7 @@ std::ostream& operator<<(std::ostream& output, const EnvSerializeInfo& i) {
18981901
void Environment::EnqueueDeserializeRequest(DeserializeRequestCallback cb,
18991902
Local<Object> holder,
19001903
int index,
1901-
InternalFieldInfo* info) {
1904+
InternalFieldInfoBase* info) {
19021905
DCHECK_EQ(index, BaseObject::kEmbedderType);
19031906
DeserializeRequest request{cb, {isolate(), holder}, index, info};
19041907
deserialize_requests_.push_back(std::move(request));

‎src/env.h

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -956,19 +956,19 @@ class CleanupHookCallback {
956956
typedef void (*DeserializeRequestCallback)(v8::Local<v8::Context> context,
957957
v8::Local<v8::Object> holder,
958958
int index,
959-
InternalFieldInfo* info);
959+
InternalFieldInfoBase* info);
960960
struct DeserializeRequest {
961961
DeserializeRequestCallback cb;
962962
v8::Global<v8::Object> holder;
963963
int index;
964-
InternalFieldInfo* info = nullptr; // Owned by the request
964+
InternalFieldInfoBase* info = nullptr; // Owned by the request
965965

966966
// Move constructor
967967
DeserializeRequest(DeserializeRequest&& other) = default;
968968
};
969969

970970
struct EnvSerializeInfo {
971-
std::vector<PropInfo> bindings;
971+
std::vector<PropInfo> native_objects;
972972
std::vector<std::string> builtins;
973973
AsyncHooks::SerializeInfo async_hooks;
974974
TickInfo::SerializeInfo tick_info;
@@ -1062,7 +1062,7 @@ class Environment : public MemoryRetainer {
10621062
void EnqueueDeserializeRequest(DeserializeRequestCallback cb,
10631063
v8::Local<v8::Object> holder,
10641064
int index,
1065-
InternalFieldInfo* info);
1065+
InternalFieldInfoBase* info);
10661066
void RunDeserializeRequests();
10671067
// Should be called before InitializeInspector()
10681068
void InitializeDiagnostics();
@@ -1528,7 +1528,7 @@ class Environment : public MemoryRetainer {
15281528
void RemoveUnmanagedFd(int fd);
15291529

15301530
template <typename T>
1531-
void ForEachBindingData(T&& iterator);
1531+
void ForEachBaseObject(T&& iterator);
15321532

15331533
private:
15341534
inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
@@ -1683,9 +1683,6 @@ class Environment : public MemoryRetainer {
16831683
std::function<void(Environment*, int)> process_exit_handler_ {
16841684
DefaultProcessExitHandler };
16851685

1686-
template <typename T>
1687-
void ForEachBaseObject(T&& iterator);
1688-
16891686
#define V(PropertyName, TypeName) v8::Global<TypeName> PropertyName ## _;
16901687
ENVIRONMENT_STRONG_PERSISTENT_VALUES(V)
16911688
#undef V

‎src/node_blob.cc

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -459,11 +459,10 @@ BlobBindingData::StoredDataObject BlobBindingData::get_data_object(
459459
return entry->second;
460460
}
461461

462-
void BlobBindingData::Deserialize(
463-
Local<Context> context,
464-
Local<Object> holder,
465-
int index,
466-
InternalFieldInfo* info) {
462+
void BlobBindingData::Deserialize(Local<Context> context,
463+
Local<Object> holder,
464+
int index,
465+
InternalFieldInfoBase* info) {
467466
DCHECK_EQ(index, BaseObject::kEmbedderType);
468467
HandleScope scope(context->GetIsolate());
469468
Environment* env = Environment::GetCurrent(context);
@@ -472,15 +471,18 @@ void BlobBindingData::Deserialize(
472471
CHECK_NOT_NULL(binding);
473472
}
474473

475-
void BlobBindingData::PrepareForSerialization(
476-
Local<Context> context,
477-
v8::SnapshotCreator* creator) {
474+
bool BlobBindingData::PrepareForSerialization(Local<Context> context,
475+
v8::SnapshotCreator* creator) {
478476
// Stored blob objects are not actually persisted.
477+
// Return true because we need to maintain the reference to the binding from
478+
// JS land.
479+
return true;
479480
}
480481

481-
InternalFieldInfo* BlobBindingData::Serialize(int index) {
482+
InternalFieldInfoBase* BlobBindingData::Serialize(int index) {
482483
DCHECK_EQ(index, BaseObject::kEmbedderType);
483-
InternalFieldInfo* info = InternalFieldInfo::New(type());
484+
InternalFieldInfo* info =
485+
InternalFieldInfoBase::New<InternalFieldInfo>(type());
484486
return info;
485487
}
486488

‎src/node_blob.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,8 @@ class BlobBindingData : public SnapshotableObject {
146146
public:
147147
explicit BlobBindingData(Environment* env, v8::Local<v8::Object> wrap);
148148

149+
using InternalFieldInfo = InternalFieldInfoBase;
150+
149151
SERIALIZABLE_OBJECT_METHODS()
150152

151153
static constexpr FastStringKey type_name{"node::BlobBindingData"};

‎src/node_file.cc

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2400,26 +2400,30 @@ BindingData::BindingData(Environment* env, v8::Local<v8::Object> wrap)
24002400
void BindingData::Deserialize(Local<Context> context,
24012401
Local<Object> holder,
24022402
int index,
2403-
InternalFieldInfo* info) {
2403+
InternalFieldInfoBase* info) {
24042404
DCHECK_EQ(index, BaseObject::kEmbedderType);
24052405
HandleScope scope(context->GetIsolate());
24062406
Environment* env = Environment::GetCurrent(context);
24072407
BindingData* binding = env->AddBindingData<BindingData>(context, holder);
24082408
CHECK_NOT_NULL(binding);
24092409
}
24102410

2411-
void BindingData::PrepareForSerialization(Local<Context> context,
2411+
bool BindingData::PrepareForSerialization(Local<Context> context,
24122412
v8::SnapshotCreator* creator) {
24132413
CHECK(file_handle_read_wrap_freelist.empty());
24142414
// We'll just re-initialize the buffers in the constructor since their
24152415
// contents can be thrown away once consumed in the previous call.
24162416
stats_field_array.Release();
24172417
stats_field_bigint_array.Release();
2418+
// Return true because we need to maintain the reference to the binding from
2419+
// JS land.
2420+
return true;
24182421
}
24192422

2420-
InternalFieldInfo* BindingData::Serialize(int index) {
2423+
InternalFieldInfoBase* BindingData::Serialize(int index) {
24212424
DCHECK_EQ(index, BaseObject::kEmbedderType);
2422-
InternalFieldInfo* info = InternalFieldInfo::New(type());
2425+
InternalFieldInfo* info =
2426+
InternalFieldInfoBase::New<InternalFieldInfo>(type());
24232427
return info;
24242428
}
24252429

‎src/node_file.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ class BindingData : public SnapshotableObject {
2323
std::vector<BaseObjectPtr<FileHandleReadWrap>>
2424
file_handle_read_wrap_freelist;
2525

26+
using InternalFieldInfo = InternalFieldInfoBase;
2627
SERIALIZABLE_OBJECT_METHODS()
2728
static constexpr FastStringKey type_name{"node::fs::BindingData"};
2829
static constexpr EmbedderObjectType type_int =

‎src/node_process.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ class BindingData : public SnapshotableObject {
5050
void AddMethods();
5151
static void RegisterExternalReferences(ExternalReferenceRegistry* registry);
5252

53+
using InternalFieldInfo = InternalFieldInfoBase;
54+
5355
SERIALIZABLE_OBJECT_METHODS()
5456
static constexpr FastStringKey type_name{"node::process::BindingData"};
5557
static constexpr EmbedderObjectType type_int =

‎src/node_process_methods.cc

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -523,23 +523,27 @@ void BindingData::SlowNumber(const v8::FunctionCallbackInfo<v8::Value>& args) {
523523
NumberImpl(FromJSObject<BindingData>(args.Holder()));
524524
}
525525

526-
void BindingData::PrepareForSerialization(Local<Context> context,
526+
bool BindingData::PrepareForSerialization(Local<Context> context,
527527
v8::SnapshotCreator* creator) {
528528
// It's not worth keeping.
529529
// Release it, we will recreate it when the instance is dehydrated.
530530
array_buffer_.Reset();
531+
// Return true because we need to maintain the reference to the binding from
532+
// JS land.
533+
return true;
531534
}
532535

533-
InternalFieldInfo* BindingData::Serialize(int index) {
536+
InternalFieldInfoBase* BindingData::Serialize(int index) {
534537
DCHECK_EQ(index, BaseObject::kEmbedderType);
535-
InternalFieldInfo* info = InternalFieldInfo::New(type());
538+
InternalFieldInfo* info =
539+
InternalFieldInfoBase::New<InternalFieldInfo>(type());
536540
return info;
537541
}
538542

539543
void BindingData::Deserialize(Local<Context> context,
540544
Local<Object> holder,
541545
int index,
542-
InternalFieldInfo* info) {
546+
InternalFieldInfoBase* info) {
543547
DCHECK_EQ(index, BaseObject::kEmbedderType);
544548
v8::HandleScope scope(context->GetIsolate());
545549
Environment* env = Environment::GetCurrent(context);

‎src/node_snapshotable.cc

Lines changed: 46 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -638,7 +638,7 @@ template <>
638638
EnvSerializeInfo FileReader::Read() {
639639
per_process::Debug(DebugCategory::MKSNAPSHOT, "Read<EnvSerializeInfo>()\n");
640640
EnvSerializeInfo result;
641-
result.bindings = ReadVector<PropInfo>();
641+
result.native_objects = ReadVector<PropInfo>();
642642
result.builtins = ReadVector<std::string>();
643643
result.async_hooks = Read<AsyncHooks::SerializeInfo>();
644644
result.tick_info = Read<TickInfo::SerializeInfo>();
@@ -661,7 +661,7 @@ size_t FileWriter::Write(const EnvSerializeInfo& data) {
661661
}
662662

663663
// Use += here to ensure order of evaluation.
664-
size_t written_total = WriteVector<PropInfo>(data.bindings);
664+
size_t written_total = WriteVector<PropInfo>(data.native_objects);
665665
written_total += WriteVector<std::string>(data.builtins);
666666
written_total += Write<AsyncHooks::SerializeInfo>(data.async_hooks);
667667
written_total += Write<TickInfo::SerializeInfo>(data.tick_info);
@@ -1179,17 +1179,6 @@ const char* SnapshotableObject::GetTypeNameChars() const {
11791179
}
11801180
}
11811181

1182-
bool IsSnapshotableType(FastStringKey key) {
1183-
#define V(PropertyName, NativeTypeName) \
1184-
if (key == NativeTypeName::type_name) { \
1185-
return true; \
1186-
}
1187-
SERIALIZABLE_OBJECT_TYPES(V)
1188-
#undef V
1189-
1190-
return false;
1191-
}
1192-
11931182
void DeserializeNodeInternalFields(Local<Object> holder,
11941183
int index,
11951184
StartupData payload,
@@ -1212,10 +1201,10 @@ void DeserializeNodeInternalFields(Local<Object> holder,
12121201
DCHECK_EQ(index, BaseObject::kEmbedderType);
12131202

12141203
Environment* env_ptr = static_cast<Environment*>(env);
1215-
const InternalFieldInfo* info =
1216-
reinterpret_cast<const InternalFieldInfo*>(payload.data);
1204+
const InternalFieldInfoBase* info =
1205+
reinterpret_cast<const InternalFieldInfoBase*>(payload.data);
12171206
// TODO(joyeecheung): we can add a constant kNodeEmbedderId to the
1218-
// beginning of every InternalFieldInfo to ensure that we don't
1207+
// beginning of every InternalFieldInfoBase to ensure that we don't
12191208
// step on payloads that were not serialized by Node.js.
12201209
switch (info->type) {
12211210
#define V(PropertyName, NativeTypeName) \
@@ -1225,12 +1214,25 @@ void DeserializeNodeInternalFields(Local<Object> holder,
12251214
(*holder), \
12261215
NativeTypeName::type_name.c_str()); \
12271216
env_ptr->EnqueueDeserializeRequest( \
1228-
NativeTypeName::Deserialize, holder, index, info->Copy()); \
1217+
NativeTypeName::Deserialize, \
1218+
holder, \
1219+
index, \
1220+
info->Copy<NativeTypeName::InternalFieldInfo>()); \
12291221
break; \
12301222
}
12311223
SERIALIZABLE_OBJECT_TYPES(V)
12321224
#undef V
1233-
default: { UNREACHABLE(); }
1225+
default: {
1226+
// This should only be reachable during development when trying to
1227+
// deserialize a snapshot blob built by a version of Node.js that
1228+
// has more recognizable EmbedderObjectTypes than the deserializing
1229+
// Node.js binary.
1230+
fprintf(stderr,
1231+
"Unknown embedder object type %" PRIu8 ", possibly caused by "
1232+
"mismatched Node.js versions\n",
1233+
static_cast<uint8_t>(info->type));
1234+
ABORT();
1235+
}
12341236
}
12351237
}
12361238

@@ -1263,17 +1265,17 @@ StartupData SerializeNodeContextInternalFields(Local<Object> holder,
12631265
static_cast<int>(index),
12641266
*holder);
12651267

1266-
void* binding_ptr =
1268+
void* native_ptr =
12671269
holder->GetAlignedPointerFromInternalField(BaseObject::kSlot);
1268-
per_process::Debug(DebugCategory::MKSNAPSHOT, "binding = %p\n", binding_ptr);
1269-
DCHECK(static_cast<BaseObject*>(binding_ptr)->is_snapshotable());
1270-
SnapshotableObject* obj = static_cast<SnapshotableObject*>(binding_ptr);
1270+
per_process::Debug(DebugCategory::MKSNAPSHOT, "native = %p\n", native_ptr);
1271+
DCHECK(static_cast<BaseObject*>(native_ptr)->is_snapshotable());
1272+
SnapshotableObject* obj = static_cast<SnapshotableObject*>(native_ptr);
12711273

12721274
per_process::Debug(DebugCategory::MKSNAPSHOT,
12731275
"Object %p is %s, ",
12741276
*holder,
12751277
obj->GetTypeNameChars());
1276-
InternalFieldInfo* info = obj->Serialize(index);
1278+
InternalFieldInfoBase* info = obj->Serialize(index);
12771279

12781280
per_process::Debug(DebugCategory::MKSNAPSHOT,
12791281
"payload size=%d\n",
@@ -1282,31 +1284,35 @@ StartupData SerializeNodeContextInternalFields(Local<Object> holder,
12821284
static_cast<int>(info->length)};
12831285
}
12841286

1285-
void SerializeBindingData(Environment* env,
1286-
SnapshotCreator* creator,
1287-
EnvSerializeInfo* info) {
1287+
void SerializeSnapshotableObjects(Environment* env,
1288+
SnapshotCreator* creator,
1289+
EnvSerializeInfo* info) {
12881290
uint32_t i = 0;
1289-
env->ForEachBindingData([&](FastStringKey key,
1290-
BaseObjectPtr<BaseObject> binding) {
1291+
env->ForEachBaseObject([&](BaseObject* obj) {
1292+
// If there are any BaseObjects that are not snapshotable left
1293+
// during context serialization, V8 would crash due to unregistered
1294+
// global handles and print detailed information about them.
1295+
if (!obj->is_snapshotable()) {
1296+
return;
1297+
}
1298+
SnapshotableObject* ptr = static_cast<SnapshotableObject*>(obj);
1299+
1300+
const char* type_name = ptr->GetTypeNameChars();
12911301
per_process::Debug(DebugCategory::MKSNAPSHOT,
1292-
"Serialize binding %i (%p), object=%p, type=%s\n",
1302+
"Serialize snapshotable object %i (%p), "
1303+
"object=%p, type=%s\n",
12931304
static_cast<int>(i),
1294-
binding.get(),
1295-
*(binding->object()),
1296-
key.c_str());
1305+
ptr,
1306+
*(ptr->object()),
1307+
type_name);
12971308

1298-
if (IsSnapshotableType(key)) {
1299-
SnapshotIndex index = creator->AddData(env->context(), binding->object());
1309+
if (ptr->PrepareForSerialization(env->context(), creator)) {
1310+
SnapshotIndex index = creator->AddData(env->context(), obj->object());
13001311
per_process::Debug(DebugCategory::MKSNAPSHOT,
13011312
"Serialized with index=%d\n",
13021313
static_cast<int>(index));
1303-
info->bindings.push_back({key.c_str(), i, index});
1304-
SnapshotableObject* ptr = static_cast<SnapshotableObject*>(binding.get());
1305-
ptr->PrepareForSerialization(env->context(), creator);
1306-
} else {
1307-
UNREACHABLE();
1314+
info->native_objects.push_back({type_name, i, index});
13081315
}
1309-
13101316
i++;
13111317
});
13121318
}

0 commit comments

Comments
 (0)