FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

n-api: re-implement async env cleanup hooks · nodejs/node@3c32fe0 · GitHub

/ node Public

Commit 3c32fe0

Browse files
authored and
richardlau
committed
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: #34715 Signed-off-by: Gabriel Schulhof <gabriel.schulhof@intel.com> Co-authored-by: Anna Henningsen <github@addaleax.net> PR-URL: #34819 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Zeyu Yang <himself65@outlook.com>
1 parent b091681 commit 3c32fe0

5 files changed

Lines changed: 116 additions & 48 deletions

File tree

‎doc/api/n-api.md‎

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,15 @@ typedef struct {
623623
} napi_type_tag;
624624
```
625625

626+
#### napi_async_cleanup_hook_handle
627+
<!-- YAML
628+
added: REPLACEME
629+
-->
630+
631+
An opaque value returned by [`napi_add_async_cleanup_hook`][]. It must be passed
632+
to [`napi_remove_async_cleanup_hook`][] when the chain of asynchronous cleanup
633+
events completes.
634+
626635
### N-API callback types
627636

628637
#### napi_callback_info
@@ -751,6 +760,30 @@ typedef void (*napi_threadsafe_function_call_js)(napi_env env,
751760
Unless for reasons discussed in [Object Lifetime Management][], creating a
752761
handle and/or callback scope inside the function body is not necessary.
753762

763+
#### napi_async_cleanup_hook
764+
<!-- YAML
765+
added: REPLACEME
766+
-->
767+
768+
Function pointer used with [`napi_add_async_cleanup_hook`][]. It will be called
769+
when the environment is being torn down.
770+
771+
Callback functions must satisfy the following signature:
772+
773+
```c
774+
typedef void (*napi_async_cleanup_hook)(napi_async_cleanup_hook_handle handle,
775+
void* data);
776+
```
777+
778+
* `[in] handle`: The handle that must be passed to
779+
[`napi_remove_async_cleanup_hook`][] after completion of the asynchronous
780+
cleanup.
781+
* `[in] data`: The data that was passed to [`napi_add_async_cleanup_hook`][].
782+
783+
The body of the function should initiate the asynchronous cleanup actions at the
784+
end of which `handle` must be passed in a call to
785+
[`napi_remove_async_cleanup_hook`][].
786+
754787
## Error handling
755788

756789
N-API uses both return values and JavaScript exceptions for error handling.
@@ -1580,22 +1613,33 @@ with `napi_add_env_cleanup_hook`, otherwise the process will abort.
15801613
#### napi_add_async_cleanup_hook
15811614
<!-- YAML
15821615
added: v14.8.0
1616+
changes:
1617+
- version: REPLACEME
1618+
pr-url: https://github.com/nodejs/node/pull/34819
1619+
description: Changed signature of the `hook` callback.
15831620
-->
15841621

15851622
> Stability: 1 - Experimental
15861623

15871624
```c
15881625
NAPI_EXTERN napi_status napi_add_async_cleanup_hook(
15891626
napi_env env,
1590-
void (*fun)(void* arg, void(* cb)(void*), void* cbarg),
1627+
napi_async_cleanup_hook hook,
15911628
void* arg,
15921629
napi_async_cleanup_hook_handle* remove_handle);
15931630
```
15941631

1595-
Registers `fun` as a function to be run with the `arg` parameter once the
1596-
current Node.js environment exits. Unlike [`napi_add_env_cleanup_hook`][],
1597-
the hook is allowed to be asynchronous in this case, and must invoke the passed
1598-
`cb()` function with `cbarg` once all asynchronous activity is finished.
1632+
* `[in] env`: The environment that the API is invoked under.
1633+
* `[in] hook`: The function pointer to call at environment teardown.
1634+
* `[in] arg`: The pointer to pass to `hook` when it gets called.
1635+
* `[out] remove_handle`: Optional handle that refers to the asynchronous cleanup
1636+
hook.
1637+
1638+
Registers `hook`, which is a function of type [`napi_async_cleanup_hook`][], as
1639+
a function to be run with the `remove_handle` and `arg` parameters once the
1640+
current Node.js environment exits.
1641+
1642+
Unlike [`napi_add_env_cleanup_hook`][], the hook is allowed to be asynchronous.
15991643

16001644
Otherwise, behavior generally matches that of [`napi_add_env_cleanup_hook`][].
16011645

@@ -1608,19 +1652,25 @@ is being torn down anyway.
16081652
#### napi_remove_async_cleanup_hook
16091653
<!-- YAML
16101654
added: v14.8.0
1655+
changes:
1656+
- version: REPLACEME
1657+
pr-url: https://github.com/nodejs/node/pull/34819
1658+
description: Removed `env` parameter.
16111659
-->
16121660

16131661
> Stability: 1 - Experimental
16141662

16151663
```c
16161664
NAPI_EXTERN napi_status napi_remove_async_cleanup_hook(
1617-
napi_env env,
16181665
napi_async_cleanup_hook_handle remove_handle);
16191666
```
16201667

1668+
* `[in] remove_handle`: The handle to an asynchronous cleanup hook that was
1669+
created with [`napi_add_async_cleanup_hook`][].
1670+
16211671
Unregisters the cleanup hook corresponding to `remove_handle`. This will prevent
16221672
the hook from being executed, unless it has already started executing.
1623-
This must be called on any `napi_async_cleanup_hook_handle` value retrieved
1673+
This must be called on any `napi_async_cleanup_hook_handle` value obtained
16241674
from [`napi_add_async_cleanup_hook`][].
16251675

16261676
## Module registration
@@ -5757,6 +5807,7 @@ This API may only be called from the main thread.
57575807
[`napi_add_async_cleanup_hook`]: #n_api_napi_add_async_cleanup_hook
57585808
[`napi_add_env_cleanup_hook`]: #n_api_napi_add_env_cleanup_hook
57595809
[`napi_add_finalizer`]: #n_api_napi_add_finalizer
5810+
[`napi_async_cleanup_hook`]: #n_api_napi_async_cleanup_hook
57605811
[`napi_async_complete_callback`]: #n_api_napi_async_complete_callback
57615812
[`napi_async_init`]: #n_api_napi_async_init
57625813
[`napi_callback`]: #n_api_napi_callback

‎src/node_api.cc‎

Lines changed: 45 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -519,41 +519,68 @@ napi_status napi_remove_env_cleanup_hook(napi_env env,
519519
}
520520

521521
struct napi_async_cleanup_hook_handle__ {
522-
node::AsyncCleanupHookHandle handle;
522+
napi_async_cleanup_hook_handle__(napi_env env,
523+
napi_async_cleanup_hook user_hook,
524+
void* user_data):
525+
env_(env),
526+
user_hook_(user_hook),
527+
user_data_(user_data) {
528+
handle_ = node::AddEnvironmentCleanupHook(env->isolate, Hook, this);
529+
env->Ref();
530+
}
531+
532+
~napi_async_cleanup_hook_handle__() {
533+
node::RemoveEnvironmentCleanupHook(std::move(handle_));
534+
if (done_cb_ != nullptr)
535+
done_cb_(done_data_);
536+
537+
// Release the `env` handle asynchronously since it would be surprising if
538+
// a call to a N-API function would destroy `env` synchronously.
539+
static_cast<node_napi_env>(env_)->node_env()
540+
->SetImmediate([env = env_](node::Environment*) { env->Unref(); });
541+
}
542+
543+
static void Hook(void* data, void (*done_cb)(void*), void* done_data) {
544+
auto handle = static_cast<napi_async_cleanup_hook_handle__*>(data);
545+
handle->done_cb_ = done_cb;
546+
handle->done_data_ = done_data;
547+
handle->user_hook_(handle, handle->user_data_);
548+
}
549+
550+
node::AsyncCleanupHookHandle handle_;
551+
napi_env env_ = nullptr;
552+
napi_async_cleanup_hook user_hook_ = nullptr;
553+
void* user_data_ = nullptr;
554+
void (*done_cb_)(void*) = nullptr;
555+
void* done_data_ = nullptr;
523556
};
524557

525558
napi_status napi_add_async_cleanup_hook(
526559
napi_env env,
527-
void (*fun)(void* arg, void(* cb)(void*), void* cbarg),
560+
napi_async_cleanup_hook hook,
528561
void* arg,
529562
napi_async_cleanup_hook_handle* remove_handle) {
530563
CHECK_ENV(env);
531-
CHECK_ARG(env, fun);
564+
CHECK_ARG(env, hook);
532565

533-
auto handle = node::AddEnvironmentCleanupHook(env->isolate, fun, arg);
534-
if (remove_handle != nullptr) {
535-
*remove_handle = new napi_async_cleanup_hook_handle__ { std::move(handle) };
536-
env->Ref();
537-
}
566+
napi_async_cleanup_hook_handle__* handle =
567+
new napi_async_cleanup_hook_handle__(env, hook, arg);
568+
569+
if (remove_handle != nullptr)
570+
*remove_handle = handle;
538571

539572
return napi_clear_last_error(env);
540573
}
541574

542575
napi_status napi_remove_async_cleanup_hook(
543-
napi_env env,
544576
napi_async_cleanup_hook_handle remove_handle) {
545-
CHECK_ENV(env);
546-
CHECK_ARG(env, remove_handle);
547577

548-
node::RemoveEnvironmentCleanupHook(std::move(remove_handle->handle));
549-
delete remove_handle;
578+
if (remove_handle == nullptr)
579+
return napi_invalid_arg;
550580

551-
// Release the `env` handle asynchronously since it would be surprising if
552-
// a call to a N-API function would destroy `env` synchronously.
553-
static_cast<node_napi_env>(env)->node_env()
554-
->SetImmediate([env](node::Environment*) { env->Unref(); });
581+
delete remove_handle;
555582

556-
return napi_clear_last_error(env);
583+
return napi_ok;
557584
}
558585

559586
napi_status napi_fatal_exception(napi_env env, napi_value err) {

‎src/node_api.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,12 +254,11 @@ napi_ref_threadsafe_function(napi_env env, napi_threadsafe_function func);
254254

255255
NAPI_EXTERN napi_status napi_add_async_cleanup_hook(
256256
napi_env env,
257-
void (*fun)(void* arg, void(* cb)(void*), void* cbarg),
257+
napi_async_cleanup_hook hook,
258258
void* arg,
259259
napi_async_cleanup_hook_handle* remove_handle);
260260

261261
NAPI_EXTERN napi_status napi_remove_async_cleanup_hook(
262-
napi_env env,
263262
napi_async_cleanup_hook_handle remove_handle);
264263

265264
#endif // NAPI_EXPERIMENTAL

‎src/node_api_types.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ typedef struct {
4343

4444
#ifdef NAPI_EXPERIMENTAL
4545
typedef struct napi_async_cleanup_hook_handle__* napi_async_cleanup_hook_handle;
46+
typedef void (*napi_async_cleanup_hook)(napi_async_cleanup_hook_handle handle,
47+
void* data);
4648
#endif // NAPI_EXPERIMENTAL
4749

4850
#endif // SRC_NODE_API_TYPES_H_

‎test/node-api/test_async_cleanup_hook/binding.c‎

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,44 +5,34 @@
55
#include <stdlib.h>
66
#include "../../js-native-api/common.h"
77

8-
void MustNotCall(void* arg, void(*cb)(void*), void* cbarg) {
8+
static void MustNotCall(napi_async_cleanup_hook_handle hook, void* arg) {
99
assert(0);
1010
}
1111

1212
struct AsyncData {
1313
uv_async_t async;
1414
napi_env env;
1515
napi_async_cleanup_hook_handle handle;
16-
void (*done_cb)(void*);
17-
void* done_arg;
1816
};
1917

20-
struct AsyncData* CreateAsyncData() {
18+
static struct AsyncData* CreateAsyncData() {
2119
struct AsyncData* data = (struct AsyncData*) malloc(sizeof(struct AsyncData));
2220
data->handle = NULL;
2321
return data;
2422
}
2523

26-
void AfterCleanupHookTwo(uv_handle_t* handle) {
24+
static void AfterCleanupHookTwo(uv_handle_t* handle) {
2725
struct AsyncData* data = (struct AsyncData*) handle->data;
28-
data->done_cb(data->done_arg);
26+
napi_status status = napi_remove_async_cleanup_hook(data->handle);
27+
assert(status == napi_ok);
2928
free(data);
3029
}
3130

32-
void AfterCleanupHookOne(uv_async_t* async) {
33-
struct AsyncData* data = (struct AsyncData*) async->data;
34-
if (data->handle != NULL) {
35-
// Verify that removing the hook is okay between starting and finishing
36-
// of its execution.
37-
napi_status status =
38-
napi_remove_async_cleanup_hook(data->env, data->handle);
39-
assert(status == napi_ok);
40-
}
41-
31+
static void AfterCleanupHookOne(uv_async_t* async) {
4232
uv_close((uv_handle_t*) async, AfterCleanupHookTwo);
4333
}
4434

45-
void AsyncCleanupHook(void* arg, void(*cb)(void*), void* cbarg) {
35+
static void AsyncCleanupHook(napi_async_cleanup_hook_handle handle, void* arg) {
4636
struct AsyncData* data = (struct AsyncData*) arg;
4737
uv_loop_t* loop;
4838
napi_status status = napi_get_uv_event_loop(data->env, &loop);
@@ -51,12 +41,11 @@ void AsyncCleanupHook(void* arg, void(*cb)(void*), void* cbarg) {
5141
assert(err == 0);
5242

5343
data->async.data = data;
54-
data->done_cb = cb;
55-
data->done_arg = cbarg;
44+
data->handle = handle;
5645
uv_async_send(&data->async);
5746
}
5847

59-
napi_value Init(napi_env env, napi_value exports) {
48+
static napi_value Init(napi_env env, napi_value exports) {
6049
{
6150
struct AsyncData* data = CreateAsyncData();
6251
data->env = env;
@@ -73,7 +62,7 @@ napi_value Init(napi_env env, napi_value exports) {
7362
napi_async_cleanup_hook_handle must_not_call_handle;
7463
napi_add_async_cleanup_hook(
7564
env, MustNotCall, NULL, &must_not_call_handle);
76-
napi_remove_async_cleanup_hook(env, must_not_call_handle);
65+
napi_remove_async_cleanup_hook(must_not_call_handle);
7766
}
7867

7968
return NULL;

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL