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: #32984 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
nodejs · Jul 18, 2020 · ef9964f · ef9964f
1 parent 404302f
commit ef9964f
Show file tree

Hide file tree

Showing 7 changed files with 114 additions and 5 deletions.
diff --git 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
@@ -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<intptr_t>(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
@@ -0,0 +1,22 @@
+#include "node_external_reference.h"
+#include <cinttypes>
+#include <vector>
+#include "util.h"
+
+namespace node {
+
+const std::vector<intptr_t>& ExternalReferenceRegistry::external_references() {
+  CHECK(!is_finalized_);
+  external_references_.push_back(reinterpret_cast<intptr_t>(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
@@ -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 <cinttypes>
+#include <vector>
+#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<intptr_t>& external_references();
+
+  bool is_empty() { return external_references_.empty(); }
+
+ private:
+  template <typename T>
+  void RegisterT(T* address) {
+    external_references_.push_back(reinterpret_cast<intptr_t>(address));
+  }
+  bool is_finalized_ = false;
+  std::vector<intptr_t> 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
@@ -1,7 +1,9 @@
 #include <memory>
 
-#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<ExternalReferenceRegistry> 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<intptr_t>& 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> NodeMainInstance::Create(
     Isolate* isolate,
     uv_loop_t* event_loop,

diff --git 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<size_t>* GetIsolateDataIndexes();
   static v8::StartupData* GetEmbeddedSnapshotBlob();
+  static const std::vector<intptr_t>& CollectExternalReferences();
 
   static const size_t kNodeContextIndex = 0;
   NodeMainInstance(const NodeMainInstance&) = delete;
@@ -80,6 +83,7 @@ class NodeMainInstance {
                    const std::vector<std::string>& args,
                    const std::vector<std::string>& exec_args);
 
+  static std::unique_ptr<ExternalReferenceRegistry> registry_;
   std::vector<std::string> args_;
   std::vector<std::string> exec_args_;
   std::unique_ptr<ArrayBufferAllocator> array_buffer_allocator_;

diff --git a/tools/snapshot/snapshot_builder.cc b/tools/snapshot/snapshot_builder.cc
@@ -1,6 +1,8 @@
 #include "snapshot_builder.h"
 #include <iostream>
 #include <sstream>
+#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<size_t>* NodeMainInstance::GetIsolateDataIndexes() {
 std::string SnapshotBuilder::Generate(
     const std::vector<std::string> args,
     const std::vector<std::string> exec_args) {
-  // TODO(joyeecheung): collect external references and set it in
-  // params.external_references.
-  std::vector<intptr_t> external_references = {
-      reinterpret_cast<intptr_t>(nullptr)};
+  const std::vector<intptr_t>& external_references =
+      NodeMainInstance::CollectExternalReferences();
   Isolate* isolate = Isolate::Allocate();
   per_process::v8_platform.Platform()->RegisterIsolate(isolate,
                                                        uv_default_loop());