Skip to content

Commit 8959013

Browse files
joyeecheungjoyeecheung
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: nodejs#44192 Refs: nodejs#37476 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
1 parent 418a35b commit 8959013

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
@@ -826,17 +826,6 @@ void Environment::ForEachBaseObject(T&& iterator) {
826826
}
827827
}
828828

829-
template <typename T>
830-
void Environment::ForEachBindingData(T&& iterator) {
831-
BindingDataStore* map = static_cast<BindingDataStore*>(
832-
context()->GetAlignedPointerFromEmbedderData(
833-
ContextEmbedderIndex::kBindingListIndex));
834-
DCHECK_NOT_NULL(map);
835-
for (auto& it : *map) {
836-
iterator(it.first, it.second);
837-
}
838-
}
839-
840829
void Environment::modify_base_object_count(int64_t delta) {
841830
base_object_count_ += delta;
842831
}

‎src/env.cc

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

1669-
SerializeBindingData(this, creator, &info);
16701669
// Currently all modules are compiled without cache in builtin snapshot
16711670
// builder.
16721671
info.builtins = std::vector<std::string>(builtins_without_cache.begin(),
@@ -1694,6 +1693,10 @@ EnvSerializeInfo Environment::Serialize(SnapshotCreator* creator) {
16941693
ENVIRONMENT_STRONG_PERSISTENT_VALUES(V)
16951694
#undef V
16961695

1696+
// Do this after other creator->AddData() calls so that Snapshotable objects
1697+
// can use 0 to indicate that a SnapshotIndex is invalid.
1698+
SerializeSnapshotableObjects(this, creator, &info);
1699+
16971700
info.context = creator->AddData(ctx, context());
16981701
return info;
16991702
}
@@ -1726,9 +1729,9 @@ std::ostream& operator<<(std::ostream& output,
17261729

17271730
std::ostream& operator<<(std::ostream& output, const EnvSerializeInfo& i) {
17281731
output << "{\n"
1729-
<< "// -- bindings begins --\n"
1730-
<< i.bindings << ",\n"
1731-
<< "// -- bindings ends --\n"
1732+
<< "// -- native_objects begins --\n"
1733+
<< i.native_objects << ",\n"
1734+
<< "// -- native_objects ends --\n"
17321735
<< "// -- builtins begins --\n"
17331736
<< i.builtins << ",\n"
17341737
<< "// -- builtins ends --\n"
@@ -1755,7 +1758,7 @@ std::ostream& operator<<(std::ostream& output, const EnvSerializeInfo& i) {
17551758
void Environment::EnqueueDeserializeRequest(DeserializeRequestCallback cb,
17561759
Local<Object> holder,
17571760
int index,
1758-
InternalFieldInfo* info) {
1761+
InternalFieldInfoBase* info) {
17591762
DCHECK_EQ(index, BaseObject::kEmbedderType);
17601763
DeserializeRequest request{cb, {isolate(), holder}, index, info};
17611764
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();
@@ -1478,7 +1478,7 @@ class Environment : public MemoryRetainer {
14781478
void RemoveUnmanagedFd(int fd);
14791479

14801480
template <typename T>
1481-
void ForEachBindingData(T&& iterator);
1481+
void ForEachBaseObject(T&& iterator);
14821482

14831483
private:
14841484
inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
@@ -1630,9 +1630,6 @@ class Environment : public MemoryRetainer {
16301630
std::function<void(Environment*, int)> process_exit_handler_ {
16311631
DefaultProcessExitHandler };
16321632

1633-
template <typename T>
1634-
void ForEachBaseObject(T&& iterator);
1635-
16361633
#define V(PropertyName, TypeName) v8::Global<TypeName> PropertyName ## _;
16371634
ENVIRONMENT_STRONG_PERSISTENT_VALUES(V)
16381635
#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)