n-api: do not require JS Context for `napi_async_destroy()`

Allow the function to be called during GC, which is a common use case.

Fixes: https://github.com/nodejs/node/issues/27218

PR-URL: https://github.com/nodejs/node/pull/27255
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

3 files changed, 78 insertions, 2 deletions

diff --git a/src/node_api.cc b/src/node_api.cc
index ecacd7013f..ab48caa237 100644
--- a/src/node_api.cc
+++ b/src/node_api.cc
@@ -632,10 +632,11 @@ napi_status napi_async_destroy(napi_env env,
   CHECK_ENV(env);
   CHECK_ARG(env, async_context);
 
-  v8::Isolate* isolate = env->isolate;
   node::async_context* node_async_context =
       reinterpret_cast<node::async_context*>(async_context);
-  node::EmitAsyncDestroy(isolate, *node_async_context);
+  node::EmitAsyncDestroy(
+      reinterpret_cast<node_napi_env>(env)->node_env(),
+      *node_async_context);
 
   delete node_async_context;
 
diff --git a/test/node-api/test_make_callback/binding.c b/test/node-api/test_make_callback/binding.c
index 04e95e2570..5731e28a5b 100644
--- a/test/node-api/test_make_callback/binding.c
+++ b/test/node-api/test_make_callback/binding.c
@@ -1,4 +1,6 @@
+#define NAPI_EXPERIMENTAL  // napi_add_finalizer
 #include <node_api.h>
+#include <assert.h>
 #include "../../js-native-api/common.h"
 
 #define MAX_ARGUMENTS 10
@@ -44,6 +46,30 @@ static napi_value MakeCallback(napi_env env, napi_callback_info info) {
   return result;
 }
 
+static void AsyncDestroyCb(napi_env env, void* data, void* hint) {
+  napi_status status = napi_async_destroy(env, (napi_async_context)data);
+  // We cannot use NAPI_ASSERT_RETURN_VOID because we need to have a JS stack
+  // below in order to use exceptions.
+  assert(status == napi_ok);
+}
+
+static napi_value CreateAsyncResource(napi_env env, napi_callback_info info) {
+  napi_value object;
+  NAPI_CALL(env, napi_create_object(env, &object));
+
+  napi_value resource_name;
+  NAPI_CALL(env, napi_create_string_utf8(
+      env, "test_gcable", NAPI_AUTO_LENGTH, &resource_name));
+
+  napi_async_context context;
+  NAPI_CALL(env, napi_async_init(env, object, resource_name, &context));
+
+  NAPI_CALL(env, napi_add_finalizer(
+      env, object, (void*)context, AsyncDestroyCb, NULL, NULL));
+
+  return object;
+}
+
 static
 napi_value Init(napi_env env, napi_value exports) {
   napi_value fn;
@@ -51,6 +77,11 @@ napi_value Init(napi_env env, napi_value exports) {
       // NOLINTNEXTLINE (readability/null_usage)
       env, NULL, NAPI_AUTO_LENGTH, MakeCallback, NULL, &fn));
   NAPI_CALL(env, napi_set_named_property(env, exports, "makeCallback", fn));
+  NAPI_CALL(env, napi_create_function(
+      // NOLINTNEXTLINE (readability/null_usage)
+      env, NULL, NAPI_AUTO_LENGTH, CreateAsyncResource, NULL, &fn));
+  NAPI_CALL(env, napi_set_named_property(
+      env, exports, "createAsyncResource", fn));
   return exports;
 }
 
diff --git a/test/node-api/test_make_callback/test-async-hooks-gcable.js b/test/node-api/test_make_callback/test-async-hooks-gcable.js
new file mode 100644
index 0000000000..a9b4d3d75d
--- /dev/null
+++ b/test/node-api/test_make_callback/test-async-hooks-gcable.js
@@ -0,0 +1,44 @@
+'use strict';
+// Flags: --gc-interval=100 --gc-global
+
+const common = require('../../common');
+const assert = require('assert');
+const async_hooks = require('async_hooks');
+const { createAsyncResource } = require(`./build/${common.buildType}/binding`);
+
+// Test for https://github.com/nodejs/node/issues/27218:
+// napi_async_destroy() can be called during a regular garbage collection run.
+
+const hook_result = {
+  id: null,
+  init_called: false,
+  destroy_called: false,
+};
+
+const test_hook = async_hooks.createHook({
+  init: (id, type) => {
+    if (type === 'test_gcable') {
+      hook_result.id = id;
+      hook_result.init_called = true;
+    }
+  },
+  destroy: (id) => {
+    if (id === hook_result.id) hook_result.destroy_called = true;
+  },
+});
+
+test_hook.enable();
+createAsyncResource();
+
+// Trigger GC. This does *not* use global.gc(), because what we want to verify
+// is that `napi_async_destroy()` can be called when there is no JS context
+// on the stack at the time of GC.
+// Currently, using --gc-interval=100 + 1M elements seems to work fine for this.
+const arr = new Array(1024 * 1024);
+for (let i = 0; i < arr.length; i++)
+  arr[i] = {};
+
+assert.strictEqual(hook_result.destroy_called, false);
+setImmediate(() => {
+  assert.strictEqual(hook_result.destroy_called, true);
+});