Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

n-api,src: provide asynchronous cleanup hooks #34572

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions doc/api/addons.md
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,12 @@ NODE_MODULE_INIT(/* exports, module, context */) {
```

#### Worker support
<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/34572
description: Cleanup hooks may now be asynchronous.
-->

In order to be loaded from multiple Node.js environments,
such as a main thread and a Worker thread, an add-on needs to either:
Expand All @@ -254,6 +260,11 @@ down. If necessary, such hooks can be removed using
`RemoveEnvironmentCleanupHook()` before they are run, which has the same
signature. Callbacks are run in last-in first-out order.

If necessary, there is an additional pair of `AddEnvironmentCleanupHook()`
and `RemoveEnvironmentCleanupHook()` overloads, where the cleanup hook takes a
callback function. This can be used for shutting down asynchronous resources,
for example any libuv handles registered by the addon.

The following `addon.cc` uses `AddEnvironmentCleanupHook`:

```cpp
Expand Down
53 changes: 52 additions & 1 deletion doc/api/n-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1531,10 +1531,12 @@ and will lead the process to abort.
The hooks will be called in reverse order, i.e. the most recently added one
will be called first.

Removing this hook can be done by using `napi_remove_env_cleanup_hook`.
Removing this hook can be done by using [`napi_remove_env_cleanup_hook`][].
Typically, that happens when the resource for which this hook was added
is being torn down anyway.

For asynchronous cleanup, [`napi_add_async_cleanup_hook`][] is available.

#### napi_remove_env_cleanup_hook
<!-- YAML
added: v10.2.0
Expand All @@ -1554,6 +1556,52 @@ need to be exact matches.
The function must have originally been registered
with `napi_add_env_cleanup_hook`, otherwise the process will abort.

#### napi_add_async_cleanup_hook
<!-- YAML
added: REPLACEME
-->

> 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),
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.

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.

#### napi_remove_async_cleanup_hook
<!-- YAML
added: REPLACEME
-->

> Stability: 1 - Experimental

```c
NAPI_EXTERN napi_status napi_remove_async_cleanup_hook(
napi_env env,
napi_async_cleanup_hook_handle remove_handle);
```

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

## Module registration
N-API modules are registered in a manner similar to other modules
except that instead of using the `NODE_MODULE` macro the following
Expand Down Expand Up @@ -5495,6 +5543,7 @@ This API may only be called from the main thread.
[`Worker`]: worker_threads.html#worker_threads_class_worker
[`global`]: globals.html#globals_global
[`init` hooks]: async_hooks.html#async_hooks_init_asyncid_type_triggerasyncid_resource
[`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_complete_callback`]: #n_api_napi_async_complete_callback
Expand Down Expand Up @@ -5534,6 +5583,8 @@ This API may only be called from the main thread.
[`napi_queue_async_work`]: #n_api_napi_queue_async_work
[`napi_reference_ref`]: #n_api_napi_reference_ref
[`napi_reference_unref`]: #n_api_napi_reference_unref
[`napi_remove_async_cleanup_hook`]: #n_api_napi_remove_async_cleanup_hook
[`napi_remove_env_cleanup_hook`]: #n_api_napi_remove_env_cleanup_hook
[`napi_set_instance_data`]: #n_api_napi_set_instance_data
[`napi_set_property`]: #n_api_napi_set_property
[`napi_threadsafe_function_call_js`]: #n_api_napi_threadsafe_function_call_js
Expand Down
68 changes: 66 additions & 2 deletions src/api/hooks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,22 +73,86 @@ int EmitExit(Environment* env) {
.ToChecked();
}

typedef void (*CleanupHook)(void* arg);
typedef void (*AsyncCleanupHook)(void* arg, void(*)(void*), void*);

struct AsyncCleanupHookInfo final {
Environment* env;
AsyncCleanupHook fun;
void* arg;
bool started = false;
// Use a self-reference to make sure the storage is kept alive while the
// cleanup hook is registered but not yet finished.
std::shared_ptr<AsyncCleanupHookInfo> self;
};

