Skip to content

Commit

Permalink
n-api: re-implement async env cleanup hooks
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
Co-authored-by: Anna Henningsen <[email protected]>
PR-URL: #34819
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
  • Loading branch information
2 people authored and richardlau committed Sep 1, 2020
1 parent b091681 commit 3c32fe0
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 48 deletions.
65 changes: 58 additions & 7 deletions doc/api/n-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -1580,22 +1613,33 @@ 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

```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`][].

Expand All @@ -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
Expand Down Expand Up @@ -5757,6 +5807,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
Expand Down
63 changes: 45 additions & 18 deletions src/node_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 1 addition & 2 deletions src/node_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/node_api_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_
31 changes: 10 additions & 21 deletions test/node-api/test_async_cleanup_hook/binding.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,44 +5,34 @@
#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);
}

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);
Expand All @@ -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;
Expand All @@ -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;
Expand Down

0 comments on commit 3c32fe0

Please sign in to comment.