node-api: allow napi_delete_reference in finalizers

`napi_delete_reference` must be called immediately for a
`napi_reference` returned from `napi_wrap` in the corresponding
finalizer, in order to clean up the `napi_reference` timely.

`napi_delete_reference` is safe to be invoked during GC.

PR-URL: https://github.com/nodejs/node/pull/55620
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com>

src/js_native_api_v8.cc

@@ -2769,10 +2769,12 @@ napi_status NAPI_CDECL napi_create_reference(napi_env env,
 
 // Deletes a reference. The referenced value is released, and may be GC'd unless
 // there are other references to it.
+// For a napi_reference returned from `napi_wrap`, this must be called in the
+// finalizer.
 napi_status NAPI_CDECL napi_delete_reference(napi_env env, napi_ref ref) {
   // Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw
   // JS exceptions.
-  CHECK_ENV_NOT_IN_GC(env);
+  CHECK_ENV(env);
   CHECK_ARG(env, ref);
 
   delete reinterpret_cast<v8impl::Reference*>(ref);

test/js-native-api/6_object_wrap/6_object_wrap.cc

@@ -3,6 +3,8 @@
 #include "assert.h"
 #include "myobject.h"
 
+typedef int32_t FinalizerData;
+
 napi_ref MyObject::constructor;
 
 MyObject::MyObject(double value)
@@ -10,10 +12,16 @@ MyObject::MyObject(double value)
 
 MyObject::~MyObject() { napi_delete_reference(env_, wrapper_); }
 
-void MyObject::Destructor(
-  napi_env env, void* nativeObject, void* /*finalize_hint*/) {
+void MyObject::Destructor(node_api_basic_env env,
+                          void* nativeObject,
+                          void* /*finalize_hint*/) {
   MyObject* obj = static_cast<MyObject*>(nativeObject);
   delete obj;
+
+  FinalizerData* data;
+  NODE_API_BASIC_CALL_RETURN_VOID(
+      env, napi_get_instance_data(env, reinterpret_cast<void**>(&data)));
+  *data += 1;
 }
 
 void MyObject::Init(napi_env env, napi_value exports) {
@@ -154,7 +162,7 @@ napi_value MyObject::Multiply(napi_env env, napi_callback_info info) {
 }
 
 // This finalizer should never be invoked.
-void ObjectWrapDanglingReferenceFinalizer(napi_env env,
+void ObjectWrapDanglingReferenceFinalizer(node_api_basic_env env,
                                           void* finalize_data,
                                           void* finalize_hint) {
   assert(0 && "unreachable");
@@ -198,8 +206,30 @@ napi_value ObjectWrapDanglingReferenceTest(napi_env env,
   return ret;
 }
 
+static napi_value GetFinalizerCallCount(napi_env env, napi_callback_info info) {
+  size_t argc = 1;
+  napi_value argv[1];
+  FinalizerData* data;
+  napi_value result;
+
+  NODE_API_CALL(env,
+                napi_get_cb_info(env, info, &argc, argv, nullptr, nullptr));
+  NODE_API_CALL(env,
+                napi_get_instance_data(env, reinterpret_cast<void**>(&data)));
+  NODE_API_CALL(env, napi_create_int32(env, *data, &result));
+  return result;
+}
+
+static void finalizeData(napi_env env, void* data, void* hint) {
+  delete reinterpret_cast<FinalizerData*>(data);
+}
+
 EXTERN_C_START
 napi_value Init(napi_env env, napi_value exports) {
+  FinalizerData* data = new FinalizerData;
+  *data = 0;
+  NODE_API_CALL(env, napi_set_instance_data(env, data, finalizeData, nullptr));
+
   MyObject::Init(env, exports);
 
   napi_property_descriptor descriptors[] = {
@@ -207,6 +237,7 @@ napi_value Init(napi_env env, napi_value exports) {
                                 ObjectWrapDanglingReference),
       DECLARE_NODE_API_PROPERTY("objectWrapDanglingReferenceTest",
                                 ObjectWrapDanglingReferenceTest),
+      DECLARE_NODE_API_PROPERTY("getFinalizerCallCount", GetFinalizerCallCount),
   };
 
   NODE_API_CALL(

test/js-native-api/6_object_wrap/binding.gyp

@@ -5,6 +5,13 @@
       "sources": [
         "6_object_wrap.cc"
       ]
+    },
+    {
+      "target_name": "6_object_wrap_basic_finalizer",
+      "defines": [ "NAPI_EXPERIMENTAL" ],
+      "sources": [
+        "6_object_wrap.cc"
+      ]
     }
   ]
 }

test/js-native-api/6_object_wrap/myobject.h

@@ -6,7 +6,9 @@
 class MyObject {
  public:
   static void Init(napi_env env, napi_value exports);
-  static void Destructor(napi_env env, void* nativeObject, void* finalize_hint);
+  static void Destructor(node_api_basic_env env,
+                         void* nativeObject,
+                         void* finalize_hint);
 
  private:
   explicit MyObject(double value_ = 0);

test/js-native-api/6_object_wrap/test-basic-finalizer.js

@@ -0,0 +1,24 @@
+// Flags: --expose-gc
+
+'use strict';
+const common = require('../../common');
+const assert = require('assert');
+const addon = require(`./build/${common.buildType}/6_object_wrap_basic_finalizer`);
+
+// This test verifies that ObjectWrap can be correctly finalized with a node_api_basic_finalizer
+// in the current JS loop tick
+(() => {
+  let obj = new addon.MyObject(9);
+  obj = null;
+  // Silent eslint about unused variables.
+  assert.strictEqual(obj, null);
+})();
+
+for (let i = 0; i < 10; ++i) {
+  global.gc();
+  if (addon.getFinalizerCallCount() === 1) {
+    break;
+  }
+}
+
+assert.strictEqual(addon.getFinalizerCallCount(), 1);