n-api: re-implement async env cleanup hooks

* Avoid passing core `void*` and function pointers into add-on.
* Document `napi_async_cleanup_hook_handle` type.
* Render receipt of the handle mandatory from the point where the
  hook gets called. Removal of the handle remains mandatory.

Fixes: https://github.com/nodejs/node/issues/34715
Signed-off-by: Gabriel Schulhof <gabriel.schulhof@intel.com>
Co-authored-by: Anna Henningsen <github@addaleax.net>
PR-URL: https://github.com/nodejs/node/pull/34819
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
This commit is contained in:
Gabriel Schulhof 2020-08-17 10:13:00 -07:00
parent 3089f96ed0
commit 0848f56cb3
5 changed files with 116 additions and 48 deletions

View File

@ -623,6 +623,15 @@ typedef struct {
} napi_type_tag; } napi_type_tag;
``` ```
#### napi_async_cleanup_hook_handle
<!-- YAML
added: REPLACEME
-->
An opaque value returned by [`napi_add_async_cleanup_hook`][]. It must be passed
to [`napi_remove_async_cleanup_hook`][] when the chain of asynchronous cleanup
events completes.
### N-API callback types ### N-API callback types
#### napi_callback_info #### napi_callback_info
@ -751,6 +760,30 @@ typedef void (*napi_threadsafe_function_call_js)(napi_env env,
Unless for reasons discussed in [Object Lifetime Management][], creating a Unless for reasons discussed in [Object Lifetime Management][], creating a
handle and/or callback scope inside the function body is not necessary. handle and/or callback scope inside the function body is not necessary.
#### napi_async_cleanup_hook
<!-- YAML
added: REPLACEME
-->
Function pointer used with [`napi_add_async_cleanup_hook`][]. It will be called
when the environment is being torn down.
Callback functions must satisfy the following signature:
```c
typedef void (*napi_async_cleanup_hook)(napi_async_cleanup_hook_handle handle,
void* data);
```
* `[in] handle`: The handle that must be passed to
[`napi_remove_async_cleanup_hook`][] after completion of the asynchronous
cleanup.
* `[in] data`: The data that was passed to [`napi_add_async_cleanup_hook`][].
The body of the function should initiate the asynchronous cleanup actions at the
end of which `handle` must be passed in a call to
[`napi_remove_async_cleanup_hook`][].
## Error handling ## Error handling
N-API uses both return values and JavaScript exceptions for error handling. N-API uses both return values and JavaScript exceptions for error handling.
@ -1580,6 +1613,10 @@ with `napi_add_env_cleanup_hook`, otherwise the process will abort.
#### napi_add_async_cleanup_hook #### napi_add_async_cleanup_hook
<!-- YAML <!-- YAML
added: v14.8.0 added: v14.8.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/34819
description: Changed signature of the `hook` callback.
--> -->
> Stability: 1 - Experimental > Stability: 1 - Experimental
@ -1587,15 +1624,22 @@ added: v14.8.0
```c ```c
NAPI_EXTERN napi_status napi_add_async_cleanup_hook( NAPI_EXTERN napi_status napi_add_async_cleanup_hook(
napi_env env, napi_env env,
void (*fun)(void* arg, void(* cb)(void*), void* cbarg), napi_async_cleanup_hook hook,
void* arg, void* arg,
napi_async_cleanup_hook_handle* remove_handle); napi_async_cleanup_hook_handle* remove_handle);
``` ```
Registers `fun` as a function to be run with the `arg` parameter once the * `[in] env`: The environment that the API is invoked under.
current Node.js environment exits. Unlike [`napi_add_env_cleanup_hook`][], * `[in] hook`: The function pointer to call at environment teardown.
the hook is allowed to be asynchronous in this case, and must invoke the passed * `[in] arg`: The pointer to pass to `hook` when it gets called.
`cb()` function with `cbarg` once all asynchronous activity is finished. * `[out] remove_handle`: Optional handle that refers to the asynchronous cleanup
hook.
Registers `hook`, which is a function of type [`napi_async_cleanup_hook`][], as
a function to be run with the `remove_handle` and `arg` parameters once the
current Node.js environment exits.
Unlike [`napi_add_env_cleanup_hook`][], the hook is allowed to be asynchronous.
Otherwise, behavior generally matches that of [`napi_add_env_cleanup_hook`][]. Otherwise, behavior generally matches that of [`napi_add_env_cleanup_hook`][].
@ -1608,19 +1652,25 @@ is being torn down anyway.
#### napi_remove_async_cleanup_hook #### napi_remove_async_cleanup_hook
<!-- YAML <!-- YAML
added: v14.8.0 added: v14.8.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/34819
description: Removed `env` parameter.
--> -->
> Stability: 1 - Experimental > Stability: 1 - Experimental
```c ```c
NAPI_EXTERN napi_status napi_remove_async_cleanup_hook( NAPI_EXTERN napi_status napi_remove_async_cleanup_hook(
napi_env env,
napi_async_cleanup_hook_handle remove_handle); napi_async_cleanup_hook_handle remove_handle);
``` ```
* `[in] remove_handle`: The handle to an asynchronous cleanup hook that was
created with [`napi_add_async_cleanup_hook`][].
Unregisters the cleanup hook corresponding to `remove_handle`. This will prevent Unregisters the cleanup hook corresponding to `remove_handle`. This will prevent
the hook from being executed, unless it has already started executing. the hook from being executed, unless it has already started executing.
This must be called on any `napi_async_cleanup_hook_handle` value retrieved This must be called on any `napi_async_cleanup_hook_handle` value obtained
from [`napi_add_async_cleanup_hook`][]. from [`napi_add_async_cleanup_hook`][].
## Module registration ## Module registration
@ -5763,6 +5813,7 @@ This API may only be called from the main thread.
[`napi_add_async_cleanup_hook`]: #n_api_napi_add_async_cleanup_hook [`napi_add_async_cleanup_hook`]: #n_api_napi_add_async_cleanup_hook
[`napi_add_env_cleanup_hook`]: #n_api_napi_add_env_cleanup_hook [`napi_add_env_cleanup_hook`]: #n_api_napi_add_env_cleanup_hook
[`napi_add_finalizer`]: #n_api_napi_add_finalizer [`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_complete_callback`]: #n_api_napi_async_complete_callback
[`napi_async_init`]: #n_api_napi_async_init [`napi_async_init`]: #n_api_napi_async_init
[`napi_callback`]: #n_api_napi_callback [`napi_callback`]: #n_api_napi_callback

View File

@ -519,41 +519,68 @@ napi_status napi_remove_env_cleanup_hook(napi_env env,
} }
struct napi_async_cleanup_hook_handle__ { struct napi_async_cleanup_hook_handle__ {
node::AsyncCleanupHookHandle handle; napi_async_cleanup_hook_handle__(napi_env env,
napi_async_cleanup_hook user_hook,
void* user_data):
env_(env),
user_hook_(user_hook),
user_data_(user_data) {
handle_ = node::AddEnvironmentCleanupHook(env->isolate, Hook, this);
env->Ref();
}
~napi_async_cleanup_hook_handle__() {
node::RemoveEnvironmentCleanupHook(std::move(handle_));
if (done_cb_ != nullptr)
done_cb_(done_data_);
// Release the `env` handle asynchronously since it would be surprising if
// a call to a N-API function would destroy `env` synchronously.
static_cast<node_napi_env>(env_)->node_env()
->SetImmediate([env = env_](node::Environment*) { env->Unref(); });
}
static void Hook(void* data, void (*done_cb)(void*), void* done_data) {
auto handle = static_cast<napi_async_cleanup_hook_handle__*>(data);
handle->done_cb_ = done_cb;
handle->done_data_ = done_data;
handle->user_hook_(handle, handle->user_data_);
}
node::AsyncCleanupHookHandle handle_;
napi_env env_ = nullptr;
napi_async_cleanup_hook user_hook_ = nullptr;
void* user_data_ = nullptr;
void (*done_cb_)(void*) = nullptr;
void* done_data_ = nullptr;
}; };
napi_status napi_add_async_cleanup_hook( napi_status napi_add_async_cleanup_hook(
napi_env env, napi_env env,
void (*fun)(void* arg, void(* cb)(void*), void* cbarg), napi_async_cleanup_hook hook,
void* arg, void* arg,
napi_async_cleanup_hook_handle* remove_handle) { napi_async_cleanup_hook_handle* remove_handle) {
CHECK_ENV(env); CHECK_ENV(env);
CHECK_ARG(env, fun); CHECK_ARG(env, hook);
auto handle = node::AddEnvironmentCleanupHook(env->isolate, fun, arg); napi_async_cleanup_hook_handle__* handle =
if (remove_handle != nullptr) { new napi_async_cleanup_hook_handle__(env, hook, arg);
*remove_handle = new napi_async_cleanup_hook_handle__ { std::move(handle) };
env->Ref(); if (remove_handle != nullptr)
} *remove_handle = handle;
return napi_clear_last_error(env); return napi_clear_last_error(env);
} }
napi_status napi_remove_async_cleanup_hook( napi_status napi_remove_async_cleanup_hook(
napi_env env,
napi_async_cleanup_hook_handle remove_handle) { napi_async_cleanup_hook_handle remove_handle) {
CHECK_ENV(env);
CHECK_ARG(env, remove_handle);
node::RemoveEnvironmentCleanupHook(std::move(remove_handle->handle)); if (remove_handle == nullptr)
return napi_invalid_arg;
delete remove_handle; delete remove_handle;
// Release the `env` handle asynchronously since it would be surprising if return napi_ok;
// a call to a N-API function would destroy `env` synchronously.
static_cast<node_napi_env>(env)->node_env()
->SetImmediate([env](node::Environment*) { env->Unref(); });
return napi_clear_last_error(env);
} }
napi_status napi_fatal_exception(napi_env env, napi_value err) { napi_status napi_fatal_exception(napi_env env, napi_value err) {

View File

@ -254,12 +254,11 @@ napi_ref_threadsafe_function(napi_env env, napi_threadsafe_function func);
NAPI_EXTERN napi_status napi_add_async_cleanup_hook( NAPI_EXTERN napi_status napi_add_async_cleanup_hook(
napi_env env, napi_env env,
void (*fun)(void* arg, void(* cb)(void*), void* cbarg), napi_async_cleanup_hook hook,
void* arg, void* arg,
napi_async_cleanup_hook_handle* remove_handle); napi_async_cleanup_hook_handle* remove_handle);
NAPI_EXTERN napi_status napi_remove_async_cleanup_hook( NAPI_EXTERN napi_status napi_remove_async_cleanup_hook(
napi_env env,
napi_async_cleanup_hook_handle remove_handle); napi_async_cleanup_hook_handle remove_handle);
#endif // NAPI_EXPERIMENTAL #endif // NAPI_EXPERIMENTAL

View File

@ -43,6 +43,8 @@ typedef struct {
#ifdef NAPI_EXPERIMENTAL #ifdef NAPI_EXPERIMENTAL
typedef struct napi_async_cleanup_hook_handle__* napi_async_cleanup_hook_handle; typedef struct napi_async_cleanup_hook_handle__* napi_async_cleanup_hook_handle;
typedef void (*napi_async_cleanup_hook)(napi_async_cleanup_hook_handle handle,
void* data);
#endif // NAPI_EXPERIMENTAL #endif // NAPI_EXPERIMENTAL
#endif // SRC_NODE_API_TYPES_H_ #endif // SRC_NODE_API_TYPES_H_

View File

@ -5,7 +5,7 @@
#include <stdlib.h> #include <stdlib.h>
#include "../../js-native-api/common.h" #include "../../js-native-api/common.h"
void MustNotCall(void* arg, void(*cb)(void*), void* cbarg) { static void MustNotCall(napi_async_cleanup_hook_handle hook, void* arg) {
assert(0); assert(0);
} }
@ -13,36 +13,26 @@ struct AsyncData {
uv_async_t async; uv_async_t async;
napi_env env; napi_env env;
napi_async_cleanup_hook_handle handle; napi_async_cleanup_hook_handle handle;
void (*done_cb)(void*);
void* done_arg;
}; };
struct AsyncData* CreateAsyncData() { static struct AsyncData* CreateAsyncData() {
struct AsyncData* data = (struct AsyncData*) malloc(sizeof(struct AsyncData)); struct AsyncData* data = (struct AsyncData*) malloc(sizeof(struct AsyncData));
data->handle = NULL; data->handle = NULL;
return data; return data;
} }
void AfterCleanupHookTwo(uv_handle_t* handle) { static void AfterCleanupHookTwo(uv_handle_t* handle) {
struct AsyncData* data = (struct AsyncData*) handle->data; struct AsyncData* data = (struct AsyncData*) handle->data;
data->done_cb(data->done_arg); napi_status status = napi_remove_async_cleanup_hook(data->handle);
assert(status == napi_ok);
free(data); free(data);
} }
void AfterCleanupHookOne(uv_async_t* async) { static void AfterCleanupHookOne(uv_async_t* async) {
struct AsyncData* data = (struct AsyncData*) async->data;
if (data->handle != NULL) {
// Verify that removing the hook is okay between starting and finishing
// of its execution.
napi_status status =
napi_remove_async_cleanup_hook(data->env, data->handle);
assert(status == napi_ok);
}
uv_close((uv_handle_t*) async, AfterCleanupHookTwo); uv_close((uv_handle_t*) async, AfterCleanupHookTwo);
} }
void AsyncCleanupHook(void* arg, void(*cb)(void*), void* cbarg) { static void AsyncCleanupHook(napi_async_cleanup_hook_handle handle, void* arg) {
struct AsyncData* data = (struct AsyncData*) arg; struct AsyncData* data = (struct AsyncData*) arg;
uv_loop_t* loop; uv_loop_t* loop;
napi_status status = napi_get_uv_event_loop(data->env, &loop); napi_status status = napi_get_uv_event_loop(data->env, &loop);
@ -51,12 +41,11 @@ void AsyncCleanupHook(void* arg, void(*cb)(void*), void* cbarg) {
assert(err == 0); assert(err == 0);
data->async.data = data; data->async.data = data;
data->done_cb = cb; data->handle = handle;
data->done_arg = cbarg;
uv_async_send(&data->async); uv_async_send(&data->async);
} }
napi_value Init(napi_env env, napi_value exports) { static napi_value Init(napi_env env, napi_value exports) {
{ {
struct AsyncData* data = CreateAsyncData(); struct AsyncData* data = CreateAsyncData();
data->env = env; data->env = env;
@ -73,7 +62,7 @@ napi_value Init(napi_env env, napi_value exports) {
napi_async_cleanup_hook_handle must_not_call_handle; napi_async_cleanup_hook_handle must_not_call_handle;
napi_add_async_cleanup_hook( napi_add_async_cleanup_hook(
env, MustNotCall, NULL, &must_not_call_handle); env, MustNotCall, NULL, &must_not_call_handle);
napi_remove_async_cleanup_hook(env, must_not_call_handle); napi_remove_async_cleanup_hook(must_not_call_handle);
} }
return NULL; return NULL;