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: #27218

PR-URL: #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>

Author: addaleax

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;
 

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,13 +46,42 @@ 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;
   NAPI_CALL(env, napi_create_function(
       // 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;
 }
 

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);
+});