node-api: make reference weak parameter an indirect link to references

src/js_native_api_v8.cc

@@ -314,16 +314,16 @@ class RefBase : protected Finalizer, RefTracker {
 };
 
 class Reference : public RefBase {
+  using SecondPassCallParameterRef = Reference*;
+
  protected:
   template <typename... Args>
-  Reference(napi_env env,
-            v8::Local<v8::Value> value,
-            Args&&... args)
+  Reference(napi_env env, v8::Local<v8::Value> value, Args&&... args)
       : RefBase(env, std::forward<Args>(args)...),
-            _persistent(env->isolate, value) {
+        _persistent(env->isolate, value),
+        _secondPassParameter(new SecondPassCallParameterRef(this)) {
     if (RefCount() == 0) {
-      _persistent.SetWeak(
-          this, FinalizeCallback, v8::WeakCallbackType::kParameter);
+      SetWeak();
     }
   }
 
@@ -344,10 +344,19 @@ class Reference : public RefBase {
                          finalize_hint);
   }
 
+  virtual ~Reference() {
+    // If the second pass callback is scheduled, it will delete the
+    // parameter passed to it, otherwise it will never be scheduled
+    // and we need to delete it here.
+    if (_secondPassParameter != nullptr) {
+      delete _secondPassParameter;
+    }
+  }
+
   inline uint32_t Ref() {
     uint32_t refcount = RefBase::Ref();
     if (refcount == 1) {
-      _persistent.ClearWeak();
+      ClearWeak();
     }
     return refcount;
   }
@@ -356,8 +365,7 @@ class Reference : public RefBase {
     uint32_t old_refcount = RefCount();
     uint32_t refcount = RefBase::Unref();
     if (old_refcount == 1 && refcount == 0) {
-      _persistent.SetWeak(
-          this, FinalizeCallback, v8::WeakCallbackType::kParameter);
+      SetWeak();
     }
     return refcount;
   }
@@ -374,35 +382,90 @@ class Reference : public RefBase {
   inline void Finalize(bool is_env_teardown = false) override {
     // During env teardown, `~napi_env()` alone is responsible for finalizing.
     // Thus, we don't want any stray gc passes to trigger a second call to
-    // `Finalize()`, so let's reset the persistent here if nothing is
-    // keeping it alive.
-    if (is_env_teardown && _persistent.IsWeak()) {
-      _persistent.ClearWeak();
+    // `RefBase::Finalize()`. ClearWeak will ensure that even if the
+    // gc is in progress no Finalization will be run for this Reference
+    // by the gc.
+    if (is_env_teardown) {
+      ClearWeak();
     }
 
     // Chain up to perform the rest of the finalization.
     RefBase::Finalize(is_env_teardown);
   }
 
  private:
+  // ClearWeak is marking the Reference so that the gc should not
+  // collect it, but it is possible that a second pass callback
+  // may have been scheduled already if we are in shutdown. We clear
+  // the secondPassParameter so that even if it has been
+  // secheduled no Finalization will be run.
+  inline void ClearWeak() {
+    if (!_persistent.IsEmpty()) {
+      _persistent.ClearWeak();
+    }
+    if (_secondPassParameter != nullptr) {
+      *_secondPassParameter = nullptr;
+    }
+  }
+
+  // Mark the reference as weak and eligible for collection
+  // by the gc.
+  inline void SetWeak() {
+    if (_secondPassParameter == nullptr) {
+      // This means that the Reference has already been processed
+      // by the second pass calback, so its already been Finalized, do
+      // nothing
+      return;
+    }
+    _persistent.SetWeak(
+        _secondPassParameter, FinalizeCallback, v8::WeakCallbackType::kParameter);
+    *_secondPassParameter = this;
+  }
+
   // 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. However, it provides a mechanism for adding
   // a finalizer which may make calls back into the engine by allowing us to
   // attach such a second-pass finalizer from the first pass finalizer. Thus,
   // we do that here to ensure that the N-API finalizer callback is free to call
   // into the engine.
-  static void FinalizeCallback(const v8::WeakCallbackInfo<Reference>& data) {
-    Reference* reference = data.GetParameter();
+  static void FinalizeCallback(
+      const v8::WeakCallbackInfo<SecondPassCallParameterRef>& data) {
+    SecondPassCallParameterRef* parameter = data.GetParameter();
+    Reference* reference = *parameter;
+    if (reference == nullptr) {
+      return;
+    }
 
     // The reference must be reset during the first pass.
     reference->_persistent.Reset();
+    // Mark the parameter not delete-able until the second pass callback is
+    // invoked.
+    reference->_secondPassParameter = nullptr;
 
     data.SetSecondPassCallback(SecondPassCallback);
   }
 
-  static void SecondPassCallback(const v8::WeakCallbackInfo<Reference>& data) {
-    data.GetParameter()->Finalize();
+  // Second pass callbacks are scheduled with platform tasks. At env teardown,
+  // the tasks may have already be scheduled and we are unable to cancel the
+  // second pass callback task. We have to make sure that parameter is kept
+  // alive until the second pass callback is been invoked. In order to do
+  // this and still allow our code to Finalize/delete the Reference during
+  // shutdown we have to use a seperately allocated parameter instead
+  // of a parameter within the Reference object itself. This is what
+  // is stored in _secondPassParameter and it is alocated in the
+  // constructor for the Reference.
+  SecondPassCallParameterRef* _secondPassParameter;
+  static void SecondPassCallback(
+      const v8::WeakCallbackInfo<SecondPassCallParameterRef>& data) {
+    SecondPassCallParameterRef* parameter = data.GetParameter();
+    Reference* reference = *parameter;
+    delete parameter;
+    if (reference == nullptr) {
+      // the reference itself has already been deleted so nothing to do
+      return;
+    }
+    reference->Finalize();
   }
 
   v8impl::Persistent<v8::Value> _persistent;