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.

Sign up for GitHub

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

Jump to bottom

node-api: document node-api shutdown finalization #45903

Merged
merged 1 commit into from
Mar 20, 2023
+31 −14
Merged

node-api: document node-api shutdown finalization #45903

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 31 additions & 14 deletions doc/api/n-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,11 +477,11 @@ may be called multiple times, from multiple contexts, and even concurrently from
multiple threads.

Native addons may need to allocate global state which they use during
their entire life cycle such that the state must be unique to each instance of
the addon.
their life cycle of an Node.js environment such that the state can be
unique to each instance of the addon.

To this end, Node-API provides a way to allocate data such that its life cycle
is tied to the life cycle of the Agent.
To this end, Node-API provides a way to associate data such that its life cycle
is tied to the life cycle of a Node.js environment.

### `napi_set_instance_data`

Expand Down Expand Up @@ -509,11 +509,11 @@ napi_status napi_set_instance_data(napi_env env,

Returns `napi_ok` if the API succeeded.

This API associates `data` with the currently running Agent. `data` can later
be retrieved using `napi_get_instance_data()`. Any existing data associated with
the currently running Agent which was set by means of a previous call to
`napi_set_instance_data()` will be overwritten. If a `finalize_cb` was provided
by the previous call, it will not be called.
This API associates `data` with the currently running Node.js environment. `data`
can later be retrieved using `napi_get_instance_data()`. Any existing data
associated with the currently running Node.js environment which was set by means
of a previous call to `napi_set_instance_data()` will be overwritten. If a
`finalize_cb` was provided by the previous call, it will not be called.

### `napi_get_instance_data`

Expand All @@ -531,13 +531,13 @@ napi_status napi_get_instance_data(napi_env env,

* `[in] env`: The environment that the Node-API call is invoked under.
* `[out] data`: The data item that was previously associated with the currently
running Agent by a call to `napi_set_instance_data()`.
running Node.js environment by a call to `napi_set_instance_data()`.

Returns `napi_ok` if the API succeeded.

This API retrieves data that was previously associated with the currently
running Agent via `napi_set_instance_data()`. If no data is set, the call will
succeed and `data` will be set to `NULL`.
running Node.js environment via `napi_set_instance_data()`. If no data is set,
the call will succeed and `data` will be set to `NULL`.

## Basic Node-API data types

Expand Down Expand Up @@ -1798,11 +1798,11 @@ If still valid, this API returns the `napi_value` representing the
JavaScript `Object` associated with the `napi_ref`. Otherwise, result
will be `NULL`.

### Cleanup on exit of the current Node.js instance
### Cleanup on exit of the current Node.js environment

While a Node.js process typically releases all its resources when exiting,
embedders of Node.js, or future Worker support, may require addons to register
clean-up hooks that will be run once the current Node.js instance exits.
clean-up hooks that will be run once the current Node.js environment exits.

Node-API provides functions for registering and un-registering such callbacks.
When those callbacks are run, all resources that are being held by the addon
Expand Down Expand Up @@ -1928,6 +1928,22 @@ the hook from being executed, unless it has already started executing.
This must be called on any `napi_async_cleanup_hook_handle` value obtained
from [`napi_add_async_cleanup_hook`][].

### Finalization on the exit of the Node.js environment

The Node.js environment may be torn down at an arbitrary time as soon as
possible with JavaScript execution disallowed, like on the request of
[`worker.terminate()`][]. When the environment is being torn down, the
registered `napi_finalize` callbacks of JavaScript objects, Thread-safe
functions and environment instance data are invoked immediately and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean these run in parallel?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, their napi_finalize callbacks can be run in an interleaving pattern, on the JavaScript thread.

independently.

The invocation of `napi_finalize` callbacks are scheduled after the manually
registered cleanup hooks. In order to ensure a proper order of addon
finalization during environment shutdown to avoid use-after-free in the
`napi_finalize` callback, addons should register a cleanup hook with
`napi_add_env_cleanup_hook` and `napi_add_async_cleanup_hook` to manually
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you expand a bit here? Is the suggestion to use the hooks instead of napi_finalize callback?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is specifically focused on the finalization at shutdown. Still, addons should set an napi_finalize callback to finalize the native objects properly.

However, their napi_finalize callbacks can be run in an interleaving pattern forcefully, regardless of if they are actually referenced. This would lead to the problem mentioned in nodejs/node-addon-api#1109:

Normally, we expect the order to be:

  1. create an ObjectWrap,
  2. create a ThreadSafeFunction and save it to a field of that ObjectWrap,
  3. release the ObjectWrap,
  4. release the ThreadSafeFunction in the ~ObjectWrap.

However, when the environment is shutting down, the order would be:

  1. create an ObjectWrap,
  2. create a ThreadSafeFunction and save it to a field of that ObjectWrap,
  3. env shutdown, run napi_finalize in reversed creation order,
  4. release the ThreadSafeFunction,
  5. release the ObjectWrap => crash as the ThreadSafeFunction has already been finalized.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@legendecas Does this mean that I am supposed to maintain a central directory of all ThreadSafeFunction by environment so that I can Release in the cleanup hook because by the time the ObjectWrap destructor is called, it would be too late?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@legendecas in particular, if I use napi_add_async_cleanup_hook, can I delay the destruction of the environment until all ObjectWraps has been destroyed? Ie, will the destructors get called while Node is waiting for napi_remove_async_cleanup_hook? I strongly suggest that you do at least this, because otherwise this would mean that all ObjectWraps are to have a two-staged destruction with a C++ destructor and a, lets call it ::Destroy(), method which can be called both from the destructor and the environment cleanup hook. This would throw out of the window all the work done on the clean node-addon-api interface which aligns the JS object lifecycle with the C++ constructor/destructor.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mmomtchev Does this mean that I am supposed to maintain a central directory of all ThreadSafeFunction by environment so that I can Release in the cleanup hook because by the time the ObjectWrap destructor is called, it would be too late?

If the ThreadSafeFunction is saved as a field of a c++ ObjectWrap, it should be released in the destructor of the ObjectWrap. So it is sufficient for an add-on to keep track of ObjectWraps and released them in a napi_cleanup_hook.

in particular, if I use napi_add_async_cleanup_hook, can I delay the destruction of the environment until all ObjectWraps has been destroyed? Ie, will the destructors get called while Node is waiting for napi_remove_async_cleanup_hook?

This is how napi_async_cleanup_hook_handle works now. If an napi_async_cleanup_hook_handle is still pending removal by napi_remove_async_cleanup_hook, the napi_env is not going to be destructed. This is verified with tests at #46692.

release the allocated resource in a proper order.

## Module registration

Node-API modules are registered in a manner similar to other modules
Expand Down Expand Up @@ -6452,6 +6468,7 @@ the add-on's file name during loading.
[`process.release`]: process.md#processrelease
[`uv_ref`]: https://docs.libuv.org/en/v1.x/handle.html#c.uv_ref
[`uv_unref`]: https://docs.libuv.org/en/v1.x/handle.html#c.uv_unref
[`worker.terminate()`]: worker_threads.md#workerterminate
[async_hooks `type`]: async_hooks.md#type
[context-aware addons]: addons.md#context-aware-addons
[docs]: https://github.com/nodejs/node-addon-api#api-documentation
Expand Down