// Opaque type that is basically an alias for `shared_ptr<AsyncCleanupHookInfo>`
// (but not publicly so for easier ABI/API changes). In particular,
// std::shared_ptr does not generally maintain a consistent ABI even on a
// specific platform.
struct ACHHandle final {
std::shared_ptr<AsyncCleanupHookInfo> info;
};
// This is implemented as an operator on a struct because otherwise you can't
// default-initialize AsyncCleanupHookHandle, because in C++ for a
// std::unique_ptr to be default-initializable the deleter type also needs
// to be default-initializable; in particular, function types don't satisfy
// this.
void DeleteACHHandle::operator ()(ACHHandle* handle) const { delete handle; }

void AddEnvironmentCleanupHook(Isolate* isolate,
void (*fun)(void* arg),
CleanupHook fun,
void* arg) {
Environment* env = Environment::GetCurrent(isolate);
CHECK_NOT_NULL(env);
env->AddCleanupHook(fun, arg);
}

void RemoveEnvironmentCleanupHook(Isolate* isolate,
void (*fun)(void* arg),
CleanupHook fun,
void* arg) {
Environment* env = Environment::GetCurrent(isolate);
CHECK_NOT_NULL(env);
env->RemoveCleanupHook(fun, arg);
}

static void FinishAsyncCleanupHook(void* arg) {
AsyncCleanupHookInfo* info = static_cast<AsyncCleanupHookInfo*>(arg);
std::shared_ptr<AsyncCleanupHookInfo> keep_alive = info->self;

info->env->DecreaseWaitingRequestCounter();
info->self.reset();
}

static void RunAsyncCleanupHook(void* arg) {
AsyncCleanupHookInfo* info = static_cast<AsyncCleanupHookInfo*>(arg);
info->env->IncreaseWaitingRequestCounter();
info->started = true;
info->fun(info->arg, FinishAsyncCleanupHook, info);
}

AsyncCleanupHookHandle AddEnvironmentCleanupHook(
Isolate* isolate,
AsyncCleanupHook fun,
void* arg) {
Environment* env = Environment::GetCurrent(isolate);
CHECK_NOT_NULL(env);
auto info = std::make_shared<AsyncCleanupHookInfo>();
info->env = env;
info->fun = fun;
info->arg = arg;
info->self = info;
env->AddCleanupHook(RunAsyncCleanupHook, info.get());
return AsyncCleanupHookHandle(new ACHHandle { info });
}

void RemoveEnvironmentCleanupHook(
AsyncCleanupHookHandle handle) {
if (handle->info->started) return;
handle->info->self.reset();
handle->info->env->RemoveCleanupHook(RunAsyncCleanupHook, handle->info.get());
}

async_id AsyncHooksGetExecutionAsyncId(Isolate* isolate) {
Environment* env = Environment::GetCurrent(isolate);
if (env == nullptr) return -1;
Expand Down
14 changes: 14 additions & 0 deletions src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -867,6 +867,20 @@ NODE_EXTERN void RemoveEnvironmentCleanupHook(v8::Isolate* isolate,
void (*fun)(void* arg),
void* arg);

/* These are async equivalents of the above. After the cleanup hook is invoked,
* `cb(cbarg)` *must* be called, and attempting to remove the cleanup hook will
* have no effect. */
struct ACHHandle;
struct DeleteACHHandle { void operator()(ACHHandle*) const; };
typedef std::unique_ptr<ACHHandle, DeleteACHHandle> AsyncCleanupHookHandle;

NODE_EXTERN AsyncCleanupHookHandle AddEnvironmentCleanupHook(
v8::Isolate* isolate,
void (*fun)(void* arg, void (*cb)(void*), void* cbarg),
void* arg);

NODE_EXTERN void RemoveEnvironmentCleanupHook(AsyncCleanupHookHandle holder);

/* Returns the id of the current execution context. If the return value is
* zero then no execution has been set. This will happen if the user handles
* I/O from native code. */
Expand Down
32 changes: 32 additions & 0 deletions src/node_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,38 @@ napi_status napi_remove_env_cleanup_hook(napi_env env,
return napi_ok;
}

struct napi_async_cleanup_hook_handle__ {
node::AsyncCleanupHookHandle handle;
};

napi_status napi_add_async_cleanup_hook(
napi_env env,
void (*fun)(void* arg, void(* cb)(void*), void* cbarg),
void* arg,
napi_async_cleanup_hook_handle* remove_handle) {
CHECK_ENV(env);
CHECK_ARG(env, fun);

auto handle = node::AddEnvironmentCleanupHook(env->isolate, fun, arg);
if (remove_handle != nullptr) {
*remove_handle = new napi_async_cleanup_hook_handle__ { std::move(handle) };
}

return napi_ok;
addaleax marked this conversation as resolved.
Show resolved Hide resolved
}

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;

return napi_ok;
addaleax marked this conversation as resolved.
Show resolved Hide resolved
}

