diff options
author | Gabriel Schulhof <gabriel.schulhof@intel.com> | 2020-08-17 20:13:00 +0300 |
---|---|---|
committer | Gabriel Schulhof <gabriel.schulhof@intel.com> | 2020-08-27 18:18:37 +0300 |
commit | 0848f56cb39432090cdb99af9b8541fbc1a2849c (patch) | |
tree | 12f0089eba17afcaa39a03de6482dd1a221e3b2e | |
parent | 3089f96ed01653aa4d5e4bba5a5db473ffe788a0 (diff) |
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>
-rw-r--r-- | doc/api/n-api.md | 65 | ||||
-rw-r--r-- | src/node_api.cc | 63 | ||||
-rw-r--r-- | src/node_api.h | 3 | ||||
-rw-r--r-- | src/node_api_types.h | 2 | ||||
-rw-r--r-- | test/node-api/test_async_cleanup_hook/binding.c | 31 |
5 files changed, 116 insertions, 48 deletions
diff --git a/doc/api/n-api.md b/doc/api/n-api.md index dd4f1329b4e..91341a2f6c5 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -623,6 +623,15 @@ typedef struct { } 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 #### 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 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 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 <!-- YAML 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 @@ -1587,15 +1624,22 @@ added: v14.8.0 ```c NAPI_EXTERN napi_status napi_add_async_cleanup_hook( napi_env env, - void (*fun)(void* arg, void(* cb)(void*), void* cbarg), + napi_async_cleanup_hook hook, void* arg, napi_async_cleanup_hook_handle* remove_handle); ``` -Registers `fun` as a function to be run with the `arg` parameter once the -current Node.js environment exits. Unlike [`napi_add_env_cleanup_hook`][], -the hook is allowed to be asynchronous in this case, and must invoke the passed -`cb()` function with `cbarg` once all asynchronous activity is finished. +* `[in] env`: The environment that the API is invoked under. +* `[in] hook`: The function pointer to call at environment teardown. +* `[in] arg`: The pointer to pass to `hook` when it gets called. +* `[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`][]. @@ -1608,19 +1652,25 @@ is being torn down anyway. #### napi_remove_async_cleanup_hook <!-- YAML added: v14.8.0 +changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/34819 + description: Removed `env` parameter. --> > Stability: 1 - Experimental ```c NAPI_EXTERN napi_status napi_remove_async_cleanup_hook( - napi_env env, 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 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`][]. ## 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_env_cleanup_hook`]: #n_api_napi_add_env_cleanup_hook [`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_init`]: #n_api_napi_async_init [`napi_callback`]: #n_api_napi_callback diff --git a/src/node_api.cc b/src/node_api.cc index 4fbab771d58..93488146d56 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -519,41 +519,68 @@ napi_status napi_remove_env_cleanup_hook(napi_env env, } 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_env env, - void (*fun)(void* arg, void(* cb)(void*), void* cbarg), + napi_async_cleanup_hook hook, void* arg, napi_async_cleanup_hook_handle* remove_handle) { CHECK_ENV(env); - CHECK_ARG(env, fun); + CHECK_ARG(env, hook); - auto handle = node::AddEnvironmentCleanupHook(env->isolate, fun, arg); - if (remove_handle != nullptr) { - *remove_handle = new napi_async_cleanup_hook_handle__ { std::move(handle) }; - env->Ref(); - } + napi_async_cleanup_hook_handle__* handle = + new napi_async_cleanup_hook_handle__(env, hook, arg); + + if (remove_handle != nullptr) + *remove_handle = handle; return napi_clear_last_error(env); } napi_status napi_remove_async_cleanup_hook( - napi_env env, napi_async_cleanup_hook_handle remove_handle) { - CHECK_ENV(env); - CHECK_ARG(env, remove_handle); - node::RemoveEnvironmentCleanupHook(std::move(remove_handle->handle)); - delete remove_handle; + if (remove_handle == nullptr) + return napi_invalid_arg; - // 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](node::Environment*) { env->Unref(); }); + delete remove_handle; - return napi_clear_last_error(env); + return napi_ok; } napi_status napi_fatal_exception(napi_env env, napi_value err) { diff --git a/src/node_api.h b/src/node_api.h index 4f3eb8f2caa..577a1dcd949 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -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_env env, - void (*fun)(void* arg, void(* cb)(void*), void* cbarg), + napi_async_cleanup_hook hook, void* arg, napi_async_cleanup_hook_handle* remove_handle); NAPI_EXTERN napi_status napi_remove_async_cleanup_hook( - napi_env env, napi_async_cleanup_hook_handle remove_handle); #endif // NAPI_EXPERIMENTAL diff --git a/src/node_api_types.h b/src/node_api_types.h index b8711d3eddc..0e400e9676d 100644 --- a/src/node_api_types.h +++ b/src/node_api_types.h @@ -43,6 +43,8 @@ typedef struct { #ifdef NAPI_EXPERIMENTAL 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 // SRC_NODE_API_TYPES_H_ diff --git a/test/node-api/test_async_cleanup_hook/binding.c b/test/node-api/test_async_cleanup_hook/binding.c index f0c9cd97a26..7bbde56bb0e 100644 --- a/test/node-api/test_async_cleanup_hook/binding.c +++ b/test/node-api/test_async_cleanup_hook/binding.c @@ -5,7 +5,7 @@ #include <stdlib.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); } @@ -13,36 +13,26 @@ struct AsyncData { uv_async_t async; napi_env env; 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)); data->handle = NULL; return data; } -void AfterCleanupHookTwo(uv_handle_t* handle) { +static void AfterCleanupHookTwo(uv_handle_t* handle) { 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); } -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); - } - +static void AfterCleanupHookOne(uv_async_t* async) { 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; uv_loop_t* 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); data->async.data = data; - data->done_cb = cb; - data->done_arg = cbarg; + data->handle = handle; 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(); 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_add_async_cleanup_hook( 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; |