node-api: avoid calling virtual methods in base's dtor

src/js_native_api_v8.h

@@ -55,17 +55,14 @@ struct napi_env__ {
     CHECK_EQ(isolate, context->GetIsolate());
     napi_clear_last_error(this);
   }
-  virtual ~napi_env__() { FinalizeAll(); }
-  v8::Isolate* const isolate;  // Shortcut for context()->GetIsolate()
-  v8impl::Persistent<v8::Context> context_persistent;
 
   inline v8::Local<v8::Context> context() const {
     return v8impl::PersistentToLocal::Strong(context_persistent);
   }
 
   inline void Ref() { refs++; }
   inline void Unref() {
-    if (--refs == 0) delete this;
+    if (--refs == 0) DeleteMe();
   }
 
   virtual bool can_call_into_js() const { return true; }
@@ -99,16 +96,20 @@ struct napi_env__ {
     CallIntoModule([&](napi_env env) { cb(env, data, hint); });
   }
 
-  void FinalizeAll() {
+  virtual void DeleteMe() {
     // First we must finalize those references that have `napi_finalizer`
     // callbacks. The reason is that addons might store other references which
     // they delete during their `napi_finalizer` callbacks. If we deleted such
     // references here first, they would be doubly deleted when the
     // `napi_finalizer` deleted them subsequently.
     v8impl::RefTracker::FinalizeAll(&finalizing_reflist);
     v8impl::RefTracker::FinalizeAll(&reflist);
+    delete this;
   }
 
+  v8::Isolate* const isolate;  // Shortcut for context()->GetIsolate()
+  v8impl::Persistent<v8::Context> context_persistent;
+
   v8impl::Persistent<v8::Value> last_exception;
 
   // We store references in two different lists, depending on whether they have
@@ -121,6 +122,11 @@ struct napi_env__ {
   int open_callback_scopes = 0;
   int refs = 1;
   void* instance_data = nullptr;
+
+ protected:
+  // Should not be deleted directly. Delete with `napi_env__::DeleteMe()`
+  // instead.
+  virtual ~napi_env__() = default;
 };
 
 // This class is used to keep a napi_env live in a way that

src/node_api.cc

@@ -25,9 +25,9 @@ node_napi_env__::node_napi_env__(v8::Local<v8::Context> context,
   CHECK_NOT_NULL(node_env());
 }
 
-node_napi_env__::~node_napi_env__() {
+void node_napi_env__::DeleteMe() {
   destructing = true;
-  FinalizeAll();
+  napi_env__::DeleteMe();
 }
 
 bool node_napi_env__::can_call_into_js() const {

src/node_api_internals.h

@@ -11,7 +11,6 @@
 struct node_napi_env__ : public napi_env__ {
   node_napi_env__(v8::Local<v8::Context> context,
                   const std::string& module_filename);
-  ~node_napi_env__();
 
   bool can_call_into_js() const override;
   v8::Maybe<bool> mark_arraybuffer_as_untransferable(
@@ -24,6 +23,8 @@ struct node_napi_env__ : public napi_env__ {
   template <bool enforceUncaughtExceptionPolicy, typename T>
   void CallbackIntoModule(T&& call);
 
+  void DeleteMe() override;
+
   inline node::Environment* node_env() const {
     return node::Environment::GetCurrent(context());
   }