src: retrieve binding data from the context

Instead of passing them through the data bound to function
templates, store references to them in a list embedded inside
the context.
This makes the function templates more context-independent,
and makes it possible to embed binding data in non-main contexts.

Co-authored-by: Anna Henningsen <anna@addaleax.net>

PR-URL: nodejs#33139
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

src/README.md

@@ -400,16 +400,23 @@ NODE_MODULE_CONTEXT_AWARE_INTERNAL(cares_wrap, Initialize)
 
 Some internal bindings, such as the HTTP parser, maintain internal state that
 only affects that particular binding. In that case, one common way to store
-that state is through the use of `Environment::BindingScope`, which gives all
-new functions created within it access to an object for storing such state.
+that state is through the use of `Environment::AddBindingData`, which gives
+binding functions access to an object for storing such state.
 That object is always a [`BaseObject`][].
 
+Its class needs to have a static `binding_data_name` field that based on a
+constant string, in order to disambiguate it from other classes of this type,
+and which could e.g. match the binding’s name (in the example above, that would
+be `cares_wrap`).
+
 ```c++
 // In the HTTP parser source code file:
 class BindingData : public BaseObject {
  public:
   BindingData(Environment* env, Local<Object> obj) : BaseObject(env, obj) {}
 
+  static constexpr FastStringKey binding_data_name { "http_parser" };
+
   std::vector<char> parser_buffer;
   bool parser_buffer_in_use = false;
 
@@ -418,22 +425,21 @@ class BindingData : public BaseObject {
 
 // Available for binding functions, e.g. the HTTP Parser constructor:
 static void New(const FunctionCallbackInfo<Value>& args) {
-  BindingData* binding_data = Unwrap<BindingData>(args.Data());
+  BindingData* binding_data = Environment::GetBindingData<BindingData>(args);
   new Parser(binding_data, args.This());
 }
 
-// ... because the initialization function told the Environment to use this
-// BindingData class for all functions created by it:
+// ... because the initialization function told the Environment to store the
+// BindingData object:
 void InitializeHttpParser(Local<Object> target,
                           Local<Value> unused,
                           Local<Context> context,
                           void* priv) {
   Environment* env = Environment::GetCurrent(context);
-  Environment::BindingScope<BindingData> binding_scope(env);
-  if (!binding_scope) return;
-  BindingData* binding_data = binding_scope.data;
+  BindingData* const binding_data =
+      env->AddBindingData<BindingData>(context, target);
+  if (binding_data == nullptr) return;
 
-  // Created within the Environment::BindingScope
   Local<FunctionTemplate> t = env->NewFunctionTemplate(Parser::New);
   ...
 }

src/env-inl.h

@@ -287,6 +287,10 @@ inline void Environment::AssignToContext(v8::Local<v8::Context> context,
   // Used by Environment::GetCurrent to know that we are on a node context.
   context->SetAlignedPointerInEmbedderData(
     ContextEmbedderIndex::kContextTag, Environment::kNodeContextTagPtr);
+  // Used to retrieve bindings
+  context->SetAlignedPointerInEmbedderData(
+      ContextEmbedderIndex::kBindingListIndex, &(this->bindings_));
+
 #if HAVE_INSPECTOR
   inspector_agent()->ContextCreated(context, info);
 #endif  // HAVE_INSPECTOR
@@ -318,55 +322,55 @@ inline Environment* Environment::GetCurrent(v8::Local<v8::Context> context) {
 
 inline Environment* Environment::GetCurrent(
     const v8::FunctionCallbackInfo<v8::Value>& info) {
-  return GetFromCallbackData(info.Data());
+  return GetCurrent(info.GetIsolate()->GetCurrentContext());
 }
 
 template <typename T>
 inline Environment* Environment::GetCurrent(
     const v8::PropertyCallbackInfo<T>& info) {
-  return GetFromCallbackData(info.Data());
+  return GetCurrent(info.GetIsolate()->GetCurrentContext());
 }
 
-Environment* Environment::GetFromCallbackData(v8::Local<v8::Value> val) {
-  DCHECK(val->IsObject());
-  v8::Local<v8::Object> obj = val.As<v8::Object>();
-  DCHECK_GE(obj->InternalFieldCount(),
-            BaseObject::kInternalFieldCount);
-  Environment* env = Unwrap<BaseObject>(obj)->env();
-  DCHECK(env->as_callback_data_template()->HasInstance(obj));
-  return env;
+template <typename T, typename U>
+inline T* Environment::GetBindingData(const v8::PropertyCallbackInfo<U>& info) {
+  return GetBindingData<T>(info.GetIsolate()->GetCurrentContext());
 }
 
 template <typename T>
-Environment::BindingScope<T>::BindingScope(Environment* env) : env(env) {
-  v8::Local<v8::Object> callback_data;
-  if (!env->MakeBindingCallbackData<T>().ToLocal(&callback_data))
-    return;
-  data = Unwrap<T>(callback_data);
-
-  // No nesting allowed currently.
-  CHECK_EQ(env->current_callback_data(), env->as_callback_data());
-  env->set_current_callback_data(callback_data);
+inline T* Environment::GetBindingData(
+    const v8::FunctionCallbackInfo<v8::Value>& info) {
+  return GetBindingData<T>(info.GetIsolate()->GetCurrentContext());
 }
 
 template <typename T>
-Environment::BindingScope<T>::~BindingScope() {
-  env->set_current_callback_data(env->as_callback_data());
+inline T* Environment::GetBindingData(v8::Local<v8::Context> context) {
+  BindingDataStore* map = static_cast<BindingDataStore*>(
+      context->GetAlignedPointerFromEmbedderData(
+          ContextEmbedderIndex::kBindingListIndex));
+  DCHECK_NOT_NULL(map);
+  auto it = map->find(T::binding_data_name);
+  if (UNLIKELY(it == map->end())) return nullptr;
+  T* result = static_cast<T*>(it->second.get());
+  DCHECK_NOT_NULL(result);
+  DCHECK_EQ(result->env(), GetCurrent(context));
+  return result;
 }
 
 template <typename T>
-v8::MaybeLocal<v8::Object> Environment::MakeBindingCallbackData() {
-  v8::Local<v8::Function> ctor;
-  v8::Local<v8::Object> obj;
-  if (!as_callback_data_template()->GetFunction(context()).ToLocal(&ctor) ||
-      !ctor->NewInstance(context()).ToLocal(&obj)) {
-    return v8::MaybeLocal<v8::Object>();
-  }
-  T* data = new T(this, obj);
+inline T* Environment::AddBindingData(
+    v8::Local<v8::Context> context,
+    v8::Local<v8::Object> target) {
+  DCHECK_EQ(GetCurrent(context), this);
   // This won't compile if T is not a BaseObject subclass.
-  CHECK_EQ(data, static_cast<BaseObject*>(data));
-  data->MakeWeak();
-  return obj;
+  BaseObjectPtr<T> item = MakeDetachedBaseObject<T>(this, target);
+  BindingDataStore* map = static_cast<BindingDataStore*>(
+      context->GetAlignedPointerFromEmbedderData(
+          ContextEmbedderIndex::kBindingListIndex));
+  DCHECK_NOT_NULL(map);
+  auto result = map->emplace(T::binding_data_name, item);
+  CHECK(result.second);
+  DCHECK_EQ(GetBindingData<T>(context), item.get());
+  return item.get();
 }
 
 inline Environment* Environment::GetThreadLocalEnv() {
@@ -1085,8 +1089,7 @@ inline v8::Local<v8::FunctionTemplate>
                                      v8::Local<v8::Signature> signature,
                                      v8::ConstructorBehavior behavior,
                                      v8::SideEffectType side_effect_type) {
-  v8::Local<v8::Object> external = current_callback_data();
-  return v8::FunctionTemplate::New(isolate(), callback, external,
+  return v8::FunctionTemplate::New(isolate(), callback, v8::Local<v8::Value>(),
                                    signature, 0, behavior, side_effect_type);
 }
 
@@ -1278,9 +1281,10 @@ void Environment::set_process_exit_handler(
   ENVIRONMENT_STRONG_PERSISTENT_VALUES(V)
 #undef V
 
-  inline v8::Local<v8::Context> Environment::context() const {
-    return PersistentToLocal::Strong(context_);
-  }
+v8::Local<v8::Context> Environment::context() const {
+  return PersistentToLocal::Strong(context_);
+}
+
 }  // namespace node
 
 // These two files depend on each other. Including base_object-inl.h after this

src/env.cc

@@ -261,29 +261,17 @@ void TrackingTraceStateObserver::UpdateTraceCategoryState() {
   USE(cb->Call(env_->context(), Undefined(isolate), arraysize(args), args));
 }
 
-class NoBindingData : public BaseObject {
- public:
-  NoBindingData(Environment* env, Local<Object> obj) : BaseObject(env, obj) {}
-
-  SET_NO_MEMORY_INFO()
-  SET_MEMORY_INFO_NAME(NoBindingData)
-  SET_SELF_SIZE(NoBindingData)
-};
-
 void Environment::CreateProperties() {
   HandleScope handle_scope(isolate_);
   Local<Context> ctx = context();
+
   {
     Context::Scope context_scope(ctx);
     Local<FunctionTemplate> templ = FunctionTemplate::New(isolate());
     templ->InstanceTemplate()->SetInternalFieldCount(
         BaseObject::kInternalFieldCount);
-    set_as_callback_data_template(templ);
 
-    Local<Object> obj = MakeBindingCallbackData<NoBindingData>()
-        .ToLocalChecked();
-    set_as_callback_data(obj);
-    set_current_callback_data(obj);
+    set_binding_data_ctor_template(templ);
   }
 
   // Store primordials setup by the per-context script in the environment.
@@ -674,6 +662,8 @@ void Environment::RunCleanup() {
   started_cleanup_ = true;
   TraceEventScope trace_scope(TRACING_CATEGORY_NODE1(environment),
                               "RunCleanup", this);
+  bindings_.clear();
+  initial_base_object_count_ = 0;
   CleanupHandles();
 
   while (!cleanup_hooks_.empty()) {

src/env.h

@@ -390,9 +390,9 @@ constexpr size_t kFsStatsBufferLength =
   V(zero_return_string, "ZERO_RETURN")
 
 #define ENVIRONMENT_STRONG_PERSISTENT_TEMPLATES(V)                             \
-  V(as_callback_data_template, v8::FunctionTemplate)                           \
   V(async_wrap_ctor_template, v8::FunctionTemplate)                            \
   V(async_wrap_object_ctor_template, v8::FunctionTemplate)                     \
+  V(binding_data_ctor_template, v8::FunctionTemplate)                          \
   V(compiled_fn_entry_template, v8::ObjectTemplate)                            \
   V(dir_instance_template, v8::ObjectTemplate)                                 \
   V(fd_constructor_template, v8::ObjectTemplate)                               \
@@ -420,7 +420,6 @@ constexpr size_t kFsStatsBufferLength =
   V(worker_heap_snapshot_taker_template, v8::ObjectTemplate)
 
 #define ENVIRONMENT_STRONG_PERSISTENT_VALUES(V)                                \
-  V(as_callback_data, v8::Object)                                              \
   V(async_hooks_after_function, v8::Function)                                  \
   V(async_hooks_before_function, v8::Function)                                 \
   V(async_hooks_binding, v8::Object)                                           \
@@ -429,7 +428,6 @@ constexpr size_t kFsStatsBufferLength =
   V(async_hooks_promise_resolve_function, v8::Function)                        \
   V(buffer_prototype_object, v8::Object)                                       \
   V(crypto_key_object_constructor, v8::Function)                               \
-  V(current_callback_data, v8::Object)                                         \
   V(domain_callback, v8::Function)                                             \
   V(domexception_function, v8::Function)                                       \
   V(enhance_fatal_stack_after_inspector, v8::Function)                         \
@@ -864,25 +862,24 @@ class Environment : public MemoryRetainer {
   static inline Environment* GetCurrent(
       const v8::PropertyCallbackInfo<T>& info);
 
-  static inline Environment* GetFromCallbackData(v8::Local<v8::Value> val);
-
   // Methods created using SetMethod(), SetPrototypeMethod(), etc. inside
   // this scope can access the created T* object using
-  // Unwrap<T>(args.Data()) later.
+  // GetBindingData<T>(args) later.
   template <typename T>
-  struct BindingScope {
-    explicit inline BindingScope(Environment* env);
-    inline ~BindingScope();
-
-    T* data = nullptr;
-    Environment* env;
-
-    inline operator bool() const { return data != nullptr; }
-    inline bool operator !() const { return data == nullptr; }
-  };
-
+  T* AddBindingData(v8::Local<v8::Context> context,
+                    v8::Local<v8::Object> target);
+  template <typename T, typename U>
+  static inline T* GetBindingData(const v8::PropertyCallbackInfo<U>& info);
   template <typename T>
-  inline v8::MaybeLocal<v8::Object> MakeBindingCallbackData();
+  static inline T* GetBindingData(
+      const v8::FunctionCallbackInfo<v8::Value>& info);
+  template <typename T>
+  static inline T* GetBindingData(v8::Local<v8::Context> context);
+
+  typedef std::unordered_map<
+      FastStringKey,
+      BaseObjectPtr<BaseObject>,
+      FastStringKey::Hash> BindingDataStore;
 
   static uv_key_t thread_local_env;
   static inline Environment* GetThreadLocalEnv();
@@ -1428,6 +1425,8 @@ class Environment : public MemoryRetainer {
   void RequestInterruptFromV8();
   static void CheckImmediate(uv_check_t* handle);
 
+  BindingDataStore bindings_;
+
   // Use an unordered_set, so that we have efficient insertion and removal.
   std::unordered_set<CleanupHookCallback,
                      CleanupHookCallback::Hash,

src/fs_event_wrap.cc

@@ -108,7 +108,7 @@ void FSEventWrap::Initialize(Local<Object> target,
   Local<FunctionTemplate> get_initialized_templ =
       FunctionTemplate::New(env->isolate(),
                             GetInitialized,
-                            env->current_callback_data(),
+                            Local<Value>(),
                             Signature::New(env->isolate(), t));
 
   t->PrototypeTemplate()->SetAccessorProperty(

src/node.cc

@@ -331,8 +331,7 @@ MaybeLocal<Value> Environment::BootstrapNode() {
 
   Local<String> env_string = FIXED_ONE_BYTE_STRING(isolate_, "env");
   Local<Object> env_var_proxy;
-  if (!CreateEnvVarProxy(context(), isolate_, current_callback_data())
-           .ToLocal(&env_var_proxy) ||
+  if (!CreateEnvVarProxy(context(), isolate_).ToLocal(&env_var_proxy) ||
       process_object()->Set(context(), env_string, env_var_proxy).IsNothing()) {
     return MaybeLocal<Value>();
   }

src/node_binding.cc

@@ -230,6 +230,7 @@ namespace node {
 
 using v8::Context;
 using v8::Exception;
+using v8::Function;
 using v8::FunctionCallbackInfo;
 using v8::Local;
 using v8::NewStringType;
@@ -556,8 +557,11 @@ inline struct node_module* FindModule(struct node_module* list,
 static Local<Object> InitModule(Environment* env,
                                 node_module* mod,
                                 Local<String> module) {
-  Local<Object> exports = Object::New(env->isolate());
   // Internal bindings don't have a "module" object, only exports.
+  Local<Function> ctor = env->binding_data_ctor_template()
+                             ->GetFunction(env->context())
+                             .ToLocalChecked();
+  Local<Object> exports = ctor->NewInstance(env->context()).ToLocalChecked();
   CHECK_NULL(mod->nm_register_func);
   CHECK_NOT_NULL(mod->nm_context_register_func);
   Local<Value> unused = Undefined(env->isolate());

src/node_context_data.h

@@ -25,11 +25,16 @@ namespace node {
 #define NODE_CONTEXT_TAG 35
 #endif
 
+#ifndef NODE_BINDING_LIST
+#define NODE_BINDING_LIST_INDEX 36
+#endif
+
 enum ContextEmbedderIndex {
   kEnvironment = NODE_CONTEXT_EMBEDDER_DATA_INDEX,
   kSandboxObject = NODE_CONTEXT_SANDBOX_OBJECT_INDEX,
   kAllowWasmCodeGeneration = NODE_CONTEXT_ALLOW_WASM_CODE_GENERATION_INDEX,
   kContextTag = NODE_CONTEXT_TAG,
+  kBindingListIndex = NODE_BINDING_LIST_INDEX
 };
 
 }  // namespace node

src/node_crypto.cc

@@ -505,7 +505,7 @@ void SecureContext::Initialize(Environment* env, Local<Object> target) {
   Local<FunctionTemplate> ctx_getter_templ =
       FunctionTemplate::New(env->isolate(),
                             CtxGetter,
-                            env->current_callback_data(),
+                            Local<Value>(),
                             Signature::New(env->isolate(), t));
 
 
@@ -5103,7 +5103,7 @@ void DiffieHellman::Initialize(Environment* env, Local<Object> target) {
     Local<FunctionTemplate> verify_error_getter_templ =
         FunctionTemplate::New(env->isolate(),
                               DiffieHellman::VerifyErrorGetter,
-                              env->current_callback_data(),
+                              Local<Value>(),
                               Signature::New(env->isolate(), t),
                               /* length */ 0,
                               ConstructorBehavior::kThrow,

src/node_env_var.cc

@@ -377,13 +377,11 @@ static void EnvEnumerator(const PropertyCallbackInfo<Array>& info) {
       env->env_vars()->Enumerate(env->isolate()));
 }
 
-MaybeLocal<Object> CreateEnvVarProxy(Local<Context> context,
-                                     Isolate* isolate,
-                                     Local<Object> data) {
+MaybeLocal<Object> CreateEnvVarProxy(Local<Context> context, Isolate* isolate) {
   EscapableHandleScope scope(isolate);
   Local<ObjectTemplate> env_proxy_template = ObjectTemplate::New(isolate);
   env_proxy_template->SetHandler(NamedPropertyHandlerConfiguration(
-      EnvGetter, EnvSetter, EnvQuery, EnvDeleter, EnvEnumerator, data,
+      EnvGetter, EnvSetter, EnvQuery, EnvDeleter, EnvEnumerator, Local<Value>(),
       PropertyHandlerFlags::kHasNoSideEffect));
   return scope.EscapeMaybe(env_proxy_template->NewInstance(context));
 }

src/node_file-inl.h

@@ -227,7 +227,7 @@ FSReqBase* GetReqWrap(const v8::FunctionCallbackInfo<v8::Value>& args,
     return Unwrap<FSReqBase>(value.As<v8::Object>());
   }
 
-  BindingData* binding_data = Unwrap<BindingData>(args.Data());
+  BindingData* binding_data = Environment::GetBindingData<BindingData>(args);
   Environment* env = binding_data->env();
   if (value->StrictEquals(env->fs_use_promises_symbol())) {
     if (use_bigint) {