Skip to content

Commit 4ca398a

Browse files
joyeecheungjoyeecheung
authored
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 <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
1 parent 74c2c8e commit 4ca398a

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
@@ -1669,7 +1669,6 @@ EnvSerializeInfo Environment::Serialize(SnapshotCreator* creator) {
16691669
EnvSerializeInfo info;
16701670
Local<Context> ctx = context();
16711671

1672-
SerializeBindingData(this, creator, &info);
16731672
// Currently all modules are compiled without cache in builtin snapshot
16741673
// builder.
16751674
info.builtins = std::vector<std::string>(builtins_without_cache.begin(),
@@ -1697,6 +1696,10 @@ EnvSerializeInfo Environment::Serialize(SnapshotCreator* creator) {
16971696
ENVIRONMENT_STRONG_PERSISTENT_VALUES(V)
16981697
#undef V
16991698

1699+
// Do this after other creator->AddData() calls so that Snapshotable objects
1700+
// can use 0 to indicate that a SnapshotIndex is invalid.
1701+
SerializeSnapshotableObjects(this, creator, &info);
1702+
17001703
info.context = creator->AddData(ctx, context());
17011704
return info;
17021705
}
@@ -1729,9 +1732,9 @@ std::ostream& operator<<(std::ostream& output,
17291732

17301733
std::ostream& operator<<(std::ostream& output, const EnvSerializeInfo& i) {
17311734
output << "{\n"
1732-
<< "// -- bindings begins --\n"
1733-
<< i.bindings << ",\n"
1734-
<< "// -- bindings ends --\n"
1735+
<< "// -- native_objects begins --\n"
1736+
<< i.native_objects << ",\n"
1737+
<< "// -- native_objects ends --\n"
17351738
<< "// -- builtins begins --\n"
17361739
<< i.builtins << ",\n"
17371740
<< "// -- builtins ends --\n"
@@ -1758,7 +1761,7 @@ std::ostream& operator<<(std::ostream& output, const EnvSerializeInfo& i) {
17581761
void Environment::EnqueueDeserializeRequest(DeserializeRequestCallback cb,
17591762
Local<Object> holder,
17601763
int index,
1761-
InternalFieldInfo* info) {
1764+
InternalFieldInfoBase* info) {
17621765
DCHECK_EQ(index, BaseObject::kEmbedderType);
17631766
DeserializeRequest request{cb, {isolate(), holder}, index, info};
17641767
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();
@@ -1485,7 +1485,7 @@ class Environment : public MemoryRetainer {
14851485
void RemoveUnmanagedFd(int fd);
14861486

14871487
template <typename T>
1488-
void ForEachBindingData(T&& iterator);
1488+
void ForEachBaseObject(T&& iterator);
14891489

14901490
private:
14911491
inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
@@ -1643,9 +1643,6 @@ class Environment : public MemoryRetainer {
16431643
std::function<void(Environment*, int)> process_exit_handler_ {
16441644
DefaultProcessExitHandler };
16451645

1646-
template <typename T>
1647-
void ForEachBaseObject(T&& iterator);
1648-
16491646
#define V(PropertyName, TypeName) v8::Global<TypeName> PropertyName ## _;
16501647
ENVIRONMENT_STRONG_PERSISTENT_VALUES(V)
16511648
#undef V

‎src/node_blob.cc

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -462,11 +462,10 @@ BlobBindingData::StoredDataObject BlobBindingData::get_data_object(
462462
return entry->second;
463463
}
464464

465-
void BlobBindingData::Deserialize(
466-
Local<Context> context,
467-
Local<Object> holder,
468-
int index,
469-
InternalFieldInfo* info) {
465+
void BlobBindingData::Deserialize(Local<Context> context,
466+
Local<Object> holder,
467+
int index,
468+
InternalFieldInfoBase* info) {
470469
DCHECK_EQ(index, BaseObject::kEmbedderType);
471470
HandleScope scope(context->GetIsolate());
472471
Environment* env = Environment::GetCurrent(context);
@@ -475,15 +474,18 @@ void BlobBindingData::Deserialize(
475474
CHECK_NOT_NULL(binding);
476475
}
477476

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

484-
InternalFieldInfo* BlobBindingData::Serialize(int index) {
485+
InternalFieldInfoBase* BlobBindingData::Serialize(int index) {
485486
DCHECK_EQ(index, BaseObject::kEmbedderType);
486-
InternalFieldInfo* info = InternalFieldInfo::New(type());
487+
InternalFieldInfo* info =
488+
InternalFieldInfoBase::New<InternalFieldInfo>(type());
487489
return info;
488490
}
489491

‎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
@@ -524,23 +524,27 @@ void BindingData::SlowNumber(const v8::FunctionCallbackInfo<v8::Value>& args) {
524524
NumberImpl(FromJSObject<BindingData>(args.Holder()));
525525
}
526526

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

534-
InternalFieldInfo* BindingData::Serialize(int index) {
537+
InternalFieldInfoBase* BindingData::Serialize(int index) {
535538
DCHECK_EQ(index, BaseObject::kEmbedderType);
536-
InternalFieldInfo* info = InternalFieldInfo::New(type());
539+
InternalFieldInfo* info =
540+
InternalFieldInfoBase::New<InternalFieldInfo>(type());
537541
return info;
538542
}
539543

540544
void BindingData::Deserialize(Local<Context> context,
541545
Local<Object> holder,
542546
int index,
543-
InternalFieldInfo* info) {
547+
InternalFieldInfoBase* info) {
544548
DCHECK_EQ(index, BaseObject::kEmbedderType);
545549
v8::HandleScope scope(context->GetIsolate());
546550
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)