From 4e059447184ab072a000eee763c2a8797bf1e7bf Mon Sep 17 00:00:00 2001
From: Chengzhong Wu <cwu631@bloomberg.net>
Date: Thu, 31 Oct 2024 10:58:47 +0000
Subject: [PATCH] fix: missing napi_delete_reference on ObjectWrap ref

---
 napi-inl.h | 10 +++++++---
 napi.h     |  1 +
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/napi-inl.h b/napi-inl.h
index 47e047087..b21eac2ae 100644
--- a/napi-inl.h
+++ b/napi-inl.h
@@ -3341,6 +3341,8 @@ template <typename T>
 inline Reference<T>::~Reference() {
   if (_ref != nullptr) {
     if (!_suppressDestruct) {
+      // TODO(legendecas): napi_delete_reference should be invoked immediately.
+      // Fix this when https://github.com/nodejs/node/pull/55620 lands.
 #ifdef NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER
       Env().PostFinalizer(
           [](Napi::Env env, napi_ref ref) { napi_delete_reference(env, ref); },
@@ -4595,7 +4597,7 @@ template <typename T>
 inline ObjectWrap<T>::~ObjectWrap() {
   // If the JS object still exists at this point, remove the finalizer added
   // through `napi_wrap()`.
-  if (!IsEmpty()) {
+  if (!IsEmpty() && !_finalized) {
     Object object = Value();
     // It is not valid to call `napi_remove_wrap()` with an empty `object`.
     // This happens e.g. during garbage collection.
@@ -5044,8 +5046,10 @@ inline void ObjectWrap<T>::FinalizeCallback(node_addon_api_basic_env env,
   (void)env;
   T* instance = static_cast<T*>(data);
 
-  // Prevent ~ObjectWrap from calling napi_remove_wrap
-  instance->_ref = nullptr;
+  // Prevent ~ObjectWrap from calling napi_remove_wrap.
+  // The instance->_ref should be deleted with napi_delete_reference in
+  // ~Reference.
+  instance->_finalized = true;
 
   // If class overrides the basic finalizer, execute it.
   if constexpr (details::HasBasicFinalizer<T>::value) {
diff --git a/napi.h b/napi.h
index 7c103bf2c..99488f94e 100644
--- a/napi.h
+++ b/napi.h
@@ -2514,6 +2514,7 @@ class ObjectWrap : public InstanceWrap<T>, public Reference<Object> {
   }
 
   bool _construction_failed = true;
+  bool _finalized = false;
 };
 
 class HandleScope {