From 28c8a962d84b397a36f5fff1cae1624956e83572 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Mon, 17 Aug 2020 10:13:00 -0700 Subject: [PATCH 1/6] 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. Removal of the handle remains mandatory. Fixes: https://github.com/nodejs/node/issues/34715 Signed-off-by: Gabriel Schulhof --- doc/api/n-api.md | 64 +++++++++++++++---- src/node_api.cc | 63 ++++++++++++------ src/node_api.h | 3 +- src/node_api_types.h | 2 + .../test_async_cleanup_hook/binding.c | 36 +++-------- 5 files changed, 108 insertions(+), 60 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index ea975cdbffdf02..7ca75e68e1a26b 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -619,6 +619,15 @@ typedef struct { } napi_type_tag; ``` +#### napi_async_cleanup_hook_handle + + +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 @@ -747,6 +756,28 @@ 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 + + +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 obtained from [`napi_add_async_cleanup_hook`][]. +* `[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. @@ -1583,23 +1614,30 @@ 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`: The handle that refers to the asynchronous cleanup +hook. This handle must be passed to [`napi_remove_async_cleanup_hook`][] even if +`hook` gets called. + +Registers `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, and +must pass `remove_handle` in a call to [`napi_remove_env_cleanup_hook`][] once +all asynchronous activity is finished. Otherwise, behavior generally matches that of [`napi_add_env_cleanup_hook`][]. -If `remove_handle` is not `NULL`, an opaque value will be stored in it -that must later be passed to [`napi_remove_async_cleanup_hook`][], -regardless of whether the hook has already been invoked. -Typically, that happens when the resource for which this hook was added -is being torn down anyway. +An opaque value will be stored in `remove_handle` that must later be passed to +[`napi_remove_async_cleanup_hook`][], regardless of whether the hook has already +been invoked. Typically, that happens when the resource for which this hook was +added is being torn down anyway. #### napi_remove_async_cleanup_hook > Stability: 1 - Experimental @@ -1644,6 +1648,10 @@ added is being torn down anyway. #### napi_remove_async_cleanup_hook > Stability: 1 - Experimental From 5b694e443c562a0d4230364302244aba6df3087b Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Wed, 26 Aug 2020 15:48:51 -0700 Subject: [PATCH 6/6] storing the handle at creation is optional again --- doc/api/n-api.md | 22 +++++++++---------- src/node_api.cc | 8 +++++-- .../test_async_cleanup_hook/binding.c | 7 ++++++ 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 86226d3ab33849..0e17a9c17a39ff 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -771,7 +771,9 @@ typedef void (*napi_async_cleanup_hook)(napi_async_cleanup_hook_handle handle, void* data); ``` -* `[in] handle`: The handle obtained from [`napi_add_async_cleanup_hook`][]. +* `[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 @@ -1626,24 +1628,22 @@ NAPI_EXTERN napi_status napi_add_async_cleanup_hook( * `[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`: The handle that refers to the asynchronous cleanup -hook. This handle must be passed to [`napi_remove_async_cleanup_hook`][] even if -`hook` 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, and must pass `remove_handle` in a call to -[`napi_remove_env_cleanup_hook`][] once all asynchronous activity is finished. +Unlike [`napi_add_env_cleanup_hook`][], the hook is allowed to be asynchronous. Otherwise, behavior generally matches that of [`napi_add_env_cleanup_hook`][]. -An opaque value will be stored in `remove_handle` that must later be passed to -[`napi_remove_async_cleanup_hook`][], regardless of whether the hook has already -been invoked. Typically, that happens when the resource for which this hook was -added is being torn down anyway. +If `remove_handle` is not `NULL`, an opaque value will be stored in it +that must later be passed to [`napi_remove_async_cleanup_hook`][], +regardless of whether the hook has already been invoked. +Typically, that happens when the resource for which this hook was added +is being torn down anyway. #### napi_remove_async_cleanup_hook