napi_status napi_fatal_exception(napi_env env, napi_value err) {
NAPI_PREAMBLE(env);
CHECK_ARG(env, err);
Expand Down
14 changes: 14 additions & 0 deletions src/node_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,20 @@ napi_ref_threadsafe_function(napi_env env, napi_threadsafe_function func);

#endif // NAPI_VERSION >= 4

#ifdef NAPI_EXPERIMENTAL

NAPI_EXTERN napi_status napi_add_async_cleanup_hook(
napi_env env,
void (*fun)(void* arg, void(* cb)(void*), void* cbarg),
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

EXTERN_C_END

#endif // SRC_NODE_API_H_
4 changes: 4 additions & 0 deletions src/node_api_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,8 @@ typedef struct {
const char* release;
} napi_node_version;

#ifdef NAPI_EXPERIMENTAL
typedef struct napi_async_cleanup_hook_handle__* napi_async_cleanup_hook_handle;
#endif // NAPI_EXPERIMENTAL

#endif // SRC_NODE_API_TYPES_H_
59 changes: 59 additions & 0 deletions test/addons/async-cleanup-hook/binding.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
#include <assert.h>
#include <node.h>
#include <uv.h>

void MustNotCall(void* arg, void(*cb)(void*), void* cbarg) {
assert(0);
}

struct AsyncData {
uv_async_t async;
v8::Isolate* isolate;
node::AsyncCleanupHookHandle handle;
void (*done_cb)(void*);
void* done_arg;
};

void AsyncCleanupHook(void* arg, void(*cb)(void*), void* cbarg) {
AsyncData* data = static_cast<AsyncData*>(arg);
uv_loop_t* loop = node::GetCurrentEventLoop(data->isolate);
assert(loop != nullptr);
int err = uv_async_init(loop, &data->async, [](uv_async_t* async) {
AsyncData* data = static_cast<AsyncData*>(async->data);
// Attempting to remove the cleanup hook here should be a no-op since it
// has already been started.
node::RemoveEnvironmentCleanupHook(std::move(data->handle));

uv_close(reinterpret_cast<uv_handle_t*>(async), [](uv_handle_t* handle) {
AsyncData* data = static_cast<AsyncData*>(handle->data);
data->done_cb(data->done_arg);
delete data;
});
});
assert(err == 0);

data->async.data = data;
data->done_cb = cb;
data->done_arg = cbarg;
uv_async_send(&data->async);
}

void Initialize(v8::Local<v8::Object> exports,
v8::Local<v8::Value> module,
v8::Local<v8::Context> context) {
AsyncData* data = new AsyncData();
data->isolate = context->GetIsolate();
auto handle = node::AddEnvironmentCleanupHook(
context->GetIsolate(),
AsyncCleanupHook,
data);
data->handle = std::move(handle);

auto must_not_call_handle = node::AddEnvironmentCleanupHook(
context->GetIsolate(),
MustNotCall,
nullptr);
node::RemoveEnvironmentCleanupHook(std::move(must_not_call_handle));
}

NODE_MODULE_CONTEXT_AWARE(NODE_GYP_MODULE_NAME, Initialize)
9 changes: 9 additions & 0 deletions test/addons/async-cleanup-hook/binding.gyp
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
'targets': [
{
'target_name': 'binding',
'sources': [ 'binding.cc' ],
'includes': ['../common.gypi'],
}
]
}
8 changes: 8 additions & 0 deletions test/addons/async-cleanup-hook/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
'use strict';
const common = require('../../common');
const path = require('path');
const { Worker } = require('worker_threads');
const binding = path.resolve(__dirname, `./build/${common.buildType}/binding`);

const w = new Worker(`require(${JSON.stringify(binding)})`, { eval: true });
w.on('exit', common.mustCall(() => require(binding)));
Loading