From 22579941753de86fd9fd46558f3e09af27fb6ef9 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 20 Apr 2019 21:35:27 +0800 Subject: [PATCH] src: add an ExternalReferenceRegistry class Add an ExternalReferenceRegistry class for registering static external references. To register the external JS to C++ references created in a binding (e.g. when a FunctionTemplate is created): - Add the binding name (same as the id used for `internalBinding()` and `NODE_MODULE_CONTEXT_AWARE_INTERNAL`) to `EXTERNAL_REFERENCE_BINDING_LIST` in `src/node_external_reference.h`. - In the file where the binding is implemented, create a registration function to register the static C++ references (e.g. the C++ functions in `v8::FunctionCallback` associated with the function templates), like this: ```c++ void RegisterExternalReferences( ExternalReferenceRegistry* registry) { registry->Register(cpp_func_1); } ``` - At the end of the file where `NODE_MODULE_CONTEXT_AWARE_INTERNAL` is also usually called, register the registration function with ``` NODE_MODULE_EXTERNAL_REFERENCE(binding_name, RegisterExternalReferences); ``` PR-URL: https://github.com/nodejs/node/pull/32984 Reviewed-By: Anna Henningsen Reviewed-By: Daniel Bevenius --- node.gyp | 2 + src/node.cc | 1 + src/node_external_reference.cc | 22 ++++++++++ src/node_external_reference.h | 67 ++++++++++++++++++++++++++++++ src/node_main_instance.cc | 15 ++++++- src/node_main_instance.h | 4 ++ tools/snapshot/snapshot_builder.cc | 8 ++-- 7 files changed, 114 insertions(+), 5 deletions(-) create mode 100644 src/node_external_reference.cc create mode 100644 src/node_external_reference.h diff --git a/node.gyp b/node.gyp index 36cc88c629ae4d..acaf2cef8fb10a 100644 --- a/node.gyp +++ b/node.gyp @@ -593,6 +593,7 @@ 'src/node_dir.cc', 'src/node_env_var.cc', 'src/node_errors.cc', + 'src/node_external_reference.cc', 'src/node_file.cc', 'src/node_http_parser.cc', 'src/node_http2.cc', @@ -687,6 +688,7 @@ 'src/node_contextify.h', 'src/node_dir.h', 'src/node_errors.h', + 'src/node_external_reference.h', 'src/node_file.h', 'src/node_file-inl.h', 'src/node_http_common.h', diff --git a/src/node.cc b/src/node.cc index eabee549a68b61..e27edec0efe7d3 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1069,6 +1069,7 @@ int Start(int argc, char** argv) { if (blob != nullptr) { // TODO(joyeecheung): collect external references and set it in // params.external_references. + external_references = NodeMainInstance::CollectExternalReferences(); external_references.push_back(reinterpret_cast(nullptr)); params.external_references = external_references.data(); params.snapshot_blob = blob; diff --git a/src/node_external_reference.cc b/src/node_external_reference.cc new file mode 100644 index 00000000000000..73e1489865d3a4 --- /dev/null +++ b/src/node_external_reference.cc @@ -0,0 +1,22 @@ +#include "node_external_reference.h" +#include +#include +#include "util.h" + +namespace node { + +const std::vector& ExternalReferenceRegistry::external_references() { + CHECK(!is_finalized_); + external_references_.push_back(reinterpret_cast(nullptr)); + is_finalized_ = true; + return external_references_; +} + +ExternalReferenceRegistry::ExternalReferenceRegistry() { +#define V(modname) _register_external_reference_##modname(this); + EXTERNAL_REFERENCE_BINDING_LIST(V) +#undef V + // TODO(joyeecheung): collect more external references here. +} + +} // namespace node diff --git a/src/node_external_reference.h b/src/node_external_reference.h new file mode 100644 index 00000000000000..544236e3af669c --- /dev/null +++ b/src/node_external_reference.h @@ -0,0 +1,67 @@ +#ifndef SRC_NODE_EXTERNAL_REFERENCE_H_ +#define SRC_NODE_EXTERNAL_REFERENCE_H_ + +#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#include +#include +#include "v8.h" + +namespace node { + +// This class manages the external references from the V8 heap +// to the C++ addresses in Node.js. +class ExternalReferenceRegistry { + public: + ExternalReferenceRegistry(); + +#define ALLOWED_EXTERNAL_REFERENCE_TYPES(V) \ + V(v8::FunctionCallback) \ + V(v8::AccessorGetterCallback) \ + V(v8::AccessorSetterCallback) \ + V(v8::AccessorNameGetterCallback) \ + V(v8::AccessorNameSetterCallback) \ + V(v8::GenericNamedPropertyDefinerCallback) \ + V(v8::GenericNamedPropertyDeleterCallback) \ + V(v8::GenericNamedPropertyEnumeratorCallback) \ + V(v8::GenericNamedPropertyQueryCallback) \ + V(v8::GenericNamedPropertySetterCallback) + +#define V(ExternalReferenceType) \ + void Register(ExternalReferenceType addr) { RegisterT(addr); } + ALLOWED_EXTERNAL_REFERENCE_TYPES(V) +#undef V + + // This can be called only once. + const std::vector& external_references(); + + bool is_empty() { return external_references_.empty(); } + + private: + template + void RegisterT(T* address) { + external_references_.push_back(reinterpret_cast(address)); + } + bool is_finalized_ = false; + std::vector external_references_; +}; + +#define EXTERNAL_REFERENCE_BINDING_LIST(V) + +} // namespace node + +// Declare all the external reference registration functions here, +// and define them later with #NODE_MODULE_EXTERNAL_REFERENCE(modname, func); +#define V(modname) \ + void _register_external_reference_##modname( \ + node::ExternalReferenceRegistry* registry); +EXTERNAL_REFERENCE_BINDING_LIST(V) +#undef V + +#define NODE_MODULE_EXTERNAL_REFERENCE(modname, func) \ + void _register_external_reference_##modname( \ + node::ExternalReferenceRegistry* registry) { \ + func(registry); \ + } +#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS +#endif // SRC_NODE_EXTERNAL_REFERENCE_H_ diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc index f638e26dba5a04..19f25574acbe95 100644 --- a/src/node_main_instance.cc +++ b/src/node_main_instance.cc @@ -1,7 +1,9 @@ #include -#include "node_main_instance.h" +#include "node_errors.h" +#include "node_external_reference.h" #include "node_internals.h" +#include "node_main_instance.h" #include "node_options-inl.h" #include "node_v8_platform-inl.h" #include "util-inl.h" @@ -22,6 +24,8 @@ using v8::Local; using v8::Locker; using v8::SealHandleScope; +std::unique_ptr NodeMainInstance::registry_ = + nullptr; NodeMainInstance::NodeMainInstance(Isolate* isolate, uv_loop_t* event_loop, MultiIsolatePlatform* platform, @@ -41,6 +45,15 @@ NodeMainInstance::NodeMainInstance(Isolate* isolate, SetIsolateMiscHandlers(isolate_, {}); } +const std::vector& NodeMainInstance::CollectExternalReferences() { + // Cannot be called more than once. + CHECK_NULL(registry_); + registry_.reset(new ExternalReferenceRegistry()); + + // TODO(joyeecheung): collect more external references here. + return registry_->external_references(); +} + std::unique_ptr NodeMainInstance::Create( Isolate* isolate, uv_loop_t* event_loop, diff --git a/src/node_main_instance.h b/src/node_main_instance.h index b8178c2774e795..de1f4dca2b7cc6 100644 --- a/src/node_main_instance.h +++ b/src/node_main_instance.h @@ -13,6 +13,8 @@ namespace node { +class ExternalReferenceRegistry; + // TODO(joyeecheung): align this with the Worker/WorkerThreadData class. // We may be able to create an abstract class to reuse some of the routines. class NodeMainInstance { @@ -66,6 +68,7 @@ class NodeMainInstance { // snapshot. static const std::vector* GetIsolateDataIndexes(); static v8::StartupData* GetEmbeddedSnapshotBlob(); + static const std::vector& CollectExternalReferences(); static const size_t kNodeContextIndex = 0; NodeMainInstance(const NodeMainInstance&) = delete; @@ -80,6 +83,7 @@ class NodeMainInstance { const std::vector& args, const std::vector& exec_args); + static std::unique_ptr registry_; std::vector args_; std::vector exec_args_; std::unique_ptr array_buffer_allocator_; diff --git a/tools/snapshot/snapshot_builder.cc b/tools/snapshot/snapshot_builder.cc index 8a97513ba905fc..7d292391ab334a 100644 --- a/tools/snapshot/snapshot_builder.cc +++ b/tools/snapshot/snapshot_builder.cc @@ -1,6 +1,8 @@ #include "snapshot_builder.h" #include #include +#include "env-inl.h" +#include "node_external_reference.h" #include "node_internals.h" #include "node_main_instance.h" #include "node_v8_platform-inl.h" @@ -63,10 +65,8 @@ const std::vector* NodeMainInstance::GetIsolateDataIndexes() { std::string SnapshotBuilder::Generate( const std::vector args, const std::vector exec_args) { - // TODO(joyeecheung): collect external references and set it in - // params.external_references. - std::vector external_references = { - reinterpret_cast(nullptr)}; + const std::vector& external_references = + NodeMainInstance::CollectExternalReferences(); Isolate* isolate = Isolate::Allocate(); per_process::v8_platform.Platform()->RegisterIsolate(isolate, uv_default_loop());