Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit a5aa3dd

Browse filesBrowse files
Gabriel Schulhofaddaleax
Gabriel Schulhof
and
addaleax
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 8545fb2 commit a5aa3dd
Copy full SHA for a5aa3dd

File tree

Expand file treeCollapse file tree

5 files changed

+116
-48
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+116
-48
lines changed
Open diff view settings
Collapse file

‎doc/api/n-api.md‎

Copy file name to clipboardExpand all lines: doc/api/n-api.md
+58-7Lines changed: 58 additions & 7 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,15 @@ typedef struct {
626626
} napi_type_tag;
627627
```
628628

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

631640
#### napi_callback_info
@@ -754,6 +763,30 @@ typedef void (*napi_threadsafe_function_call_js)(napi_env env,
754763
Unless for reasons discussed in [Object Lifetime Management][], creating a
755764
handle and/or callback scope inside the function body is not necessary.
756765

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

759792
N-API uses both return values and JavaScript exceptions for error handling.
@@ -1583,22 +1616,33 @@ with `napi_add_env_cleanup_hook`, otherwise the process will abort.
15831616
#### napi_add_async_cleanup_hook
15841617
<!-- YAML
15851618
added: REPLACEME
1619+
changes:
1620+
- version: REPLACEME
1621+
pr-url: https://github.com/nodejs/node/pull/34819
1622+
description: Changed signature of the `hook` callback.
15861623
-->
15871624

15881625
> Stability: 1 - Experimental
15891626

15901627
```c
15911628
NAPI_EXTERN napi_status napi_add_async_cleanup_hook(
15921629
napi_env env,
1593-
void (*fun)(void* arg, void(* cb)(void*), void* cbarg),
1630+
napi_async_cleanup_hook hook,
15941631
void* arg,
15951632
napi_async_cleanup_hook_handle* remove_handle);
15961633
```
15971634

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

16031647
Otherwise, behavior generally matches that of [`napi_add_env_cleanup_hook`][].
16041648

@@ -1611,19 +1655,25 @@ is being torn down anyway.
16111655
#### napi_remove_async_cleanup_hook
16121656
<!-- YAML
16131657
added: REPLACEME
1658+
changes:
1659+
- version: REPLACEME
1660+
pr-url: https://github.com/nodejs/node/pull/34819
1661+
description: Removed `env` parameter.
16141662
-->
16151663

16161664
> Stability: 1 - Experimental
16171665

16181666
```c
16191667
NAPI_EXTERN napi_status napi_remove_async_cleanup_hook(
1620-
napi_env env,
16211668
napi_async_cleanup_hook_handle remove_handle);
16221669
```
16231670

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

16291679
## Module registration
@@ -5731,6 +5781,7 @@ This API may only be called from the main thread.
57315781
[`napi_add_async_cleanup_hook`]: #n_api_napi_add_async_cleanup_hook
57325782
[`napi_add_env_cleanup_hook`]: #n_api_napi_add_env_cleanup_hook
57335783
[`napi_add_finalizer`]: #n_api_napi_add_finalizer
5784+
[`napi_async_cleanup_hook`]: #n_api_napi_async_cleanup_hook
57345785
[`napi_async_complete_callback`]: #n_api_napi_async_complete_callback
57355786
[`napi_async_init`]: #n_api_napi_async_init
57365787
[`napi_callback`]: #n_api_napi_callback
Collapse file

‎src/node_api.cc‎

Copy file name to clipboardExpand all lines: src/node_api.cc
+45-18Lines 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) {
Collapse file

‎src/node_api.h‎

Copy file name to clipboardExpand all lines: src/node_api.h
+1-2Lines 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
Collapse file

‎src/node_api_types.h‎

Copy file name to clipboardExpand all lines: src/node_api_types.h
+2Lines 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_
Collapse file

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

Copy file name to clipboardExpand all lines: test/node-api/test_async_cleanup_hook/binding.c
+10-21Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.