From 6ccd5a8e65dd6c35e5924394d10a98bc21e9c582 Mon Sep 17 00:00:00 2001
From: legendecas <legendecas@gmail.com>
Date: Wed, 7 Oct 2020 02:05:15 +0800
Subject: [PATCH] n-api: napi_make_callback emit async init with resource of
async_context
instead of emit async init with receiver of the callback.
---
doc/api/n-api.md | 36 +++-
src/node_api.cc | 187 +++++++++++++-----
test/node-api/test_async_context/binding.c | 128 ++++++++++++
test/node-api/test_async_context/binding.gyp | 9 +
.../test-gcable-callback.js | 65 ++++++
.../test-gcable.js} | 4 +-
test/node-api/test_async_context/test.js | 63 ++++++
test/node-api/test_make_callback/binding.c | 45 +----
.../test_make_callback/test-async-hooks.js | 11 +-
test/node-api/test_make_callback/test.js | 15 +-
10 files changed, 465 insertions(+), 98 deletions(-)
create mode 100644 test/node-api/test_async_context/binding.c
create mode 100644 test/node-api/test_async_context/binding.gyp
create mode 100644 test/node-api/test_async_context/test-gcable-callback.js
rename test/node-api/{test_make_callback/test-async-hooks-gcable.js => test_async_context/test-gcable.js} (95%)
create mode 100644 test/node-api/test_async_context/test.js
diff --git a/doc/api/n-api.md b/doc/api/n-api.md
index 287940d1d5485c..9f16d2d2933b90 100644
--- a/doc/api/n-api.md
+++ b/doc/api/n-api.md
@@ -5189,19 +5189,31 @@ napi_status napi_async_init(napi_env env,
* `[in] env`: The environment that the API is invoked under.
* `[in] async_resource`: Object associated with the async work
- that will be passed to possible `async_hooks` [`init` hooks][].
- In order to retain ABI compatibility with previous versions,
- passing `NULL` for `async_resource` does not result in an error. However,
- this results in incorrect operation of async hooks for the
- napi_async_context created. Potential issues include
- loss of async context when using the AsyncLocalStorage API.
-* `[in] async_resource_name`: Identifier for the kind of resource
- that is being provided for diagnostic information exposed by the
- `async_hooks` API.
+ that will be passed to possible `async_hooks` [`init` hooks][] and can be
+ accessed by [`async_hooks.executionAsyncResource()`][].
+* `[in] async_resource_name`: Identifier for the kind of resource that is being
+ provided for diagnostic information exposed by the `async_hooks` API.
* `[out] result`: The initialized async context.
Returns `napi_ok` if the API succeeded.
+The `async_resource` object needs to be kept alive until
+[`napi_async_destroy`][] to keep `async_hooks` related API acts correctly. In
+order to retain ABI compatibility with previous versions, `napi_async_context`s
+are not maintaining the strong reference to the `async_resource` objects to
+avoid introducing causing memory leaks. However, if the `async_resource` is
+garbage collected by JavaScript engine before the `napi_async_context` was
+destroyed by `napi_async_destroy`, calling `napi_async_context` related APIs
+like [`napi_open_callback_scope`][] and [`napi_make_callback`][] can cause
+problems like loss of async context when using the `AsyncLocalStoage` API.
+
+In order to retain ABI compatibility with previous versions, passing `NULL`
+for `async_resource` does not result in an error. However, this is not
+recommended as this will result poor results with `async_hooks`
+[`init` hooks][] and `async_hooks.executionAsyncResource()` as the resource is
+now required by the underlying `async_hooks` implementation in order to provide
+the linkage between async callbacks.
+
### napi_async_destroy
<!-- YAML
added: v8.6.0
@@ -5288,7 +5300,9 @@ NAPI_EXTERN napi_status napi_open_callback_scope(napi_env env,
* `[in] env`: The environment that the API is invoked under.
* `[in] resource_object`: An object associated with the async work
- that will be passed to possible `async_hooks` [`init` hooks][].
+ that will be passed to possible `async_hooks` [`init` hooks][]. This
+ parameter has been deprecated and is ignored at runtime. Use the
+ `async_resource` parameter in [`napi_async_init`][] instead.
* `[in] context`: Context for the async operation that is invoking the callback.
This should be a value previously obtained from [`napi_async_init`][].
* `[out] result`: The newly created scope.
@@ -5985,6 +5999,7 @@ This API may only be called from the main thread.
[`Number.MAX_SAFE_INTEGER`]: https://tc39.github.io/ecma262/#sec-number.max_safe_integer
[`Number.MIN_SAFE_INTEGER`]: https://tc39.github.io/ecma262/#sec-number.min_safe_integer
[`Worker`]: worker_threads.md#worker_threads_class_worker
+[`async_hooks.executionAsyncResource()`]: async_hooks.md#async_hooks_async_hooks_executionasyncresource
[`global`]: globals.md#globals_global
[`init` hooks]: async_hooks.md#async_hooks_init_asyncid_type_triggerasyncid_resource
[`napi_add_async_cleanup_hook`]: #n_api_napi_add_async_cleanup_hook
@@ -5992,6 +6007,7 @@ This API may only be called from the main thread.
[`napi_add_finalizer`]: #n_api_napi_add_finalizer
[`napi_async_cleanup_hook`]: #n_api_napi_async_cleanup_hook
[`napi_async_complete_callback`]: #n_api_napi_async_complete_callback
+[`napi_async_destroy`]: #n_api_napi_async_destroy
[`napi_async_init`]: #n_api_napi_async_init
[`napi_callback`]: #n_api_napi_callback
[`napi_cancel_async_work`]: #n_api_napi_cancel_async_work
diff --git a/src/node_api.cc b/src/node_api.cc
index 12f369a809734f..4e932c19c2bf8a 100644
--- a/src/node_api.cc
+++ b/src/node_api.cc
@@ -1,12 +1,15 @@
+#include "async_wrap-inl.h"
#include "env-inl.h"
#define NAPI_EXPERIMENTAL
#include "js_native_api_v8.h"
+#include "memory_tracker-inl.h"
#include "node_api.h"
#include "node_binding.h"
#include "node_buffer.h"
#include "node_errors.h"
#include "node_internals.h"
#include "threadpoolwork-inl.h"
+#include "tracing/traced_value.h"
#include "util-inl.h"
#include <memory>
@@ -104,16 +107,6 @@ static inline napi_env NewEnv(v8::Local<v8::Context> context) {
return result;
}
-static inline napi_callback_scope
-JsCallbackScopeFromV8CallbackScope(node::CallbackScope* s) {
- return reinterpret_cast<napi_callback_scope>(s);
-}
-
-static inline node::CallbackScope*
-V8CallbackScopeFromJsCallbackScope(napi_callback_scope s) {
- return reinterpret_cast<node::CallbackScope*>(s);
-}
-
static inline void trigger_fatal_exception(
napi_env env, v8::Local<v8::Value> local_err) {
v8::Local<v8::Message> local_msg =
@@ -435,6 +428,111 @@ class ThreadSafeFunction : public node::AsyncResource {
bool handles_closing;
};
+/**
+ * Compared to node::AsyncResource, the resource object in AsyncContext is
+ * gc-able. AsyncContext holds a weak reference to the resource object.
+ * AsyncContext::MakeCallback doesn't implicitly set the receiver of the
+ * callback to the resource object.
+ */
+class AsyncContext {
+ public:
+ AsyncContext(node_napi_env env,
+ v8::Local<v8::Object> resource_object,
+ const v8::Local<v8::String> resource_name,
+ bool externally_managed_resource)
+ : env_(env) {
+ async_id_ = node_env()->new_async_id();
+ trigger_async_id_ = node_env()->get_default_trigger_async_id();
+ resource_.Reset(node_env()->isolate(), resource_object);
+ lost_reference_ = false;
+ if (externally_managed_resource) {
+ resource_.SetWeak(
+ this, AsyncContext::WeakCallback, v8::WeakCallbackType::kParameter);
+ }
+
+ node::AsyncWrap::EmitAsyncInit(node_env(),
+ resource_object,
+ resource_name,
+ async_id_,
+ trigger_async_id_);
+ }
+
+ ~AsyncContext() {
+ resource_.Reset();
+ lost_reference_ = true;
+ node::AsyncWrap::EmitDestroy(node_env(), async_id_);
+ }
+
+ inline v8::MaybeLocal<v8::Value> MakeCallback(
+ v8::Local<v8::Object> recv,
+ const v8::Local<v8::Function> callback,
+ int argc,
+ v8::Local<v8::Value> argv[]) {
+ EnsureReference();
+ return node::InternalMakeCallback(node_env(),
+ resource(),
+ recv,
+ callback,
+ argc,
+ argv,
+ {async_id_, trigger_async_id_});
+ }
+
+ inline napi_callback_scope OpenCallbackScope() {
+ EnsureReference();
+ napi_callback_scope it =
+ reinterpret_cast<napi_callback_scope>(new CallbackScope(this));
+ env_->open_callback_scopes++;
+ return it;
+ }
+
+ inline void EnsureReference() {
+ if (lost_reference_) {
+ const v8::HandleScope handle_scope(node_env()->isolate());
+ resource_.Reset(node_env()->isolate(),
+ v8::Object::New(node_env()->isolate()));
+ lost_reference_ = false;
+ }
+ }
+
+ inline node::Environment* node_env() { return env_->node_env(); }
+ inline v8::Local<v8::Object> resource() {
+ return resource_.Get(node_env()->isolate());
+ }
+ inline node::async_context async_context() {
+ return {async_id_, trigger_async_id_};
+ }
+
+ static inline void CloseCallbackScope(node_napi_env env,
+ napi_callback_scope s) {
+ CallbackScope* callback_scope = reinterpret_cast<CallbackScope*>(s);
+ delete callback_scope;
+ env->open_callback_scopes--;
+ }
+
+ static void WeakCallback(const v8::WeakCallbackInfo<AsyncContext>& data) {
+ AsyncContext* async_context = data.GetParameter();
+ async_context->resource_.Reset();
+ async_context->lost_reference_ = true;
+ }
+
+ private:
+ class CallbackScope : public node::CallbackScope {
+ public:
+ explicit CallbackScope(AsyncContext* async_context)
+ : node::CallbackScope(async_context->node_env()->isolate(),
+ async_context->resource_.Get(
+ async_context->node_env()->isolate()),
+ async_context->async_context()) {}
+ };
+
+ node_napi_env env_;
+ double async_id_;
+ double trigger_async_id_;
+ v8::Global<v8::Object> resource_;
+ bool lost_reference_;
+};
+
} // end of anonymous namespace
} // end of namespace v8impl
@@ -627,7 +725,7 @@ NAPI_NO_RETURN void napi_fatal_error(const char* location,
}
napi_status napi_open_callback_scope(napi_env env,
- napi_value resource_object,
+ napi_value /** ignored */,
napi_async_context async_context_handle,
napi_callback_scope* result) {
// Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw
@@ -635,20 +733,11 @@ napi_status napi_open_callback_scope(napi_env env,
CHECK_ENV(env);
CHECK_ARG(env, result);
- v8::Local<v8::Context> context = env->context();
-
- node::async_context* node_async_context =
- reinterpret_cast<node::async_context*>(async_context_handle);
-
- v8::Local<v8::Object> resource;
- CHECK_TO_OBJECT(env, context, resource, resource_object);
+ v8impl::AsyncContext* node_async_context =
+ reinterpret_cast<v8impl::AsyncContext*>(async_context_handle);
- *result = v8impl::JsCallbackScopeFromV8CallbackScope(
- new node::CallbackScope(env->isolate,
- resource,
- *node_async_context));
+ *result = node_async_context->OpenCallbackScope();
- env->open_callback_scopes++;
return napi_clear_last_error(env);
}
@@ -661,8 +750,9 @@ napi_status napi_close_callback_scope(napi_env env, napi_callback_scope scope) {
return napi_callback_scope_mismatch;
}
- env->open_callback_scopes--;
- delete v8impl::V8CallbackScopeFromJsCallbackScope(scope);
+ v8impl::AsyncContext::CloseCallbackScope(reinterpret_cast<node_napi_env>(env),
+ scope);
+
return napi_clear_last_error(env);
}
@@ -678,20 +768,24 @@ napi_status napi_async_init(napi_env env,
v8::Local<v8::Context> context = env->context();
v8::Local<v8::Object> v8_resource;
+ bool externally_managed_resource;
if (async_resource != nullptr) {
CHECK_TO_OBJECT(env, context, v8_resource, async_resource);
+ externally_managed_resource = true;
} else {
v8_resource = v8::Object::New(isolate);
+ externally_managed_resource = false;
}
v8::Local<v8::String> v8_resource_name;
CHECK_TO_STRING(env, context, v8_resource_name, async_resource_name);
- // TODO(jasongin): Consider avoiding allocation here by using
- // a tagged pointer with 2×31 bit fields instead.
- node::async_context* async_context = new node::async_context();
+ auto async_context =
+ new v8impl::AsyncContext(reinterpret_cast<node_napi_env>(env),
+ v8_resource,
+ v8_resource_name,
+ externally_managed_resource);
- *async_context = node::EmitAsyncInit(isolate, v8_resource, v8_resource_name);
*result = reinterpret_cast<napi_async_context>(async_context);
return napi_clear_last_error(env);
@@ -702,11 +796,8 @@ napi_status napi_async_destroy(napi_env env,
CHECK_ENV(env);
CHECK_ARG(env, async_context);
- node::async_context* node_async_context =
- reinterpret_cast<node::async_context*>(async_context);
- node::EmitAsyncDestroy(
- reinterpret_cast<node_napi_env>(env)->node_env(),
- *node_async_context);
+ v8impl::AsyncContext* node_async_context =
+ reinterpret_cast<v8impl::AsyncContext*>(async_context);
delete node_async_context;
@@ -734,17 +825,25 @@ napi_status napi_make_callback(napi_env env,
v8::Local<v8::Function> v8func;
CHECK_TO_FUNCTION(env, v8func, func);
- node::async_context* node_async_context =
- reinterpret_cast<node::async_context*>(async_context);
- if (node_async_context == nullptr) {
- static node::async_context empty_context = { 0, 0 };
- node_async_context = &empty_context;
- }
+ v8::MaybeLocal<v8::Value> callback_result;
- v8::MaybeLocal<v8::Value> callback_result = node::MakeCallback(
- env->isolate, v8recv, v8func, argc,
- reinterpret_cast<v8::Local<v8::Value>*>(const_cast<napi_value*>(argv)),
- *node_async_context);
+ if (async_context == nullptr) {
+ callback_result = node::MakeCallback(
+ env->isolate,
+ v8recv,
+ v8func,
+ argc,
+ reinterpret_cast<v8::Local<v8::Value>*>(const_cast<napi_value*>(argv)),
+ {0, 0});
+ } else {
+ auto node_async_context =
+ reinterpret_cast<v8impl::AsyncContext*>(async_context);
+ callback_result = node_async_context->MakeCallback(
+ v8recv,
+ v8func,
+ argc,
+ reinterpret_cast<v8::Local<v8::Value>*>(const_cast<napi_value*>(argv)));
+ }
if (try_catch.HasCaught()) {
return napi_set_last_error(env, napi_pending_exception);
diff --git a/test/node-api/test_async_context/binding.c b/test/node-api/test_async_context/binding.c
new file mode 100644
index 00000000000000..749bb05d2503e7
--- /dev/null
+++ b/test/node-api/test_async_context/binding.c
@@ -0,0 +1,128 @@
+#include <node_api.h>
+#include <assert.h>
+#include "../../js-native-api/common.h"
+
+#define MAX_ARGUMENTS 10
+#define RESERVED_ARGS 3
+
+static napi_value MakeCallback(napi_env env, napi_callback_info info) {
+ size_t argc = MAX_ARGUMENTS;
+ size_t n;
+ napi_value args[MAX_ARGUMENTS];
+ // NOLINTNEXTLINE (readability/null_usage)
+ NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
+
+ NAPI_ASSERT(env, argc > 0, "Wrong number of arguments");
+
+ napi_value async_context_wrap = args[0];
+ napi_value recv = args[1];
+ napi_value func = args[2];
+
+ napi_value argv[MAX_ARGUMENTS - RESERVED_ARGS];
+ for (n = RESERVED_ARGS; n < argc; n += 1) {
+ argv[n - RESERVED_ARGS] = args[n];
+ }
+
+ napi_valuetype func_type;
+ NAPI_CALL(env, napi_typeof(env, func, &func_type));
+
+ napi_async_context context;
+ NAPI_CALL(env, napi_unwrap(env, async_context_wrap, &context));
+
+ napi_value result;
+ if (func_type == napi_function) {
+ NAPI_CALL(env, napi_make_callback(
+ env, context, recv, func, argc - RESERVED_ARGS, argv, &result));
+ } else {
+ NAPI_ASSERT(env, false, "Unexpected argument type");
+ }
+
+ 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);
+}
+
+#define CREATE_ASYNC_RESOURCE_ARGC 2
+
+static napi_value CreateAsyncResource(napi_env env, napi_callback_info info) {
+ napi_value async_context_wrap;
+ NAPI_CALL(env, napi_create_object(env, &async_context_wrap));
+
+ size_t argc = CREATE_ASYNC_RESOURCE_ARGC;
+ napi_value args[CREATE_ASYNC_RESOURCE_ARGC];
+ // NOLINTNEXTLINE (readability/null_usage)
+ NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
+
+ napi_value resource = args[0];
+ napi_value js_destroy_on_finalizer = args[1];
+ napi_valuetype resource_type;
+ NAPI_CALL(env, napi_typeof(env, resource, &resource_type));
+ if (resource_type != napi_object) {
+ resource = NULL;
+ }
+
+ napi_value resource_name;
+ NAPI_CALL(env, napi_create_string_utf8(
+ env, "test_async", NAPI_AUTO_LENGTH, &resource_name));
+
+ napi_async_context context;
+ NAPI_CALL(env, napi_async_init(env, resource, resource_name, &context));
+
+ bool destroy_on_finalizer = true;
+ if (argc == 2) {
+ NAPI_CALL(env, napi_get_value_bool(env, js_destroy_on_finalizer, &destroy_on_finalizer));
+ }
+ if (resource_type == napi_object && destroy_on_finalizer) {
+ NAPI_CALL(env, napi_add_finalizer(
+ env, resource, (void*)context, AsyncDestroyCb, NULL, NULL));
+ }
+ NAPI_CALL(env, napi_wrap(env, async_context_wrap, context, NULL, NULL, NULL));
+ return async_context_wrap;
+}
+
+#define DESTROY_ASYNC_RESOURCE_ARGC 1
+
+static napi_value DestroyAsyncResource(napi_env env, napi_callback_info info) {
+ size_t argc = DESTROY_ASYNC_RESOURCE_ARGC;
+ napi_value args[DESTROY_ASYNC_RESOURCE_ARGC];
+ // NOLINTNEXTLINE (readability/null_usage)
+ NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
+ NAPI_ASSERT(env, argc == 1, "Wrong number of arguments");
+
+ napi_value async_context_wrap = args[0];
+
+ napi_async_context async_context;
+ NAPI_CALL(env, napi_remove_wrap(env, async_context_wrap, &async_context));
+ NAPI_CALL(env, napi_async_destroy(env, async_context));
+
+ return async_context_wrap;
+}
+
+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));
+
+ NAPI_CALL(env, napi_create_function(
+ // NOLINTNEXTLINE (readability/null_usage)
+ env, NULL, NAPI_AUTO_LENGTH, DestroyAsyncResource, NULL, &fn));
+ NAPI_CALL(env, napi_set_named_property(
+ env, exports, "destroyAsyncResource", fn));
+
+ return exports;
+}
+
+NAPI_MODULE(NODE_GYP_MODULE_NAME, Init)
diff --git a/test/node-api/test_async_context/binding.gyp b/test/node-api/test_async_context/binding.gyp
new file mode 100644
index 00000000000000..23daf507916ff6
--- /dev/null
+++ b/test/node-api/test_async_context/binding.gyp
@@ -0,0 +1,9 @@
+{
+ 'targets': [
+ {
+ 'target_name': 'binding',
+ 'defines': [ 'V8_DEPRECATION_WARNINGS=1' ],
+ 'sources': [ 'binding.c' ]
+ }
+ ]
+}
diff --git a/test/node-api/test_async_context/test-gcable-callback.js b/test/node-api/test_async_context/test-gcable-callback.js
new file mode 100644
index 00000000000000..e66080bb6a3a90
--- /dev/null
+++ b/test/node-api/test_async_context/test-gcable-callback.js
@@ -0,0 +1,65 @@
+'use strict';
+// Flags: --gc-interval=100 --gc-global
+
+const common = require('../../common');
+const assert = require('assert');
+const async_hooks = require('async_hooks');
+const {
+ createAsyncResource,
+ destroyAsyncResource,
+ makeCallback,
+} = 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_async') {
+ hook_result.id = id;
+ hook_result.init_called = true;
+ }
+ },
+ destroy: (id) => {
+ if (id === hook_result.id) hook_result.destroy_called = true;
+ },
+});
+
+test_hook.enable();
+const asyncResource = createAsyncResource(
+ { foo: 'bar' },
+ /* destroy_on_finalizer */false
+);
+
+// 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, false);
+ makeCallback(asyncResource, process, () => {
+ const executionAsyncResource = async_hooks.executionAsyncResource();
+ // Assuming the executionAsyncResource was created for the absence of the
+ // initial `{ foo: 'bar' }`.
+ // This is the worst path of `napi_async_context` related API of
+ // recovering from the condition and not break the executionAsyncResource
+ // shape, although the executionAsyncResource might not be correct.
+ assert.strictEqual(typeof executionAsyncResource, 'object');
+ assert.strictEqual(executionAsyncResource.foo, undefined);
+ destroyAsyncResource(asyncResource);
+ setImmediate(() => {
+ assert.strictEqual(hook_result.destroy_called, true);
+ });
+ });
+});
diff --git a/test/node-api/test_make_callback/test-async-hooks-gcable.js b/test/node-api/test_async_context/test-gcable.js
similarity index 95%
rename from test/node-api/test_make_callback/test-async-hooks-gcable.js
rename to test/node-api/test_async_context/test-gcable.js
index a9b4d3d75d6040..288b7412267dcc 100644
--- a/test/node-api/test_make_callback/test-async-hooks-gcable.js
+++ b/test/node-api/test_async_context/test-gcable.js
@@ -17,7 +17,7 @@ const hook_result = {
const test_hook = async_hooks.createHook({
init: (id, type) => {
- if (type === 'test_gcable') {
+ if (type === 'test_async') {
hook_result.id = id;
hook_result.init_called = true;
}
@@ -28,7 +28,7 @@ const test_hook = async_hooks.createHook({
});
test_hook.enable();
-createAsyncResource();
+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
diff --git a/test/node-api/test_async_context/test.js b/test/node-api/test_async_context/test.js
new file mode 100644
index 00000000000000..2cf00b1ef7bb0e
--- /dev/null
+++ b/test/node-api/test_async_context/test.js
@@ -0,0 +1,63 @@
+'use strict';
+// Flags: --gc-interval=100 --gc-global
+
+const common = require('../../common');
+const assert = require('assert');
+const async_hooks = require('async_hooks');
+const {
+ makeCallback,
+ createAsyncResource,
+ destroyAsyncResource,
+} = require(`./build/${common.buildType}/binding`);
+
+const hook_result = {
+ id: null,
+ resource: null,
+ init_called: false,
+ destroy_called: false,
+};
+
+const test_hook = async_hooks.createHook({
+ init: (id, type, triggerAsyncId, resource) => {
+ if (type === 'test_async') {
+ hook_result.id = id;
+ hook_result.init_called = true;
+ hook_result.resource = resource;
+ }
+ },
+ destroy: (id) => {
+ if (id === hook_result.id) hook_result.destroy_called = true;
+ },
+});
+
+test_hook.enable();
+const resourceWrap = createAsyncResource(
+ /**
+ * set resource to NULL to generate a managed resource object
+ */
+ undefined
+);
+
+assert.strictEqual(hook_result.destroy_called, false);
+const recv = {};
+makeCallback(resourceWrap, recv, function callback() {
+ assert.strictEqual(hook_result.destroy_called, false);
+ assert.strictEqual(
+ hook_result.resource,
+ async_hooks.executionAsyncResource()
+ );
+ assert.strictEqual(this, recv);
+
+ setImmediate(() => {
+ assert.strictEqual(hook_result.destroy_called, false);
+ assert.notStrictEqual(
+ hook_result.resource,
+ async_hooks.executionAsyncResource()
+ );
+
+ destroyAsyncResource(resourceWrap);
+ setImmediate(() => {
+ assert.strictEqual(hook_result.destroy_called, true);
+ });
+ });
+});
diff --git a/test/node-api/test_make_callback/binding.c b/test/node-api/test_make_callback/binding.c
index 782cf0f6fb8f54..214e0a4e182385 100644
--- a/test/node-api/test_make_callback/binding.c
+++ b/test/node-api/test_make_callback/binding.c
@@ -3,6 +3,7 @@
#include "../../js-native-api/common.h"
#define MAX_ARGUMENTS 10
+#define RESERVED_ARGS 3
static napi_value MakeCallback(napi_env env, napi_callback_info info) {
size_t argc = MAX_ARGUMENTS;
@@ -13,12 +14,13 @@ static napi_value MakeCallback(napi_env env, napi_callback_info info) {
NAPI_ASSERT(env, argc > 0, "Wrong number of arguments");
- napi_value recv = args[0];
- napi_value func = args[1];
+ napi_value resource = args[0];
+ napi_value recv = args[1];
+ napi_value func = args[2];
- napi_value argv[MAX_ARGUMENTS - 2];
- for (n = 2; n < argc; n += 1) {
- argv[n - 2] = args[n];
+ napi_value argv[MAX_ARGUMENTS - RESERVED_ARGS];
+ for (n = RESERVED_ARGS; n < argc; n += 1) {
+ argv[n - RESERVED_ARGS] = args[n];
}
napi_valuetype func_type;
@@ -30,12 +32,12 @@ static napi_value MakeCallback(napi_env env, napi_callback_info info) {
env, "test", NAPI_AUTO_LENGTH, &resource_name));
napi_async_context context;
- NAPI_CALL(env, napi_async_init(env, func, resource_name, &context));
+ NAPI_CALL(env, napi_async_init(env, resource, resource_name, &context));
napi_value result;
if (func_type == napi_function) {
NAPI_CALL(env, napi_make_callback(
- env, context, recv, func, argc - 2, argv, &result));
+ env, context, recv, func, argc - RESERVED_ARGS, argv, &result));
} else {
NAPI_ASSERT(env, false, "Unexpected argument type");
}
@@ -45,30 +47,6 @@ 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;
@@ -76,11 +54,6 @@ 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.js b/test/node-api/test_make_callback/test-async-hooks.js
index 755a2389c68591..ca31ac4995ae5b 100644
--- a/test/node-api/test_make_callback/test-async-hooks.js
+++ b/test/node-api/test_make_callback/test-async-hooks.js
@@ -33,7 +33,16 @@ const test_hook = async_hooks.createHook({
});
test_hook.enable();
-makeCallback(process, function() {});
+
+/**
+ * Resource should be able to be arbitrary objects without special internal
+ * slots. Testing with plain object here.
+ */
+const resource = {};
+makeCallback(resource, process, function cb() {
+ assert.strictEqual(this, process);
+ assert.strictEqual(async_hooks.executionAsyncResource(), resource);
+});
assert.strictEqual(hook_result.init_called, true);
assert.strictEqual(hook_result.before_called, true);
diff --git a/test/node-api/test_make_callback/test.js b/test/node-api/test_make_callback/test.js
index dba550a492f0a3..d0b4b22500c9f6 100644
--- a/test/node-api/test_make_callback/test.js
+++ b/test/node-api/test_make_callback/test.js
@@ -13,20 +13,25 @@ function myMultiArgFunc(arg1, arg2, arg3) {
return 42;
}
-assert.strictEqual(makeCallback(process, common.mustCall(function() {
+/**
+ * Resource should be able to be arbitrary objects without special internal
+ * slots. Testing with plain object here.
+ */
+const resource = {};
+assert.strictEqual(makeCallback(resource, process, common.mustCall(function() {
assert.strictEqual(arguments.length, 0);
assert.strictEqual(this, process);
return 42;
})), 42);
-assert.strictEqual(makeCallback(process, common.mustCall(function(x) {
+assert.strictEqual(makeCallback(resource, process, common.mustCall(function(x) {
assert.strictEqual(arguments.length, 1);
assert.strictEqual(this, process);
assert.strictEqual(x, 1337);
return 42;
}), 1337), 42);
-assert.strictEqual(makeCallback(this,
+assert.strictEqual(makeCallback(resource, this,
common.mustCall(myMultiArgFunc), 1, 2, 3), 42);
// TODO(node-api): napi_make_callback needs to support
@@ -62,7 +67,7 @@ const target = vm.runInNewContext(`
return Object;
})
`);
-assert.notStrictEqual(makeCallback(process, target, Object), Object);
+assert.notStrictEqual(makeCallback(resource, process, target, Object), Object);
// Runs in inner context.
const forward = vm.runInNewContext(`
@@ -78,4 +83,4 @@ function endpoint($Object) {
return Object;
}
-assert.strictEqual(makeCallback(process, forward, endpoint), Object);
+assert.strictEqual(makeCallback(resource, process, forward, endpoint), Object);