node-api: remove RefBase and CallbackWrapper

PR-URL: #53590
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>

Author: vmoroz

src/js_native_api_v8.cc

@@ -134,43 +134,38 @@ napi_status NewExternalString(napi_env env,
   return status;
 }
 
-class TrackedStringResource : public Finalizer, RefTracker {
+class TrackedStringResource : private RefTracker {
  public:
   TrackedStringResource(napi_env env,
                         napi_finalize finalize_callback,
                         void* data,
                         void* finalize_hint)
-      : Finalizer(env, finalize_callback, data, finalize_hint) {
+      : RefTracker(), finalizer_(env, finalize_callback, data, finalize_hint) {
     Link(finalize_callback == nullptr ? &env->reflist
                                       : &env->finalizing_reflist);
   }
 
  protected:
-  // The only time Finalize() gets called before Dispose() is if the
+  // The only time Finalize() gets called before destructor is if the
   // environment is dying. Finalize() expects that the item will be unlinked,
-  // so we do it here. V8 will still call Dispose() on us later, so we don't do
-  // any deleting here. We just null out env_ to avoid passing a stale pointer
-  // to the user's finalizer when V8 does finally call Dispose().
+  // so we do it here. V8 will still call destructor on us later, so we don't do
+  // any deleting here. We just null out env to avoid passing a stale pointer
+  // to the user's finalizer when V8 does finally call destructor.
   void Finalize() override {
     Unlink();
-    env_ = nullptr;
+    finalizer_.ResetEnv();
   }
 
-  ~TrackedStringResource() {
-    if (finalize_callback_ == nullptr) return;
-    if (env_ == nullptr) {
-      // The environment is dead. Call the finalizer directly.
-      finalize_callback_(nullptr, finalize_data_, finalize_hint_);
-    } else {
-      // The environment is still alive. Let's remove ourselves from its list
-      // of references and call the user's finalizer.
-      Unlink();
-      env_->CallFinalizer(finalize_callback_, finalize_data_, finalize_hint_);
-    }
+  ~TrackedStringResource() override {
+    Unlink();
+    finalizer_.CallFinalizer();
   }
+
+ private:
+  Finalizer finalizer_;
 };
 
-class ExternalOneByteStringResource
+class ExternalOneByteStringResource final
     : public v8::String::ExternalOneByteStringResource,
       TrackedStringResource {
  public:
@@ -191,8 +186,8 @@ class ExternalOneByteStringResource
   const size_t length_;
 };
 
-class ExternalStringResource : public v8::String::ExternalStringResource,
-                               TrackedStringResource {
+class ExternalStringResource final : public v8::String::ExternalStringResource,
+                                     TrackedStringResource {
  public:
   ExternalStringResource(napi_env env,
                          char16_t* string,
@@ -368,7 +363,7 @@ inline napi_status Unwrap(napi_env env,
   if (action == RemoveWrap) {
     CHECK(obj->DeletePrivate(context, NAPI_PRIVATE_KEY(context, wrapper))
               .FromJust());
-    if (reference->ownership() == Ownership::kUserland) {
+    if (reference->ownership() == ReferenceOwnership::kUserland) {
       // When the wrap is been removed, the finalizer should be reset.
       reference->ResetFinalizer();
     } else {
@@ -400,10 +395,16 @@ class CallbackBundle {
     bundle->env = env;
 
     v8::Local<v8::Value> cbdata = v8::External::New(env->isolate, bundle);
-    Reference::New(
-        env, cbdata, 0, Ownership::kRuntime, Delete, bundle, nullptr);
+    ReferenceWithFinalizer::New(
+        env, cbdata, 0, ReferenceOwnership::kRuntime, Delete, bundle, nullptr);
     return cbdata;
   }
+
+  static CallbackBundle* FromCallbackData(v8::Local<v8::Value> data) {
+    return reinterpret_cast<CallbackBundle*>(data.As<v8::External>()->Value());
+  }
+
+ public:
   napi_env env;   // Necessary to invoke C++ NAPI callback
   void* cb_data;  // The user provided callback data
   napi_callback cb;
@@ -415,71 +416,9 @@ class CallbackBundle {
   }
 };
 
-// Base class extended by classes that wrap V8 function and property callback
-// info.
-class CallbackWrapper {
- public:
-  inline CallbackWrapper(napi_value this_arg, size_t args_length, void* data)
-      : _this(this_arg), _args_length(args_length), _data(data) {}
-
-  virtual napi_value GetNewTarget() = 0;
-  virtual void Args(napi_value* buffer, size_t bufferlength) = 0;
-  virtual void SetReturnValue(napi_value value) = 0;
-
-  napi_value This() { return _this; }
-
-  size_t ArgsLength() { return _args_length; }
-
-  void* Data() { return _data; }
-
- protected:
-  const napi_value _this;
-  const size_t _args_length;
-  void* _data;
-};
-
-class CallbackWrapperBase : public CallbackWrapper {
- public:
-  inline CallbackWrapperBase(const v8::FunctionCallbackInfo<v8::Value>& cbinfo,
-                             const size_t args_length)
-      : CallbackWrapper(
-            JsValueFromV8LocalValue(cbinfo.This()), args_length, nullptr),
-        _cbinfo(cbinfo) {
-    _bundle = reinterpret_cast<CallbackBundle*>(
-        cbinfo.Data().As<v8::External>()->Value());
-    _data = _bundle->cb_data;
-  }
-
- protected:
-  inline void InvokeCallback() {
-    napi_callback_info cbinfo_wrapper = reinterpret_cast<napi_callback_info>(
-        static_cast<CallbackWrapper*>(this));
-
-    // All other pointers we need are stored in `_bundle`
-    napi_env env = _bundle->env;
-    napi_callback cb = _bundle->cb;
-
-    napi_value result = nullptr;
-    bool exceptionOccurred = false;
-    env->CallIntoModule([&](napi_env env) { result = cb(env, cbinfo_wrapper); },
-                        [&](napi_env env, v8::Local<v8::Value> value) {
-                          exceptionOccurred = true;
-                          if (env->terminatedOrTerminating()) {
-                            return;
-                          }
-                          env->isolate->ThrowException(value);
-                        });
-
-    if (!exceptionOccurred && (result != nullptr)) {
-      this->SetReturnValue(result);
-    }
-  }
-
-  const v8::FunctionCallbackInfo<v8::Value>& _cbinfo;
-  CallbackBundle* _bundle;
-};
-
-class FunctionCallbackWrapper : public CallbackWrapperBase {
+// Wraps up v8::FunctionCallbackInfo.
+// The class must be stack allocated.
+class FunctionCallbackWrapper {
  public:
   static void Invoke(const v8::FunctionCallbackInfo<v8::Value>& info) {
     FunctionCallbackWrapper cbwrapper(info);
@@ -514,41 +453,70 @@ class FunctionCallbackWrapper : public CallbackWrapperBase {
     return napi_clear_last_error(env);
   }
 
-  explicit FunctionCallbackWrapper(
-      const v8::FunctionCallbackInfo<v8::Value>& cbinfo)
-      : CallbackWrapperBase(cbinfo, cbinfo.Length()) {}
-
-  napi_value GetNewTarget() override {
-    if (_cbinfo.IsConstructCall()) {
-      return v8impl::JsValueFromV8LocalValue(_cbinfo.NewTarget());
+  napi_value GetNewTarget() {
+    if (cbinfo_.IsConstructCall()) {
+      return v8impl::JsValueFromV8LocalValue(cbinfo_.NewTarget());
     } else {
       return nullptr;
     }
   }
 
-  /*virtual*/
-  void Args(napi_value* buffer, size_t buffer_length) override {
+  void Args(napi_value* buffer, size_t buffer_length) {
     size_t i = 0;
-    size_t min = std::min(buffer_length, _args_length);
+    size_t min_arg_count = std::min(buffer_length, ArgsLength());
 
-    for (; i < min; i += 1) {
-      buffer[i] = v8impl::JsValueFromV8LocalValue(_cbinfo[i]);
+    for (; i < min_arg_count; ++i) {
+      buffer[i] = JsValueFromV8LocalValue(cbinfo_[i]);
     }
 
     if (i < buffer_length) {
       napi_value undefined =
-          v8impl::JsValueFromV8LocalValue(v8::Undefined(_cbinfo.GetIsolate()));
-      for (; i < buffer_length; i += 1) {
+          JsValueFromV8LocalValue(v8::Undefined(cbinfo_.GetIsolate()));
+      for (; i < buffer_length; ++i) {
         buffer[i] = undefined;
       }
     }
   }
 
-  /*virtual*/
-  void SetReturnValue(napi_value value) override {
-    v8::Local<v8::Value> val = v8impl::V8LocalValueFromJsValue(value);
-    _cbinfo.GetReturnValue().Set(val);
+  napi_value This() { return JsValueFromV8LocalValue(cbinfo_.This()); }
+
+  size_t ArgsLength() { return static_cast<size_t>(cbinfo_.Length()); }
+
+  void* Data() { return bundle_->cb_data; }
+
+ private:
+  explicit FunctionCallbackWrapper(
+      const v8::FunctionCallbackInfo<v8::Value>& cbinfo)
+      : cbinfo_(cbinfo),
+        bundle_(CallbackBundle::FromCallbackData(cbinfo.Data())) {}
+
+  void InvokeCallback() {
+    napi_callback_info cbinfo_wrapper =
+        reinterpret_cast<napi_callback_info>(this);
+
+    // All other pointers we need are stored in `_bundle`
+    napi_env env = bundle_->env;
+    napi_callback cb = bundle_->cb;
+
+    napi_value result = nullptr;
+    bool exceptionOccurred = false;
+    env->CallIntoModule([&](napi_env env) { result = cb(env, cbinfo_wrapper); },
+                        [&](napi_env env, v8::Local<v8::Value> value) {
+                          exceptionOccurred = true;
+                          if (env->terminatedOrTerminating()) {
+                            return;
+                          }
+                          env->isolate->ThrowException(value);
+                        });
+
+    if (!exceptionOccurred && (result != nullptr)) {
+      cbinfo_.GetReturnValue().Set(V8LocalValueFromJsValue(result));
+    }
   }
+
+ private:
+  const v8::FunctionCallbackInfo<v8::Value>& cbinfo_;
+  CallbackBundle* bundle_;
 };
 
 inline napi_status Wrap(napi_env env,
@@ -579,24 +547,29 @@ inline napi_status Wrap(napi_env env,
     // before then, then the finalize callback will never be invoked.)
     // Therefore a finalize callback is required when returning a reference.
     CHECK_ARG(env, finalize_cb);
-    reference = v8impl::Reference::New(env,
-                                       obj,
-                                       0,
-                                       v8impl::Ownership::kUserland,
-                                       finalize_cb,
-                                       native_object,
-                                       finalize_hint);
+    reference = v8impl::ReferenceWithFinalizer::New(
+        env,
+        obj,
+        0,
+        v8impl::ReferenceOwnership::kUserland,
+        finalize_cb,
+        native_object,
+        finalize_hint);
     *result = reinterpret_cast<napi_ref>(reference);
-  } else {
+  } else if (finalize_cb != nullptr) {
     // Create a self-deleting reference.
-    reference = v8impl::Reference::New(
+    reference = v8impl::ReferenceWithFinalizer::New(
         env,
         obj,
         0,
-        v8impl::Ownership::kRuntime,
+        v8impl::ReferenceOwnership::kRuntime,
         finalize_cb,
         native_object,
-        finalize_cb == nullptr ? nullptr : finalize_hint);
+        finalize_hint);
+  } else {
+    // Create a self-deleting reference.
+    reference = v8impl::ReferenceWithData::New(
+        env, obj, 0, v8impl::ReferenceOwnership::kRuntime, native_object);
   }
 
   CHECK(obj->SetPrivate(context,
@@ -621,27 +594,46 @@ inline bool CanBeHeldWeakly(v8::Local<v8::Value> value) {
 
 }  // end of anonymous namespace
 
+void Finalizer::ResetEnv() {
+  env_ = nullptr;
+}
+
 void Finalizer::ResetFinalizer() {
   finalize_callback_ = nullptr;
   finalize_data_ = nullptr;
   finalize_hint_ = nullptr;
 }
 
+void Finalizer::CallFinalizer() {
+  napi_finalize finalize_callback = finalize_callback_;
+  void* finalize_data = finalize_data_;
+  void* finalize_hint = finalize_hint_;
+  ResetFinalizer();
+
+  if (finalize_callback == nullptr) return;
+  if (env_ == nullptr) {
+    // The environment is dead. Call the finalizer directly.
+    finalize_callback(nullptr, finalize_data, finalize_hint);
+  } else {
+    env_->CallFinalizer(finalize_callback, finalize_data, finalize_hint);
+  }
+}
+
 TrackedFinalizer::TrackedFinalizer(napi_env env,
                                    napi_finalize finalize_callback,
                                    void* finalize_data,
                                    void* finalize_hint)
-    : Finalizer(env, finalize_callback, finalize_data, finalize_hint),
-      RefTracker() {
-  Link(finalize_callback == nullptr ? &env->reflist : &env->finalizing_reflist);
-}
+    : RefTracker(),
+      finalizer_(env, finalize_callback, finalize_data, finalize_hint) {}
 
 TrackedFinalizer* TrackedFinalizer::New(napi_env env,
                                         napi_finalize finalize_callback,
                                         void* finalize_data,
                                         void* finalize_hint) {
-  return new TrackedFinalizer(
+  TrackedFinalizer* finalizer = new TrackedFinalizer(
       env, finalize_callback, finalize_data, finalize_hint);
+  finalizer->Link(&env->finalizing_reflist);
+  return finalizer;
 }
 
 // When a TrackedFinalizer is being deleted, it may have been queued to call its
@@ -650,115 +642,44 @@ TrackedFinalizer::~TrackedFinalizer() {
   // Remove from the env's tracked list.
   Unlink();
   // Try to remove the finalizer from the scheduled second pass callback.
-  env_->DequeueFinalizer(this);
+  finalizer_.env()->DequeueFinalizer(this);
 }
 
 void TrackedFinalizer::Finalize() {
-  FinalizeCore(/*deleteMe:*/ true);
-}
-
-void TrackedFinalizer::FinalizeCore(bool deleteMe) {
-  // Swap out the field finalize_callback so that it can not be accidentally
-  // called more than once.
-  napi_finalize finalize_callback = finalize_callback_;
-  void* finalize_data = finalize_data_;
-  void* finalize_hint = finalize_hint_;
-  ResetFinalizer();
-
-  // Either the RefBase is going to be deleted in the finalize_callback or not,
-  // it should be removed from the tracked list.
   Unlink();
-  // If the finalize_callback is present, it should either delete the
-  // derived RefBase, or the RefBase ownership was set to Ownership::kRuntime
-  // and the deleteMe parameter is true.
-  if (finalize_callback != nullptr) {
-    env_->CallFinalizer(finalize_callback, finalize_data, finalize_hint);
-  }
-
-  if (deleteMe) {
-    delete this;
-  }
-}
-
-// Wrapper around v8impl::Persistent that implements reference counting.
-RefBase::RefBase(napi_env env,
-                 uint32_t initial_refcount,
-                 Ownership ownership,
-                 napi_finalize finalize_callback,
-                 void* finalize_data,
-                 void* finalize_hint)
-    : TrackedFinalizer(env, finalize_callback, finalize_data, finalize_hint),
-      refcount_(initial_refcount),
-      ownership_(ownership) {}
-
-RefBase* RefBase::New(napi_env env,
-                      uint32_t initial_refcount,
-                      Ownership ownership,
-                      napi_finalize finalize_callback,
-                      void* finalize_data,
-                      void* finalize_hint) {
-  return new RefBase(env,
-                     initial_refcount,
-                     ownership,
-                     finalize_callback,
-                     finalize_data,
-                     finalize_hint);
-}
-
-void* RefBase::Data() {
-  return finalize_data_;
-}
-
-uint32_t RefBase::Ref() {
-  return ++refcount_;
-}
-
-uint32_t RefBase::Unref() {
-  if (refcount_ == 0) {
-    return 0;
-  }
-  return --refcount_;
+  finalizer_.CallFinalizer();
+  delete this;
 }
 
-uint32_t RefBase::RefCount() {
-  return refcount_;
-}
-
-void RefBase::Finalize() {
-  // If the RefBase is not Ownership::kRuntime, userland code should delete it.
-  // Delete it if it is Ownership::kRuntime.
-  FinalizeCore(/*deleteMe:*/ ownership_ == Ownership::kRuntime);
-}
-
-template <typename... Args>
-Reference::Reference(napi_env env, v8::Local<v8::Value> value, Args&&... args)
-    : RefBase(env, std::forward<Args>(args)...),
+Reference::Reference(napi_env env,
+                     v8::Local<v8::Value> value,
+                     uint32_t initial_refcount,
+                     ReferenceOwnership ownership)
+    : RefTracker(),
       persistent_(env->isolate, value),
+      refcount_(initial_refcount),
+      ownership_(ownership),
       can_be_weak_(CanBeHeldWeakly(value)) {
-  if (RefCount() == 0) {
+  if (refcount_ == 0) {
     SetWeak();
   }
 }
 
 Reference::~Reference() {
   // Reset the handle. And no weak callback will be invoked.
   persistent_.Reset();
+
+  // Remove from the env's tracked list.
+  Unlink();
 }
 
 Reference* Reference::New(napi_env env,
                           v8::Local<v8::Value> value,
                           uint32_t initial_refcount,
-                          Ownership ownership,
-                          napi_finalize finalize_callback,
-                          void* finalize_data,
-                          void* finalize_hint) {
-  return new Reference(env,
-                       value,
-                       initial_refcount,
-                       ownership,
-                       finalize_callback,
-                       finalize_data,
-                       finalize_hint);
+                          ReferenceOwnership ownership) {
+  Reference* reference = new Reference(env, value, initial_refcount, ownership);
+  reference->Link(&env->reflist);
+  return reference;
 }
 
 uint32_t Reference::Ref() {
@@ -767,32 +688,29 @@ uint32_t Reference::Ref() {
   if (persistent_.IsEmpty()) {
     return 0;
   }
-  uint32_t refcount = RefBase::Ref();
-  if (refcount == 1 && can_be_weak_) {
+  if (++refcount_ == 1 && can_be_weak_) {
     persistent_.ClearWeak();
   }
-  return refcount;
+  return refcount_;
 }
 
 uint32_t Reference::Unref() {
   // When the persistent_ is cleared in the WeakCallback, and a second pass
   // callback is pending, return 0 unconditionally.
-  if (persistent_.IsEmpty()) {
+  if (persistent_.IsEmpty() || refcount_ == 0) {
     return 0;
   }
-  uint32_t old_refcount = RefCount();
-  uint32_t refcount = RefBase::Unref();
-  if (old_refcount == 1 && refcount == 0) {
+  if (--refcount_ == 0) {
     SetWeak();
   }
-  return refcount;
+  return refcount_;
 }
 
-v8::Local<v8::Value> Reference::Get() {
+v8::Local<v8::Value> Reference::Get(napi_env env) {
   if (persistent_.IsEmpty()) {
     return v8::Local<v8::Value>();
   } else {
-    return v8::Local<v8::Value>::New(env_->isolate, persistent_);
+    return v8::Local<v8::Value>::New(env->isolate, persistent_);
   }
 }
 
@@ -801,12 +719,30 @@ void Reference::Finalize() {
   // be invoked again.
   persistent_.Reset();
 
-  // Chain up to perform the rest of the finalization.
-  RefBase::Finalize();
+  // If the Reference is not ReferenceOwnership::kRuntime, userland code should
+  // delete it. Delete it if it is ReferenceOwnership::kRuntime.
+  bool deleteMe = ownership_ == ReferenceOwnership::kRuntime;
+
+  // Whether the Reference is going to be deleted in the finalize_callback
+  // or not, it should be removed from the tracked list.
+  Unlink();
+
+  // If the finalize_callback is present, it should either delete the
+  // derived Reference, or the Reference ownership was set to
+  // ReferenceOwnership::kRuntime and the deleteMe parameter is true.
+  CallUserFinalizer();
+
+  if (deleteMe) {
+    delete this;
+  }
 }
 
-// Mark the reference as weak and eligible for collection
-// by the gc.
+// Call the Finalize immediately since there is no user finalizer to call.
+void Reference::InvokeFinalizerFromGC() {
+  Finalize();
+}
+
+// Mark the reference as weak and eligible for collection by the GC.
 void Reference::SetWeak() {
   if (can_be_weak_) {
     persistent_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
@@ -815,14 +751,76 @@ void Reference::SetWeak() {
   }
 }
 
-// The N-API finalizer callback may make calls into the engine. V8's heap is
-// not in a consistent state during the weak callback, and therefore it does
-// not support calls back into it. Enqueue the invocation of the finalizer.
+// Static function called by GC. Delegate the call to the reference instance.
 void Reference::WeakCallback(const v8::WeakCallbackInfo<Reference>& data) {
   Reference* reference = data.GetParameter();
-  // The reference must be reset during the weak callback as the API protocol.
+  // The reference must be reset during the weak callback per V8 API protocol.
   reference->persistent_.Reset();
-  reference->env_->InvokeFinalizerFromGC(reference);
+  reference->InvokeFinalizerFromGC();
+}
+
+ReferenceWithData* ReferenceWithData::New(napi_env env,
+                                          v8::Local<v8::Value> value,
+                                          uint32_t initial_refcount,
+                                          ReferenceOwnership ownership,
+                                          void* data) {
+  ReferenceWithData* reference =
+      new ReferenceWithData(env, value, initial_refcount, ownership, data);
+  reference->Link(&env->reflist);
+  return reference;
+}
+
+ReferenceWithData::ReferenceWithData(napi_env env,
+                                     v8::Local<v8::Value> value,
+                                     uint32_t initial_refcount,
+                                     ReferenceOwnership ownership,
+                                     void* data)
+    : Reference(env, value, initial_refcount, ownership), data_(data) {}
+
+ReferenceWithFinalizer* ReferenceWithFinalizer::New(
+    napi_env env,
+    v8::Local<v8::Value> value,
+    uint32_t initial_refcount,
+    ReferenceOwnership ownership,
+    napi_finalize finalize_callback,
+    void* finalize_data,
+    void* finalize_hint) {
+  ReferenceWithFinalizer* reference =
+      new ReferenceWithFinalizer(env,
+                                 value,
+                                 initial_refcount,
+                                 ownership,
+                                 finalize_callback,
+                                 finalize_data,
+                                 finalize_hint);
+  reference->Link(&env->finalizing_reflist);
+  return reference;
+}
+
+ReferenceWithFinalizer::ReferenceWithFinalizer(napi_env env,
+                                               v8::Local<v8::Value> value,
+                                               uint32_t initial_refcount,
+                                               ReferenceOwnership ownership,
+                                               napi_finalize finalize_callback,
+                                               void* finalize_data,
+                                               void* finalize_hint)
+    : Reference(env, value, initial_refcount, ownership),
+      finalizer_(env, finalize_callback, finalize_data, finalize_hint) {}
+
+ReferenceWithFinalizer::~ReferenceWithFinalizer() {
+  // Try to remove the finalizer from the scheduled second pass callback.
+  finalizer_.env()->DequeueFinalizer(this);
+}
+
+void ReferenceWithFinalizer::CallUserFinalizer() {
+  finalizer_.CallFinalizer();
+}
+
+// The Node-API finalizer callback may make calls into the engine. V8's heap is
+// not in a consistent state during the weak callback, and therefore it does
+// not support calls back into it. Enqueue the invocation of the finalizer.
+void ReferenceWithFinalizer::InvokeFinalizerFromGC() {
+  finalizer_.env()->InvokeFinalizerFromGC(this);
 }
 
 /**
@@ -2051,8 +2049,8 @@ napi_status NAPI_CDECL napi_get_cb_info(
   CHECK_ENV(env);
   CHECK_ARG(env, cbinfo);
 
-  v8impl::CallbackWrapper* info =
-      reinterpret_cast<v8impl::CallbackWrapper*>(cbinfo);
+  v8impl::FunctionCallbackWrapper* info =
+      reinterpret_cast<v8impl::FunctionCallbackWrapper*>(cbinfo);
 
   if (argv != nullptr) {
     CHECK_ARG(env, argc);
@@ -2078,8 +2076,8 @@ napi_status NAPI_CDECL napi_get_new_target(napi_env env,
   CHECK_ARG(env, cbinfo);
   CHECK_ARG(env, result);
 
-  v8impl::CallbackWrapper* info =
-      reinterpret_cast<v8impl::CallbackWrapper*>(cbinfo);
+  v8impl::FunctionCallbackWrapper* info =
+      reinterpret_cast<v8impl::FunctionCallbackWrapper*>(cbinfo);
 
   *result = info->GetNewTarget();
   return napi_clear_last_error(env);
@@ -2605,13 +2603,13 @@ napi_create_external(napi_env env,
   if (finalize_cb) {
     // The Reference object will delete itself after invoking the finalizer
     // callback.
-    v8impl::Reference::New(env,
-                           external_value,
-                           0,
-                           v8impl::Ownership::kRuntime,
-                           finalize_cb,
-                           data,
-                           finalize_hint);
+    v8impl::ReferenceWithFinalizer::New(env,
+                                        external_value,
+                                        0,
+                                        v8impl::ReferenceOwnership::kRuntime,
+                                        finalize_cb,
+                                        data,
+                                        finalize_hint);
   }
 
   *result = v8impl::JsValueFromV8LocalValue(external_value);
@@ -2746,7 +2744,7 @@ napi_status NAPI_CDECL napi_create_reference(napi_env env,
   }
 
   v8impl::Reference* reference = v8impl::Reference::New(
-      env, v8_value, initial_refcount, v8impl::Ownership::kUserland);
+      env, v8_value, initial_refcount, v8impl::ReferenceOwnership::kUserland);
 
   *result = reinterpret_cast<napi_ref>(reference);
   return napi_clear_last_error(env);
@@ -2802,7 +2800,7 @@ napi_status NAPI_CDECL napi_reference_unref(napi_env env,
 
   v8impl::Reference* reference = reinterpret_cast<v8impl::Reference*>(ref);
 
-  if (reference->RefCount() == 0) {
+  if (reference->refcount() == 0) {
     return napi_set_last_error(env, napi_generic_failure);
   }
 
@@ -2828,7 +2826,7 @@ napi_status NAPI_CDECL napi_get_reference_value(napi_env env,
   CHECK_ARG(env, result);
 
   v8impl::Reference* reference = reinterpret_cast<v8impl::Reference*>(ref);
-  *result = v8impl::JsValueFromV8LocalValue(reference->Get());
+  *result = v8impl::JsValueFromV8LocalValue(reference->Get(env));
 
   return napi_clear_last_error(env);
 }
@@ -3439,10 +3437,10 @@ napi_add_finalizer(napi_env env,
 
   // Create a self-deleting reference if the optional out-param result is not
   // set.
-  v8impl::Ownership ownership = result == nullptr
-                                    ? v8impl::Ownership::kRuntime
-                                    : v8impl::Ownership::kUserland;
-  v8impl::Reference* reference = v8impl::Reference::New(
+  v8impl::ReferenceOwnership ownership =
+      result == nullptr ? v8impl::ReferenceOwnership::kRuntime
+                        : v8impl::ReferenceOwnership::kUserland;
+  v8impl::Reference* reference = v8impl::ReferenceWithFinalizer::New(
       env, v8_value, 0, ownership, finalize_cb, finalize_data, finalize_hint);
 
   if (result != nullptr) {
@@ -3485,15 +3483,16 @@ napi_status NAPI_CDECL napi_set_instance_data(node_api_basic_env basic_env,
   napi_env env = const_cast<napi_env>(basic_env);
   CHECK_ENV(env);
 
-  v8impl::RefBase* old_data = static_cast<v8impl::RefBase*>(env->instance_data);
+  v8impl::TrackedFinalizer* old_data =
+      static_cast<v8impl::TrackedFinalizer*>(env->instance_data);
   if (old_data != nullptr) {
     // Our contract so far has been to not finalize any old data there may be.
     // So we simply delete it.
     delete old_data;
   }
 
-  env->instance_data = v8impl::RefBase::New(
-      env, 0, v8impl::Ownership::kRuntime, finalize_cb, data, finalize_hint);
+  env->instance_data =
+      v8impl::TrackedFinalizer::New(env, finalize_cb, data, finalize_hint);
 
   return napi_clear_last_error(env);
 }
@@ -3503,9 +3502,10 @@ napi_status NAPI_CDECL napi_get_instance_data(node_api_basic_env env,
   CHECK_ENV(env);
   CHECK_ARG(env, data);
 
-  v8impl::RefBase* idata = static_cast<v8impl::RefBase*>(env->instance_data);
+  v8impl::TrackedFinalizer* idata =
+      static_cast<v8impl::TrackedFinalizer*>(env->instance_data);
 
-  *data = (idata == nullptr ? nullptr : idata->Data());
+  *data = (idata == nullptr ? nullptr : idata->data());
 
   return napi_clear_last_error(env);
 }

src/js_native_api_v8.h

@@ -8,14 +8,15 @@ inline napi_status napi_clear_last_error(node_api_basic_env env);
 
 namespace v8impl {
 
+// Base class to track references and finalizers in a doubly linked list.
 class RefTracker {
  public:
+  using RefList = RefTracker;
+
   RefTracker() = default;
   virtual ~RefTracker() = default;
   virtual void Finalize() {}
 
-  typedef RefTracker RefList;
-
   inline void Link(RefList* list) {
     prev_ = list;
     next_ = list->next_;
@@ -47,7 +48,6 @@ class RefTracker {
   RefList* prev_ = nullptr;
 };
 
-class Finalizer;
 }  // end of namespace v8impl
 
 struct napi_env__ {
@@ -99,11 +99,7 @@ struct napi_env__ {
     }
   }
 
-  // Call finalizer immediately.
-  virtual void CallFinalizer(napi_finalize cb, void* data, void* hint) {
-    v8::HandleScope handle_scope(isolate);
-    CallIntoModule([&](napi_env env) { cb(env, data, hint); });
-  }
+  virtual void CallFinalizer(napi_finalize cb, void* data, void* hint) = 0;
 
   // Invoke finalizer from V8 garbage collector.
   void InvokeFinalizerFromGC(v8impl::RefTracker* finalizer);
@@ -323,7 +319,7 @@ inline v8::Local<v8::Value> V8LocalValueFromJsValue(napi_value v) {
 
 // Adapter for napi_finalize callbacks.
 class Finalizer {
- protected:
+ public:
   Finalizer(napi_env env,
             napi_finalize finalize_callback,
             void* finalize_data,
@@ -333,23 +329,14 @@ class Finalizer {
         finalize_data_(finalize_data),
         finalize_hint_(finalize_hint) {}
 
-  virtual ~Finalizer() = default;
-
- public:
-  static Finalizer* New(napi_env env,
-                        napi_finalize finalize_callback = nullptr,
-                        void* finalize_data = nullptr,
-                        void* finalize_hint = nullptr) {
-    return new Finalizer(env, finalize_callback, finalize_data, finalize_hint);
-  }
-
-  napi_finalize callback() { return finalize_callback_; }
+  napi_env env() { return env_; }
   void* data() { return finalize_data_; }
-  void* hint() { return finalize_hint_; }
 
+  void ResetEnv();
   void ResetFinalizer();
+  void CallFinalizer();
 
- protected:
+ private:
   napi_env env_;
   napi_finalize finalize_callback_;
   void* finalize_data_;
@@ -370,101 +357,129 @@ class TryCatch : public v8::TryCatch {
   napi_env _env;
 };
 
-// Ownership of a reference.
-enum class Ownership {
-  // The reference is owned by the runtime. No userland call is needed to
-  // destruct the reference.
-  kRuntime,
-  // The reference is owned by the userland. User code is responsible to delete
-  // the reference with appropriate node-api calls.
-  kUserland,
-};
-
 // Wrapper around Finalizer that can be tracked.
-class TrackedFinalizer : public Finalizer, public RefTracker {
- protected:
-  TrackedFinalizer(napi_env env,
-                   napi_finalize finalize_callback,
-                   void* finalize_data,
-                   void* finalize_hint);
-
+class TrackedFinalizer final : public RefTracker {
  public:
   static TrackedFinalizer* New(napi_env env,
                                napi_finalize finalize_callback,
                                void* finalize_data,
                                void* finalize_hint);
   ~TrackedFinalizer() override;
 
- protected:
-  void Finalize() override;
-  void FinalizeCore(bool deleteMe);
-};
+  void* data() { return finalizer_.data(); }
 
-// Wrapper around TrackedFinalizer that implements reference counting.
-class RefBase : public TrackedFinalizer {
- protected:
-  RefBase(napi_env env,
-          uint32_t initial_refcount,
-          Ownership ownership,
-          napi_finalize finalize_callback,
-          void* finalize_data,
-          void* finalize_hint);
-
- public:
-  static RefBase* New(napi_env env,
-                      uint32_t initial_refcount,
-                      Ownership ownership,
-                      napi_finalize finalize_callback,
-                      void* finalize_data,
-                      void* finalize_hint);
-
-  void* Data();
-  uint32_t Ref();
-  uint32_t Unref();
-  uint32_t RefCount();
-
-  Ownership ownership() { return ownership_; }
-
- protected:
+ private:
+  TrackedFinalizer(napi_env env,
+                   napi_finalize finalize_callback,
+                   void* finalize_data,
+                   void* finalize_hint);
   void Finalize() override;
 
  private:
-  uint32_t refcount_;
-  Ownership ownership_;
+  Finalizer finalizer_;
 };
 
-// Wrapper around v8impl::Persistent.
-class Reference : public RefBase {
- protected:
-  template <typename... Args>
-  Reference(napi_env env, v8::Local<v8::Value> value, Args&&... args);
+// Ownership of a reference.
+enum class ReferenceOwnership : uint8_t {
+  // The reference is owned by the runtime. No userland call is needed to
+  // destruct the reference.
+  kRuntime,
+  // The reference is owned by the userland. User code is responsible to delete
+  // the reference with appropriate node-api calls.
+  kUserland,
+};
 
+// Wrapper around v8impl::Persistent.
+class Reference : public RefTracker {
  public:
   static Reference* New(napi_env env,
                         v8::Local<v8::Value> value,
                         uint32_t initial_refcount,
-                        Ownership ownership,
-                        napi_finalize finalize_callback = nullptr,
-                        void* finalize_data = nullptr,
-                        void* finalize_hint = nullptr);
+                        ReferenceOwnership ownership);
+  ~Reference() override;
 
-  virtual ~Reference();
   uint32_t Ref();
   uint32_t Unref();
-  v8::Local<v8::Value> Get();
+  v8::Local<v8::Value> Get(napi_env env);
+
+  virtual void ResetFinalizer() {}
+  virtual void* Data() { return nullptr; }
+
+  uint32_t refcount() const { return refcount_; }
+  ReferenceOwnership ownership() { return ownership_; }
 
  protected:
-  void Finalize() override;
+  Reference(napi_env env,
+            v8::Local<v8::Value> value,
+            uint32_t initial_refcount,
+            ReferenceOwnership ownership);
+  virtual void CallUserFinalizer() {}
+  virtual void InvokeFinalizerFromGC();
 
  private:
   static void WeakCallback(const v8::WeakCallbackInfo<Reference>& data);
-
   void SetWeak();
+  void Finalize() override;
 
+ private:
   v8impl::Persistent<v8::Value> persistent_;
+  uint32_t refcount_;
+  ReferenceOwnership ownership_;
   bool can_be_weak_;
 };
 
+// Reference that can store additional data.
+class ReferenceWithData final : public Reference {
+ public:
+  static ReferenceWithData* New(napi_env env,
+                                v8::Local<v8::Value> value,
+                                uint32_t initial_refcount,
+                                ReferenceOwnership ownership,
+                                void* data);
+
+  void* Data() override { return data_; }
+
+ private:
+  ReferenceWithData(napi_env env,
+                    v8::Local<v8::Value> value,
+                    uint32_t initial_refcount,
+                    ReferenceOwnership ownership,
+                    void* data);
+
+ private:
+  void* data_;
+};
+
+// Reference that has a user finalizer callback.
+class ReferenceWithFinalizer final : public Reference {
+ public:
+  static ReferenceWithFinalizer* New(napi_env env,
+                                     v8::Local<v8::Value> value,
+                                     uint32_t initial_refcount,
+                                     ReferenceOwnership ownership,
+                                     napi_finalize finalize_callback,
+                                     void* finalize_data,
+                                     void* finalize_hint);
+  ~ReferenceWithFinalizer() override;
+
+  void ResetFinalizer() override { finalizer_.ResetFinalizer(); }
+  void* Data() override { return finalizer_.data(); }
+
+ private:
+  ReferenceWithFinalizer(napi_env env,
+                         v8::Local<v8::Value> value,
+                         uint32_t initial_refcount,
+                         ReferenceOwnership ownership,
+                         napi_finalize finalize_callback,
+                         void* finalize_data,
+                         void* finalize_hint);
+  void CallUserFinalizer() override;
+  void InvokeFinalizerFromGC() override;
+
+ private:
+  Finalizer finalizer_;
+};
+
 }  // end of namespace v8impl
 
 #endif  // SRC_JS_NATIVE_API_V8_H_

src/node_api.cc

@@ -121,23 +121,18 @@ namespace {
 class BufferFinalizer : private Finalizer {
  public:
   static BufferFinalizer* New(napi_env env,
-                              napi_finalize finalize_callback = nullptr,
-                              void* finalize_data = nullptr,
-                              void* finalize_hint = nullptr) {
+                              napi_finalize finalize_callback,
+                              void* finalize_data,
+                              void* finalize_hint) {
     return new BufferFinalizer(
         env, finalize_callback, finalize_data, finalize_hint);
   }
   // node::Buffer::FreeCallback
   static void FinalizeBufferCallback(char* data, void* hint) {
     std::unique_ptr<BufferFinalizer, Deleter> finalizer{
         static_cast<BufferFinalizer*>(hint)};
-    finalizer->finalize_data_ = data;
-
     // It is safe to call into JavaScript at this point.
-    if (finalizer->finalize_callback_ == nullptr) return;
-    finalizer->env_->CallFinalizer(finalizer->finalize_callback_,
-                                   finalizer->finalize_data_,
-                                   finalizer->finalize_hint_);
+    finalizer->CallFinalizer();
   }
 
   struct Deleter {
@@ -150,10 +145,10 @@ class BufferFinalizer : private Finalizer {
                   void* finalize_data,
                   void* finalize_hint)
       : Finalizer(env, finalize_callback, finalize_data, finalize_hint) {
-    env_->Ref();
+    env->Ref();
   }
 
-  ~BufferFinalizer() { env_->Unref(); }
+  ~BufferFinalizer() { env()->Unref(); }
 };
 
 void ThrowNodeApiVersionError(node::Environment* node_env,
@@ -1057,7 +1052,7 @@ napi_create_external_buffer(napi_env env,
 
   // The finalizer object will delete itself after invoking the callback.
   v8impl::BufferFinalizer* finalizer =
-      v8impl::BufferFinalizer::New(env, finalize_cb, nullptr, finalize_hint);
+      v8impl::BufferFinalizer::New(env, finalize_cb, data, finalize_hint);
 
   v8::MaybeLocal<v8::Object> maybe =
       node::Buffer::New(isolate,