squash! src: retrieve binding data from the context

Co-authored-by: Anna Henningsen <[email protected]>

Author: addaleax

src/README.md

@@ -402,13 +402,13 @@ 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 object is always a `BindingDataBase`.
+That object is always a [`BaseObject`][].
 
 ```c++
 // In the HTTP parser source code file:
-class BindingData : public BindingDataBase {
+class BindingData : public BaseObject {
  public:
-  BindingData(Environment* env, Local<Object> obj) : BindingDataBase(env, obj) {}
+  BindingData(Environment* env, Local<Object> obj) : BaseObject(env, obj) {}
 
   std::vector<char> parser_buffer;
   bool parser_buffer_in_use = false;
@@ -418,7 +418,7 @@ class BindingData : public BindingDataBase {
 
 // Available for binding functions, e.g. the HTTP Parser constructor:
 static void New(const FunctionCallbackInfo<Value>& args) {
-  BindingData* binding_data = BindingDataBase::Unwrap<BindingData>(args);
+  BindingData* binding_data = Environment::GetBindingData<BindingData>(args);
   new Parser(binding_data, args.This());
 }
 
@@ -429,7 +429,7 @@ void InitializeHttpParser(Local<Object> target,
                           Local<Context> context,
                           void* priv) {
   Environment* env = Environment::GetCurrent(context);
-  Environment::BindingScope<BindingData> binding_scope(env, context, target);
+  Environment::BindingScope<BindingData> binding_scope(context, target);
   if (!binding_scope) return;
   BindingData* binding_data = binding_scope.data;
 

src/env-inl.h

@@ -289,7 +289,7 @@ inline void Environment::AssignToContext(v8::Local<v8::Context> context,
     ContextEmbedderIndex::kContextTag, Environment::kNodeContextTagPtr);
   // Used to retrieve bindings
   context->SetAlignedPointerInEmbedderData(
-      ContextEmbedderIndex::KBindingListIndex, &(this->bindings_));
+      ContextEmbedderIndex::kBindingListIndex, &(this->bindings_));
 
 #if HAVE_INSPECTOR
   inspector_agent()->ContextCreated(context, info);
@@ -322,74 +322,69 @@ inline Environment* Environment::GetCurrent(v8::Local<v8::Context> context) {
 
 inline Environment* Environment::GetCurrent(
     const v8::FunctionCallbackInfo<v8::Value>& info) {
-  return BindingDataBase::Unwrap<BindingDataBase>(info)->env();
+  return GetCurrent(info.GetIsolate()->GetCurrentContext());
 }
 
 template <typename T>
 inline Environment* Environment::GetCurrent(
     const v8::PropertyCallbackInfo<T>& info) {
-  return BindingDataBase::Unwrap<BindingDataBase>(info)->env();
+  return GetCurrent(info.GetIsolate()->GetCurrentContext());
 }
 
-inline BindingDataBase::BindingDataBase(Environment* env,
-                                        v8::Local<v8::Object> target)
-    : BaseObject(env, target) {}
-
 template <typename T, typename U>
-inline T* BindingDataBase::Unwrap(const v8::PropertyCallbackInfo<U>& info) {
-  return Unwrap<T>(info.GetIsolate()->GetCurrentContext(), info.Data());
+inline T* Environment::GetBindingData(const v8::PropertyCallbackInfo<U>& info) {
+  return GetBindingData<T>(info.GetIsolate()->GetCurrentContext(), info.Data());
 }
 
 template <typename T>
-inline T* BindingDataBase::Unwrap(
+inline T* Environment::GetBindingData(
     const v8::FunctionCallbackInfo<v8::Value>& info) {
-  return Unwrap<T>(info.GetIsolate()->GetCurrentContext(), info.Data());
+  return GetBindingData<T>(info.GetIsolate()->GetCurrentContext(), info.Data());
 }
 
 template <typename T>
-inline T* BindingDataBase::Unwrap(v8::Local<v8::Context> context,
-                                  v8::Local<v8::Value> val) {
-  CHECK(val->IsUint32());
+inline T* Environment::GetBindingData(v8::Local<v8::Context> context,
+                                      v8::Local<v8::Value> val) {
+  DCHECK(val->IsUint32());
   uint32_t index = val.As<v8::Uint32>()->Value();
-  std::vector<BindingDataBase*>* list =
-      static_cast<std::vector<BindingDataBase*>*>(
-          context->GetAlignedPointerFromEmbedderData(
-              ContextEmbedderIndex::KBindingListIndex));
-  CHECK_GT(list->size(), index);
-  T* result = static_cast<T*>(list->at(index));
+  BindingDataList* list = static_cast<BindingDataList*>(
+      context->GetAlignedPointerFromEmbedderData(
+          ContextEmbedderIndex::kBindingListIndex));
+  DCHECK_NOT_NULL(list);
+  DCHECK_GT(list->size(), index);
+  T* result = static_cast<T*>(list->at(index).get());
+  DCHECK_NOT_NULL(result);
+  DCHECK_EQ(result->env(), GetCurrent(context));
   return result;
 }
 
 template <typename T>
-inline v8::Local<v8::Uint32> BindingDataBase::New(
-    Environment* env,
+inline std::pair<T*, uint32_t> Environment::NewBindingData(
     v8::Local<v8::Context> context,
     v8::Local<v8::Object> target) {
-  T* data = new T(env, target);
-  // This won't compile if T is not a BindingDataBase subclass.
-  BindingDataBase* item = static_cast<BindingDataBase*>(data);
-  std::vector<BindingDataBase*>* list =
-      static_cast<std::vector<BindingDataBase*>*>(
-          context->GetAlignedPointerFromEmbedderData(
-              ContextEmbedderIndex::KBindingListIndex));
+  DCHECK_EQ(GetCurrent(context), this);
+  // This won't compile if T is not a BaseObject subclass.
+  BaseObjectPtr<T> item = MakeDetachedBaseObject<T>(this, target);
+  BindingDataList* list = static_cast<BindingDataList*>(
+      context->GetAlignedPointerFromEmbedderData(
+          ContextEmbedderIndex::kBindingListIndex));
+  DCHECK_NOT_NULL(list);
   size_t index = list->size();
-  list->push_back(item);
-  return v8::Integer::NewFromUnsigned(env->isolate(), index).As<v8::Uint32>();
+  list->emplace_back(item);
+  return std::make_pair(item.get(), index);
 }
 
 template <typename T>
-Environment::BindingScope<T>::BindingScope(Environment* env,
-                                           v8::Local<v8::Context> context,
+Environment::BindingScope<T>::BindingScope(v8::Local<v8::Context> context,
                                            v8::Local<v8::Object> target)
-    : env(env) {
-  v8::Local<v8::Uint32> index = BindingDataBase::New<T>(env, context, target);
-  data = BindingDataBase::Unwrap<T>(context, index);
-  env->set_current_callback_data(index);
+    : env(Environment::GetCurrent(context)) {
+  std::tie(data, env->current_callback_data_) =
+      env->NewBindingData<T>(context, target);
 }
 
 template <typename T>
 Environment::BindingScope<T>::~BindingScope() {
-  env->set_current_callback_data(env->default_callback_data());
+  env->current_callback_data_ = env->default_callback_data_;
 }
 
 inline Environment* Environment::GetThreadLocalEnv() {
@@ -1299,12 +1294,20 @@ void Environment::set_process_exit_handler(
   }
   ENVIRONMENT_STRONG_PERSISTENT_TEMPLATES(V)
   ENVIRONMENT_STRONG_PERSISTENT_VALUES(V)
-  ENVIRONMENT_CALLBACK_DATA(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_);
+}
+
+v8::Local<v8::Value> Environment::as_callback_data() const {
+  return v8::Integer::NewFromUnsigned(isolate(), default_callback_data_);
+}
+
+v8::Local<v8::Value> Environment::current_callback_data() const {
+  return v8::Integer::NewFromUnsigned(isolate(), current_callback_data_);
+}
+
 }  // namespace node
 
 // These two files depend on each other. Including base_object-inl.h after this

src/env.cc

@@ -50,7 +50,6 @@ using v8::String;
 using v8::Symbol;
 using v8::TracingController;
 using v8::TryCatch;
-using v8::Uint32;
 using v8::Undefined;
 using v8::Value;
 using worker::Worker;
@@ -262,10 +261,10 @@ void TrackingTraceStateObserver::UpdateTraceCategoryState() {
   USE(cb->Call(env_->context(), Undefined(isolate), arraysize(args), args));
 }
 
-class NoBindingData : public BindingDataBase {
+class NoBindingData : public BaseObject {
  public:
   NoBindingData(Environment* env, Local<Object> obj)
-      : BindingDataBase(env, obj) {}
+      : BaseObject(env, obj) {}
 
   SET_NO_MEMORY_INFO()
   SET_MEMORY_INFO_NAME(NoBindingData)
@@ -285,9 +284,9 @@ void Environment::CreateProperties() {
     set_binding_data_ctor_template(templ);
     Local<Function> ctor = templ->GetFunction(ctx).ToLocalChecked();
     Local<Object> obj = ctor->NewInstance(ctx).ToLocalChecked();
-    Local<Uint32> index = BindingDataBase::New<NoBindingData>(this, ctx, obj);
-    set_default_callback_data(index);
-    set_current_callback_data(index);
+    uint32_t index = NewBindingData<NoBindingData>(ctx, obj).second;
+    default_callback_data_ = index;
+    current_callback_data_ = index;
   }
 
   // Store primordials setup by the per-context script in the environment.
@@ -678,6 +677,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()) {
@@ -1137,7 +1138,6 @@ void Environment::MemoryInfo(MemoryTracker* tracker) const {
 #define V(PropertyName, TypeName)                                              \
   tracker->TrackField(#PropertyName, PropertyName());
   ENVIRONMENT_STRONG_PERSISTENT_VALUES(V)
-  ENVIRONMENT_CALLBACK_DATA(V)
 #undef V
 
   // FIXME(joyeecheung): track other fields in Environment.

src/env.h

@@ -419,10 +419,6 @@ constexpr size_t kFsStatsBufferLength =
   V(write_wrap_template, v8::ObjectTemplate)                                   \
   V(worker_heap_snapshot_taker_template, v8::ObjectTemplate)
 
-#define ENVIRONMENT_CALLBACK_DATA(V)                                           \
-  V(default_callback_data, v8::Uint32)                                         \
-  V(current_callback_data, v8::Uint32)
-
 #define ENVIRONMENT_STRONG_PERSISTENT_VALUES(V)                                \
   V(async_hooks_after_function, v8::Function)                                  \
   V(async_hooks_before_function, v8::Function)                                 \
@@ -826,28 +822,6 @@ class CleanupHookCallback {
   uint64_t insertion_order_counter_;
 };
 
-class BindingDataBase : public BaseObject {
- public:
-  // Exposed for subclasses
-  inline BindingDataBase(Environment* env, v8::Local<v8::Object> target);
-  // Unwrap a subclass T object from a v8::Value which needs to be an
-  // v8::Uint32
-  template <typename T, typename U>
-  static inline T* Unwrap(const v8::PropertyCallbackInfo<U>& info);
-  template <typename T>
-  static inline T* Unwrap(const v8::FunctionCallbackInfo<v8::Value>& info);
-
-  template <typename T>
-  static inline T* Unwrap(v8::Local<v8::Context> context,
-                          v8::Local<v8::Value> val);
-  // Create a BindingData of subclass T, put it into the context binding list,
-  // return the index as v8::Integer
-  template <typename T>
-  static inline v8::Local<v8::Uint32> New(Environment* env,
-                                          v8::Local<v8::Context> context,
-                                          v8::Local<v8::Object> target);
-};
-
 class Environment : public MemoryRetainer {
  public:
   Environment(const Environment&) = delete;
@@ -890,11 +864,10 @@ class Environment : public MemoryRetainer {
 
   // Methods created using SetMethod(), SetPrototypeMethod(), etc. inside
   // this scope can access the created T* object using
-  // Unwrap<T>(args.Data()) later.
+  // GetBindingData<T>(args.Data()) later.
   template <typename T>
   struct BindingScope {
-    explicit inline BindingScope(Environment* env,
-                                 v8::Local<v8::Context> context,
+    explicit inline BindingScope(v8::Local<v8::Context> context,
                                  v8::Local<v8::Object> target);
     inline ~BindingScope();
 
@@ -905,6 +878,25 @@ class Environment : public MemoryRetainer {
     inline bool operator !() const { return data == nullptr; }
   };
 
+  template <typename T, typename U>
+  static inline T* GetBindingData(const v8::PropertyCallbackInfo<U>& info);
+  template <typename T>
+  static inline T* GetBindingData(
+      const v8::FunctionCallbackInfo<v8::Value>& info);
+
+  // Unwrap a subclass T object from a v8::Value which needs to be an
+  // v8::Uint32
+  template <typename T>
+  static inline T* GetBindingData(v8::Local<v8::Context> context,
+                                  v8::Local<v8::Value> val);
+  // Create a BindingData of subclass T, put it into the context binding list,
+  // return the index as an integer
+  template <typename T>
+  inline std::pair<T*, uint32_t> NewBindingData(v8::Local<v8::Context> context,
+                                                v8::Local<v8::Object> target);
+
+  typedef std::vector<BaseObjectPtr<BaseObject>> BindingDataList;
+
   static uv_key_t thread_local_env;
   static inline Environment* GetThreadLocalEnv();
 
@@ -1149,12 +1141,13 @@ class Environment : public MemoryRetainer {
 #define V(PropertyName, TypeName)                                             \
   inline v8::Local<TypeName> PropertyName() const;                            \
   inline void set_ ## PropertyName(v8::Local<TypeName> value);
-  ENVIRONMENT_CALLBACK_DATA(V)
   ENVIRONMENT_STRONG_PERSISTENT_VALUES(V)
   ENVIRONMENT_STRONG_PERSISTENT_TEMPLATES(V)
 #undef V
 
   inline v8::Local<v8::Context> context() const;
+  inline v8::Local<v8::Value> as_callback_data() const;
+  inline v8::Local<v8::Value> current_callback_data() const;
 
 #if HAVE_INSPECTOR
   inline inspector::Agent* inspector_agent() const {
@@ -1450,7 +1443,7 @@ class Environment : public MemoryRetainer {
   void RequestInterruptFromV8();
   static void CheckImmediate(uv_check_t* handle);
 
-  std::vector<BindingDataBase*> bindings_;
+  BindingDataList bindings_;
 
   // Use an unordered_set, so that we have efficient insertion and removal.
   std::unordered_set<CleanupHookCallback,
@@ -1470,13 +1463,15 @@ class Environment : public MemoryRetainer {
   void ForEachBaseObject(T&& iterator);
 
 #define V(PropertyName, TypeName) v8::Global<TypeName> PropertyName ## _;
-  ENVIRONMENT_CALLBACK_DATA(V)
   ENVIRONMENT_STRONG_PERSISTENT_VALUES(V)
   ENVIRONMENT_STRONG_PERSISTENT_TEMPLATES(V)
 #undef V
 
   v8::Global<v8::Context> context_;
 
+  uint32_t default_callback_data_;
+  uint32_t current_callback_data_;
+
   // Keeps the main script source alive is one was passed to LoadEnvironment().
   // We should probably find a way to just use plain `v8::String`s created from
   // the source passed to LoadEnvironment() directly instead.

src/node_context_data.h

@@ -34,7 +34,7 @@ enum ContextEmbedderIndex {
   kSandboxObject = NODE_CONTEXT_SANDBOX_OBJECT_INDEX,
   kAllowWasmCodeGeneration = NODE_CONTEXT_ALLOW_WASM_CODE_GENERATION_INDEX,
   kContextTag = NODE_CONTEXT_TAG,
-  KBindingListIndex = NODE_BINDING_LIST_INDEX
+  kBindingListIndex = NODE_BINDING_LIST_INDEX
 };
 
 }  // namespace node

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 = BindingDataBase::Unwrap<BindingData>(args);
+  BindingData* binding_data = Environment::GetBindingData<BindingData>(args);
   Environment* env = binding_data->env();
   if (value->StrictEquals(env->fs_use_promises_symbol())) {
     if (use_bigint) {