From 32906ddcac09e9f5934ae8af98dcfb0b943a4c89 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Mon, 18 Dec 2023 23:37:15 -0800 Subject: [PATCH] node-api: segregate nogc APIs from rest via type system We define a new type called `node_api_nogc_env` as the `const` version of `napi_env` and `node_api_nogc_finalize` as a variant of `napi_finalize` that accepts a `node_api_nogc_env` as its first argument. We then modify those APIs which do not affect GC state as accepting a `node_api_nogc_env`. APIs accepting finalizer callbacks are modified to accept `node_api_nogc_finalize` callbacks. Thus, the only way to attach a `napi_finalize` callback, wherein Node-APIs affecting GC state may be called is to call `node_api_post_finalizer` from a `node_api_nogc_finalize` callback. In keeping with the process of introducing new Node-APIs, this feature is guarded by `NAPI_EXPERIMENTAL`. Since this feature modifies APIs already marked as stable, it is additionally guared by `NODE_API_EXPERIMENTAL_NOGC_ENV`, so as to provide a further buffer to adoption. Nevertheless, both guards must be removed upon releasing a new version of Node-API. PR-URL: https://github.com/nodejs/node/pull/50060 Reviewed-By: Chengzhong Wu Reviewed-By: Vladimir Morozov Reviewed-By: Michael Dawson Backport-PR-URL: https://github.com/nodejs/node/pull/51804 (cherry picked from commit 7a216d5fd6331c98c117ef471d0c971c8d97f757) --- doc/api/n-api.md | 139 +++++++++++++----- doc/contributing/adding-new-napi-api.md | 13 ++ doc/contributing/releases-node-api.md | 17 ++- src/js_native_api.h | 44 +++--- src/js_native_api_types.h | 39 +++++ src/js_native_api_v8.cc | 84 ++++++----- src/js_native_api_v8.h | 8 +- src/node_api.cc | 46 +++--- src/node_api.h | 32 ++-- test/js-native-api/common.h | 37 +++++ .../test_cannot_run_js/test_cannot_run_js.c | 11 +- .../test_finalizer/test_finalizer.c | 18 ++- test/js-native-api/test_string/test_string.c | 2 +- .../test_reference_by_node_api_version.c | 8 +- .../test_uncaught_exception.c | 14 +- tools/build-addons.mjs | 2 + 16 files changed, 368 insertions(+), 146 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index c95c3f9cecc6d7..f26544a6f89b35 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -78,7 +78,7 @@ it still gets the benefits of the ABI stability provided by the C API. When using `node-addon-api` instead of the C APIs, start with the API [docs][] for `node-addon-api`. -The [Node-API Resource](https://nodejs.github.io/node-addon-examples/) offers +The [Node-API Resource](https://nodejs.github.io/node-addon-examples/) offers an excellent orientation and tips for developers just getting started with Node-API and `node-addon-api`. Additional media resources can be found on the [Node-API Media][] page. @@ -175,7 +175,8 @@ developers have run into limitations in node-gyp. [CMake.js][] is an alternative build system based on [CMake][]. CMake.js is a good choice for projects that already use CMake or for -developers affected by limitations in node-gyp. +developers affected by limitations in node-gyp. [`build_with_cmake`][] is an +example of a CMake-based native addon project. ### Uploading precompiled binaries @@ -237,6 +238,18 @@ Some of the Node-API surface is experimental and requires explicit opt-in: In this case the entire API surface, including any experimental APIs, will be available to the module code. +Occasionally, experimental features are introduced that affect already-released +and stable APIs. These features can be disabled by an opt-out: + +```c +#define NAPI_EXPERIMENTAL +#define NODE_API_EXPERIMENTAL__OPT_OUT +#include +``` + +where `` is the name of an experimental feature that affects both +experimental and stable APIs. + ## Node-API version matrix Node-API versions are additive and versioned independently from Node.js. @@ -398,7 +411,7 @@ napi_value create_addon(napi_env env) { #include #include "addon.h" -NAPI_MODULE_INIT() { +NAPI_MODULE_INIT(/* napi_env env, napi_value exports */) { // This function body is expected to return a `napi_value`. // The variables `napi_env env` and `napi_value exports` may be used within // the body, as they are provided by the definition of `NAPI_MODULE_INIT()`. @@ -443,7 +456,7 @@ napiVersion: 6 --> ```c -napi_status napi_set_instance_data(napi_env env, +napi_status napi_set_instance_data(node_api_nogc_env env, void* data, napi_finalize finalize_cb, void* finalize_hint); @@ -475,7 +488,7 @@ napiVersion: 6 --> ```c -napi_status napi_get_instance_data(napi_env env, +napi_status napi_get_instance_data(node_api_nogc_env env, void** data); ``` @@ -577,6 +590,22 @@ when an instance of a native addon is unloaded. Notification of this event is delivered through the callbacks given to [`napi_add_env_cleanup_hook`][] and [`napi_set_instance_data`][]. +### `node_api_nogc_env` + +> Stability: 1 - Experimental + +This variant of `napi_env` is passed to synchronous finalizers +([`node_api_nogc_finalize`][]). There is a subset of Node-APIs which accept +a parameter of type `node_api_nogc_env` as their first argument. These APIs do +not access the state of the JavaScript engine and are thus safe to call from +synchronous finalizers. Passing a parameter of type `napi_env` to these APIs is +allowed, however, passing a parameter of type `node_api_nogc_env` to APIs that +access the JavaScript engine state is not allowed. Attempting to do so without +a cast will produce a compiler warning or an error when add-ons are compiled +with flags which cause them to emit warnings and/or errors when incorrect +pointer types are passed into a function. Calling such APIs from a synchronous +finalizer will ultimately result in the termination of the application. + ### `napi_value` This is an opaque pointer that is used to represent a JavaScript value. @@ -741,32 +770,36 @@ typedef napi_value (*napi_callback)(napi_env, napi_callback_info); Unless for reasons discussed in [Object Lifetime Management][], creating a handle and/or callback scope inside a `napi_callback` is not necessary. -#### `napi_finalize` +#### `node_api_nogc_finalize` +> Stability: 1 - Experimental + Function pointer type for add-on provided functions that allow the user to be notified when externally-owned data is ready to be cleaned up because the -object with which it was associated with has been garbage-collected. The user -must provide a function satisfying the following signature which would get -called upon the object's collection. Currently, `napi_finalize` can be used for +object it was associated with has been garbage-collected. The user must provide +a function satisfying the following signature which would get called upon the +object's collection. Currently, `node_api_nogc_finalize` can be used for finding out when objects that have external data are collected. ```c -typedef void (*napi_finalize)(napi_env env, - void* finalize_data, - void* finalize_hint); +typedef void (*node_api_nogc_finalize)(node_api_nogc_env env, + void* finalize_data, + void* finalize_hint); ``` Unless for reasons discussed in [Object Lifetime Management][], creating a handle and/or callback scope inside the function body is not necessary. Since these functions may be called while the JavaScript engine is in a state -where it cannot execute JavaScript code, some Node-API calls may return -`napi_pending_exception` even when there is no exception pending. +where it cannot execute JavaScript code, only Node-APIs which accept a +`node_api_nogc_env` as their first parameter may be called. +[`node_api_post_finalizer`][] can be used to schedule Node-API calls that +require access to the JavaScript engine's state to run after the current +garbage collection cycle has completed. In the case of [`node_api_create_external_string_latin1`][] and [`node_api_create_external_string_utf16`][] the `env` parameter may be null, @@ -775,11 +808,39 @@ shutdown. Change History: +* experimental (`NAPI_EXPERIMENTAL`): + + Only Node-API calls that accept a `node_api_nogc_env` as their first + parameter may be called, otherwise the application will be terminated with an + appropriate error message. This feature can be turned off by defining + `NODE_API_EXPERIMENTAL_NOGC_ENV_OPT_OUT`. + +#### `napi_finalize` + + + +Function pointer type for add-on provided function that allow the user to +schedule a group of calls to Node-APIs in response to a garbage collection +event, after the garbage collection cycle has completed. These function +pointers can be used with [`node_api_post_finalizer`][]. + +```c +typedef void (*napi_finalize)(napi_env env, + void* finalize_data, + void* finalize_hint); +``` + +Change History: + * experimental (`NAPI_EXPERIMENTAL` is defined): - Node-API calls made from a finalizer will return `napi_cannot_run_js` when - the JavaScript engine is unable to execute JavaScript, and will return - `napi_exception_pending` if there is a pending exception. + A function of this type may no longer be used as a finalizer, except with + [`node_api_post_finalizer`][]. [`node_api_nogc_finalize`][] must be used + instead. This feature can be turned off by defining + `NODE_API_EXPERIMENTAL_NOGC_ENV_OPT_OUT`. #### `napi_async_execute_callback` @@ -979,7 +1040,7 @@ napiVersion: 1 ```c napi_status -napi_get_last_error_info(napi_env env, +napi_get_last_error_info(node_api_nogc_env env, const napi_extended_error_info** result); ``` @@ -1798,7 +1859,7 @@ napiVersion: 3 --> ```c -NODE_EXTERN napi_status napi_add_env_cleanup_hook(napi_env env, +NODE_EXTERN napi_status napi_add_env_cleanup_hook(node_api_nogc_env env, napi_cleanup_hook fun, void* arg); ``` @@ -1828,7 +1889,7 @@ napiVersion: 3 --> ```c -NAPI_EXTERN napi_status napi_remove_env_cleanup_hook(napi_env env, +NAPI_EXTERN napi_status napi_remove_env_cleanup_hook(node_api_nogc_env env, void (*fun)(void* arg), void* arg); ``` @@ -1857,7 +1918,7 @@ changes: ```c NAPI_EXTERN napi_status napi_add_async_cleanup_hook( - napi_env env, + node_api_nogc_env env, napi_async_cleanup_hook hook, void* arg, napi_async_cleanup_hook_handle* remove_handle); @@ -2015,7 +2076,7 @@ You can also use the `NAPI_MODULE_INIT` macro, which acts as a shorthand for `NAPI_MODULE` and defining an `Init` function: ```c -NAPI_MODULE_INIT() { +NAPI_MODULE_INIT(/* napi_env env, napi_value exports */) { napi_value answer; napi_status result; @@ -2029,6 +2090,9 @@ NAPI_MODULE_INIT() { } ``` +The parameters `env` and `exports` are provided to the body of the +`NAPI_MODULE_INIT` macro. + All Node-API addons are context-aware, meaning they may be loaded multiple times. There are a few design considerations when declaring such a module. The documentation on [context-aware addons][] provides more details. @@ -5391,7 +5455,7 @@ napiVersion: 5 napi_status napi_add_finalizer(napi_env env, napi_value js_object, void* finalize_data, - napi_finalize finalize_cb, + node_api_nogc_finalize finalize_cb, void* finalize_hint, napi_ref* result); ``` @@ -5429,7 +5493,7 @@ added: v18.19.0 > Stability: 1 - Experimental ```c -napi_status node_api_post_finalizer(napi_env env, +napi_status node_api_post_finalizer(node_api_nogc_env env, napi_finalize finalize_cb, void* finalize_data, void* finalize_hint); @@ -5499,7 +5563,7 @@ Once created the async worker can be queued for execution using the [`napi_queue_async_work`][] function: ```c -napi_status napi_queue_async_work(napi_env env, +napi_status napi_queue_async_work(node_api_nogc_env env, napi_async_work work); ``` @@ -5591,7 +5655,7 @@ napiVersion: 1 --> ```c -napi_status napi_queue_async_work(napi_env env, +napi_status napi_queue_async_work(node_api_nogc_env env, napi_async_work work); ``` @@ -5612,7 +5676,7 @@ napiVersion: 1 --> ```c -napi_status napi_cancel_async_work(napi_env env, +napi_status napi_cancel_async_work(node_api_nogc_env env, napi_async_work work); ``` @@ -5816,7 +5880,7 @@ typedef struct { const char* release; } napi_node_version; -napi_status napi_get_node_version(napi_env env, +napi_status napi_get_node_version(node_api_nogc_env env, const napi_node_version** version); ``` @@ -5839,7 +5903,7 @@ napiVersion: 1 --> ```c -napi_status napi_get_version(napi_env env, +napi_status napi_get_version(node_api_nogc_env env, uint32_t* result); ``` @@ -5872,7 +5936,7 @@ napiVersion: 1 --> ```c -NAPI_EXTERN napi_status napi_adjust_external_memory(napi_env env, +NAPI_EXTERN napi_status napi_adjust_external_memory(node_api_nogc_env env, int64_t change_in_bytes, int64_t* result); ``` @@ -6089,7 +6153,7 @@ napiVersion: 2 --> ```c -NAPI_EXTERN napi_status napi_get_uv_event_loop(napi_env env, +NAPI_EXTERN napi_status napi_get_uv_event_loop(node_api_nogc_env env, struct uv_loop_s** loop); ``` @@ -6403,7 +6467,7 @@ napiVersion: 4 ```c NAPI_EXTERN napi_status -napi_ref_threadsafe_function(napi_env env, napi_threadsafe_function func); +napi_ref_threadsafe_function(node_api_nogc_env env, napi_threadsafe_function func); ``` * `[in] env`: The environment that the API is invoked under. @@ -6429,7 +6493,7 @@ napiVersion: 4 ```c NAPI_EXTERN napi_status -napi_unref_threadsafe_function(napi_env env, napi_threadsafe_function func); +napi_unref_threadsafe_function(node_api_nogc_env env, napi_threadsafe_function func); ``` * `[in] env`: The environment that the API is invoked under. @@ -6455,7 +6519,7 @@ napiVersion: 9 ```c NAPI_EXTERN napi_status -node_api_get_module_file_name(napi_env env, const char** result); +node_api_get_module_file_name(node_api_nogc_env env, const char** result); ``` @@ -6519,6 +6583,7 @@ the add-on's file name during loading. [`Number.MIN_SAFE_INTEGER`]: https://tc39.github.io/ecma262/#sec-number.min_safe_integer [`Worker`]: worker_threads.md#class-worker [`async_hooks.executionAsyncResource()`]: async_hooks.md#async_hooksexecutionasyncresource +[`build_with_cmake`]: https://github.com/nodejs/node-addon-examples/tree/main/build_with_cmake [`global`]: globals.md#global [`init` hooks]: async_hooks.md#initasyncid-type-triggerasyncid-resource [`napi_add_async_cleanup_hook`]: #napi_add_async_cleanup_hook @@ -6582,6 +6647,8 @@ the add-on's file name during loading. [`node_api_create_external_string_latin1`]: #node_api_create_external_string_latin1 [`node_api_create_external_string_utf16`]: #node_api_create_external_string_utf16 [`node_api_create_syntax_error`]: #node_api_create_syntax_error +[`node_api_nogc_finalize`]: #node_api_nogc_finalize +[`node_api_post_finalizer`]: #node_api_post_finalizer [`node_api_throw_syntax_error`]: #node_api_throw_syntax_error [`process.release`]: process.md#processrelease [`uv_ref`]: https://docs.libuv.org/en/v1.x/handle.html#c.uv_ref diff --git a/doc/contributing/adding-new-napi-api.md b/doc/contributing/adding-new-napi-api.md index e834a395b6206f..a970ed037714ec 100644 --- a/doc/contributing/adding-new-napi-api.md +++ b/doc/contributing/adding-new-napi-api.md @@ -51,3 +51,16 @@ Node-API. to the decision to take an API out of experimental status. * The API **must** be implemented in a Node.js implementation with an alternate VM. + +Since the adoption of the policy whereby moving to a later version of Node-API +from an earlier version may entail rework of existing code, it is possible to +introduce modifications to already-released Node-APIs, as long as the +modifications affect neither the ABI nor the API of earlier versions. Such +modifications **must** be accompanied by an opt-out flag. This provides add-on +maintainers who take advantage of the initial compile-time flag to track +impending changes to Node-API with + +* a quick fix to the breakage caused, +* a notification that such breakage is impending, and thus +* a buffer to adoption above and beyond the one provided by the initial + compile-time flag. diff --git a/doc/contributing/releases-node-api.md b/doc/contributing/releases-node-api.md index 50b80bb2f743f2..3656259fd0e252 100644 --- a/doc/contributing/releases-node-api.md +++ b/doc/contributing/releases-node-api.md @@ -84,7 +84,11 @@ define guards on the declaration of the new Node-API. Check for these guards with: ```console -grep NAPI_EXPERIMENTAL src/js_native_api{_types,}.h src/node_api{_types,}.h +grep \ + -E \ + 'N(ODE_)?API_EXPERIMENTAL' \ + src/js_native_api{_types,}.h \ + src/node_api{_types,}.h ``` and update the define version guards with the release version: @@ -102,6 +106,9 @@ and update the define version guards with the release version: Remove any feature flags of the form `NODE_API_EXPERIMENTAL_HAS_`. +Remove any additional `NODE_API_EXPERIMENTAL_*` guards along with +`NAPI_EXPERIMENTAL`. + Also, update the Node-API version value of the `napi_get_version` test in `test/js-native-api/test_general/test.js` with the release version `x`: @@ -130,7 +137,11 @@ commits should already include `NAPI_EXPERIMENTAL` definition for the tests. Check for these definitions with: ```console -grep NAPI_EXPERIMENTAL test/node-api/*/{*.{h,c},binding.gyp} test/js-native-api/*/{*.{h,c},binding.gyp} +grep \ + -E \ + 'N(ODE_)?API_EXPERIMENTAL' \ + test/node-api/*/{*.{h,c},binding.gyp} \ + test/js-native-api/*/{*.{h,c},binding.gyp} ``` and substitute the `NAPI_EXPERIMENTAL` with the release version @@ -141,6 +152,8 @@ and substitute the `NAPI_EXPERIMENTAL` with the release version + #define NAPI_VERSION 10 ``` +Remove any `NODE_API_EXPERIMENTAL_*` flags. + #### Step 4. Update document If this release includes new Node-APIs that were first released in this diff --git a/src/js_native_api.h b/src/js_native_api.h index a3ee3a0e224262..53034214b82ddd 100644 --- a/src/js_native_api.h +++ b/src/js_native_api.h @@ -49,8 +49,8 @@ EXTERN_C_START -NAPI_EXTERN napi_status NAPI_CDECL -napi_get_last_error_info(napi_env env, const napi_extended_error_info** result); +NAPI_EXTERN napi_status NAPI_CDECL napi_get_last_error_info( + node_api_nogc_env env, const napi_extended_error_info** result); // Getters for defined singletons NAPI_EXTERN napi_status NAPI_CDECL napi_get_undefined(napi_env env, @@ -98,7 +98,7 @@ NAPI_EXTERN napi_status NAPI_CDECL node_api_create_external_string_latin1(napi_env env, char* str, size_t length, - napi_finalize finalize_callback, + node_api_nogc_finalize finalize_callback, void* finalize_hint, napi_value* result, bool* copied); @@ -106,7 +106,7 @@ NAPI_EXTERN napi_status NAPI_CDECL node_api_create_external_string_utf16(napi_env env, char16_t* str, size_t length, - napi_finalize finalize_callback, + node_api_nogc_finalize finalize_callback, void* finalize_hint, napi_value* result, bool* copied); @@ -290,7 +290,7 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_instanceof(napi_env env, // Gets all callback info in a single call. (Ugly, but faster.) NAPI_EXTERN napi_status NAPI_CDECL napi_get_cb_info( - napi_env env, // [in] NAPI environment handle + napi_env env, // [in] Node-API environment handle napi_callback_info cbinfo, // [in] Opaque callback-info handle size_t* argc, // [in-out] Specifies the size of the provided argv array // and receives the actual count of args. @@ -314,7 +314,7 @@ napi_define_class(napi_env env, NAPI_EXTERN napi_status NAPI_CDECL napi_wrap(napi_env env, napi_value js_object, void* native_object, - napi_finalize finalize_cb, + node_api_nogc_finalize finalize_cb, void* finalize_hint, napi_ref* result); NAPI_EXTERN napi_status NAPI_CDECL napi_unwrap(napi_env env, @@ -326,7 +326,7 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_remove_wrap(napi_env env, NAPI_EXTERN napi_status NAPI_CDECL napi_create_external(napi_env env, void* data, - napi_finalize finalize_cb, + node_api_nogc_finalize finalize_cb, void* finalize_hint, napi_value* result); NAPI_EXTERN napi_status NAPI_CDECL napi_get_value_external(napi_env env, @@ -425,7 +425,7 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_create_external_arraybuffer(napi_env env, void* external_data, size_t byte_length, - napi_finalize finalize_cb, + node_api_nogc_finalize finalize_cb, void* finalize_hint, napi_value* result); #endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED @@ -467,7 +467,7 @@ napi_get_dataview_info(napi_env env, size_t* byte_offset); // version management -NAPI_EXTERN napi_status NAPI_CDECL napi_get_version(napi_env env, +NAPI_EXTERN napi_status NAPI_CDECL napi_get_version(node_api_nogc_env env, uint32_t* result); // Promises @@ -491,7 +491,7 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_run_script(napi_env env, // Memory management NAPI_EXTERN napi_status NAPI_CDECL napi_adjust_external_memory( - napi_env env, int64_t change_in_bytes, int64_t* adjusted_value); + node_api_nogc_env env, int64_t change_in_bytes, int64_t* adjusted_value); #if NAPI_VERSION >= 5 @@ -509,12 +509,13 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_get_date_value(napi_env env, double* result); // Add finalizer for pointer -NAPI_EXTERN napi_status NAPI_CDECL napi_add_finalizer(napi_env env, - napi_value js_object, - void* finalize_data, - napi_finalize finalize_cb, - void* finalize_hint, - napi_ref* result); +NAPI_EXTERN napi_status NAPI_CDECL +napi_add_finalizer(napi_env env, + napi_value js_object, + void* finalize_data, + node_api_nogc_finalize finalize_cb, + void* finalize_hint, + napi_ref* result); #endif // NAPI_VERSION >= 5 @@ -522,7 +523,7 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_add_finalizer(napi_env env, #define NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER NAPI_EXTERN napi_status NAPI_CDECL -node_api_post_finalizer(napi_env env, +node_api_post_finalizer(node_api_nogc_env env, napi_finalize finalize_cb, void* finalize_data, void* finalize_hint); @@ -566,10 +567,13 @@ napi_get_all_property_names(napi_env env, napi_value* result); // Instance data -NAPI_EXTERN napi_status NAPI_CDECL napi_set_instance_data( - napi_env env, void* data, napi_finalize finalize_cb, void* finalize_hint); +NAPI_EXTERN napi_status NAPI_CDECL +napi_set_instance_data(node_api_nogc_env env, + void* data, + napi_finalize finalize_cb, + void* finalize_hint); -NAPI_EXTERN napi_status NAPI_CDECL napi_get_instance_data(napi_env env, +NAPI_EXTERN napi_status NAPI_CDECL napi_get_instance_data(node_api_nogc_env env, void** data); #endif // NAPI_VERSION >= 6 diff --git a/src/js_native_api_types.h b/src/js_native_api_types.h index 005382f173fee9..7cb5b080cc377a 100644 --- a/src/js_native_api_types.h +++ b/src/js_native_api_types.h @@ -22,6 +22,35 @@ typedef uint16_t char16_t; // JSVM API types are all opaque pointers for ABI stability // typedef undefined structs instead of void* for compile time type safety typedef struct napi_env__* napi_env; + +// We need to mark APIs which can be called during garbage collection (GC), +// meaning that they do not affect the state of the JS engine, and can +// therefore be called synchronously from a finalizer that itself runs +// synchronously during GC. Such APIs can receive either a `napi_env` or a +// `node_api_nogc_env` as their first parameter, because we should be able to +// also call them during normal, non-garbage-collecting operations, whereas +// APIs that affect the state of the JS engine can only receive a `napi_env` as +// their first parameter, because we must not call them during GC. In lieu of +// inheritance, we use the properties of the const qualifier to accomplish +// this, because both a const and a non-const value can be passed to an API +// expecting a const value, but only a non-const value can be passed to an API +// expecting a non-const value. +// +// In conjunction with appropriate CFLAGS to warn us if we're passing a const +// (nogc) environment into an API that expects a non-const environment, and the +// definition of nogc finalizer function pointer types below, which receive a +// nogc environment as their first parameter, and can thus only call nogc APIs +// (unless the user explicitly casts the environment), we achieve the ability +// to ensure at compile time that we do not call APIs that affect the state of +// the JS engine from a synchronous (nogc) finalizer. +#if !defined(NAPI_EXPERIMENTAL) || \ + (defined(NAPI_EXPERIMENTAL) && \ + defined(NODE_API_EXPERIMENTAL_NOGC_ENV_OPT_OUT)) +typedef struct napi_env__* node_api_nogc_env; +#else +typedef const struct napi_env__* node_api_nogc_env; +#endif + typedef struct napi_value__* napi_value; typedef struct napi_ref__* napi_ref; typedef struct napi_handle_scope__* napi_handle_scope; @@ -116,6 +145,16 @@ typedef void(NAPI_CDECL* napi_finalize)(napi_env env, void* finalize_data, void* finalize_hint); +#if !defined(NAPI_EXPERIMENTAL) || \ + (defined(NAPI_EXPERIMENTAL) && \ + defined(NODE_API_EXPERIMENTAL_NOGC_ENV_OPT_OUT)) +typedef napi_finalize node_api_nogc_finalize; +#else +typedef void(NAPI_CDECL* node_api_nogc_finalize)(node_api_nogc_env env, + void* finalize_data, + void* finalize_hint); +#endif + typedef struct { // One of utf8name or name should be NULL. const char* utf8name; diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index a1215960628c3c..c7604e9875739d 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -856,7 +856,8 @@ static const char* error_messages[] = { }; napi_status NAPI_CDECL napi_get_last_error_info( - napi_env env, const napi_extended_error_info** result) { + node_api_nogc_env nogc_env, const napi_extended_error_info** result) { + napi_env env = const_cast(nogc_env); CHECK_ENV(env); CHECK_ARG(env, result); @@ -1600,14 +1601,16 @@ napi_status NAPI_CDECL napi_create_string_utf16(napi_env env, }); } -napi_status NAPI_CDECL -node_api_create_external_string_latin1(napi_env env, - char* str, - size_t length, - napi_finalize finalize_callback, - void* finalize_hint, - napi_value* result, - bool* copied) { +napi_status NAPI_CDECL node_api_create_external_string_latin1( + napi_env env, + char* str, + size_t length, + node_api_nogc_finalize nogc_finalize_callback, + void* finalize_hint, + napi_value* result, + bool* copied) { + napi_finalize finalize_callback = + reinterpret_cast(nogc_finalize_callback); return v8impl::NewExternalString( env, str, @@ -1627,14 +1630,16 @@ node_api_create_external_string_latin1(napi_env env, }); } -napi_status NAPI_CDECL -node_api_create_external_string_utf16(napi_env env, - char16_t* str, - size_t length, - napi_finalize finalize_callback, - void* finalize_hint, - napi_value* result, - bool* copied) { +napi_status NAPI_CDECL node_api_create_external_string_utf16( + napi_env env, + char16_t* str, + size_t length, + node_api_nogc_finalize nogc_finalize_callback, + void* finalize_hint, + napi_value* result, + bool* copied) { + napi_finalize finalize_callback = + reinterpret_cast(nogc_finalize_callback); return v8impl::NewExternalString( env, str, @@ -2500,9 +2505,10 @@ GEN_COERCE_FUNCTION(STRING, String, string) napi_status NAPI_CDECL napi_wrap(napi_env env, napi_value js_object, void* native_object, - napi_finalize finalize_cb, + node_api_nogc_finalize nogc_finalize_cb, void* finalize_hint, napi_ref* result) { + napi_finalize finalize_cb = reinterpret_cast(nogc_finalize_cb); return v8impl::Wrap( env, js_object, native_object, finalize_cb, finalize_hint, result); } @@ -2519,11 +2525,13 @@ napi_status NAPI_CDECL napi_remove_wrap(napi_env env, return v8impl::Unwrap(env, obj, result, v8impl::RemoveWrap); } -napi_status NAPI_CDECL napi_create_external(napi_env env, - void* data, - napi_finalize finalize_cb, - void* finalize_hint, - napi_value* result) { +napi_status NAPI_CDECL +napi_create_external(napi_env env, + void* data, + node_api_nogc_finalize nogc_finalize_cb, + void* finalize_hint, + napi_value* result) { + napi_finalize finalize_cb = reinterpret_cast(nogc_finalize_cb); NAPI_PREAMBLE(env); CHECK_ARG(env, result); @@ -2942,7 +2950,7 @@ napi_status NAPI_CDECL napi_create_external_arraybuffer(napi_env env, void* external_data, size_t byte_length, - napi_finalize finalize_cb, + node_api_nogc_finalize finalize_cb, void* finalize_hint, napi_value* result) { // The API contract here is that the cleanup function runs on the JS thread, @@ -3207,7 +3215,8 @@ napi_status NAPI_CDECL napi_get_dataview_info(napi_env env, return napi_clear_last_error(env); } -napi_status NAPI_CDECL napi_get_version(napi_env env, uint32_t* result) { +napi_status NAPI_CDECL napi_get_version(node_api_nogc_env env, + uint32_t* result) { CHECK_ENV(env); CHECK_ARG(env, result); *result = NAPI_VERSION; @@ -3324,14 +3333,16 @@ napi_status NAPI_CDECL napi_run_script(napi_env env, return GET_RETURN_STATUS(env); } -napi_status NAPI_CDECL napi_add_finalizer(napi_env env, - napi_value js_object, - void* finalize_data, - napi_finalize finalize_cb, - void* finalize_hint, - napi_ref* result) { +napi_status NAPI_CDECL +napi_add_finalizer(napi_env env, + napi_value js_object, + void* finalize_data, + node_api_nogc_finalize nogc_finalize_cb, + void* finalize_hint, + napi_ref* result) { // Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw // JS exceptions. + napi_finalize finalize_cb = reinterpret_cast(nogc_finalize_cb); CHECK_ENV_NOT_IN_GC(env); CHECK_ARG(env, js_object); CHECK_ARG(env, finalize_cb); @@ -3355,10 +3366,11 @@ napi_status NAPI_CDECL napi_add_finalizer(napi_env env, #ifdef NAPI_EXPERIMENTAL -napi_status NAPI_CDECL node_api_post_finalizer(napi_env env, +napi_status NAPI_CDECL node_api_post_finalizer(node_api_nogc_env nogc_env, napi_finalize finalize_cb, void* finalize_data, void* finalize_hint) { + napi_env env = const_cast(nogc_env); CHECK_ENV(env); env->EnqueueFinalizer(v8impl::TrackedFinalizer::New( env, finalize_cb, finalize_data, finalize_hint)); @@ -3367,7 +3379,7 @@ napi_status NAPI_CDECL node_api_post_finalizer(napi_env env, #endif -napi_status NAPI_CDECL napi_adjust_external_memory(napi_env env, +napi_status NAPI_CDECL napi_adjust_external_memory(node_api_nogc_env env, int64_t change_in_bytes, int64_t* adjusted_value) { CHECK_ENV(env); @@ -3379,10 +3391,11 @@ napi_status NAPI_CDECL napi_adjust_external_memory(napi_env env, return napi_clear_last_error(env); } -napi_status NAPI_CDECL napi_set_instance_data(napi_env env, +napi_status NAPI_CDECL napi_set_instance_data(node_api_nogc_env nogc_env, void* data, napi_finalize finalize_cb, void* finalize_hint) { + napi_env env = const_cast(nogc_env); CHECK_ENV(env); v8impl::RefBase* old_data = static_cast(env->instance_data); @@ -3398,7 +3411,8 @@ napi_status NAPI_CDECL napi_set_instance_data(napi_env env, return napi_clear_last_error(env); } -napi_status NAPI_CDECL napi_get_instance_data(napi_env env, void** data) { +napi_status NAPI_CDECL napi_get_instance_data(node_api_nogc_env env, + void** data) { CHECK_ENV(env); CHECK_ARG(env, data); diff --git a/src/js_native_api_v8.h b/src/js_native_api_v8.h index 4ffc10a22d38f1..6beda5ba297226 100644 --- a/src/js_native_api_v8.h +++ b/src/js_native_api_v8.h @@ -4,7 +4,7 @@ #include "js_native_api_types.h" #include "js_native_api_v8_internals.h" -inline napi_status napi_clear_last_error(napi_env env); +inline napi_status napi_clear_last_error(node_api_nogc_env env); namespace v8impl { @@ -172,7 +172,8 @@ struct napi_env__ { virtual ~napi_env__() = default; }; -inline napi_status napi_clear_last_error(napi_env env) { +inline napi_status napi_clear_last_error(node_api_nogc_env nogc_env) { + napi_env env = const_cast(nogc_env); env->last_error.error_code = napi_ok; env->last_error.engine_error_code = 0; env->last_error.engine_reserved = nullptr; @@ -180,10 +181,11 @@ inline napi_status napi_clear_last_error(napi_env env) { return napi_ok; } -inline napi_status napi_set_last_error(napi_env env, +inline napi_status napi_set_last_error(node_api_nogc_env nogc_env, napi_status error_code, uint32_t engine_error_code = 0, void* engine_reserved = nullptr) { + napi_env env = const_cast(nogc_env); env->last_error.error_code = error_code; env->last_error.engine_error_code = engine_error_code; env->last_error.engine_reserved = engine_reserved; diff --git a/src/node_api.cc b/src/node_api.cc index da4c5e35a91960..793e0303ca63a3 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -764,7 +764,7 @@ void NAPI_CDECL napi_module_register(napi_module* mod) { node::node_module_register(nm); } -napi_status NAPI_CDECL napi_add_env_cleanup_hook(napi_env env, +napi_status NAPI_CDECL napi_add_env_cleanup_hook(node_api_nogc_env env, napi_cleanup_hook fun, void* arg) { CHECK_ENV(env); @@ -775,7 +775,7 @@ napi_status NAPI_CDECL napi_add_env_cleanup_hook(napi_env env, return napi_ok; } -napi_status NAPI_CDECL napi_remove_env_cleanup_hook(napi_env env, +napi_status NAPI_CDECL napi_remove_env_cleanup_hook(node_api_nogc_env env, napi_cleanup_hook fun, void* arg) { CHECK_ENV(env); @@ -822,10 +822,11 @@ struct napi_async_cleanup_hook_handle__ { }; napi_status NAPI_CDECL -napi_add_async_cleanup_hook(napi_env env, +napi_add_async_cleanup_hook(node_api_nogc_env nogc_env, napi_async_cleanup_hook hook, void* arg, napi_async_cleanup_hook_handle* remove_handle) { + napi_env env = const_cast(nogc_env); CHECK_ENV(env); CHECK_ARG(env, hook); @@ -1036,12 +1037,14 @@ napi_status NAPI_CDECL napi_create_buffer(napi_env env, return GET_RETURN_STATUS(env); } -napi_status NAPI_CDECL napi_create_external_buffer(napi_env env, - size_t length, - void* data, - napi_finalize finalize_cb, - void* finalize_hint, - napi_value* result) { +napi_status NAPI_CDECL +napi_create_external_buffer(napi_env env, + size_t length, + void* data, + node_api_nogc_finalize nogc_finalize_cb, + void* finalize_hint, + napi_value* result) { + napi_finalize finalize_cb = reinterpret_cast(nogc_finalize_cb); NAPI_PREAMBLE(env); CHECK_ARG(env, result); @@ -1125,7 +1128,7 @@ napi_status NAPI_CDECL napi_get_buffer_info(napi_env env, return napi_clear_last_error(env); } -napi_status NAPI_CDECL napi_get_node_version(napi_env env, +napi_status NAPI_CDECL napi_get_node_version(node_api_nogc_env env, const napi_node_version** result) { CHECK_ENV(env); CHECK_ARG(env, result); @@ -1269,14 +1272,16 @@ napi_status NAPI_CDECL napi_delete_async_work(napi_env env, return napi_clear_last_error(env); } -napi_status NAPI_CDECL napi_get_uv_event_loop(napi_env env, uv_loop_t** loop) { +napi_status NAPI_CDECL napi_get_uv_event_loop(node_api_nogc_env nogc_env, + uv_loop_t** loop) { + napi_env env = const_cast(nogc_env); CHECK_ENV(env); CHECK_ARG(env, loop); *loop = reinterpret_cast(env)->node_env()->event_loop(); return napi_clear_last_error(env); } -napi_status NAPI_CDECL napi_queue_async_work(napi_env env, +napi_status NAPI_CDECL napi_queue_async_work(node_api_nogc_env env, napi_async_work work) { CHECK_ENV(env); CHECK_ARG(env, work); @@ -1291,7 +1296,7 @@ napi_status NAPI_CDECL napi_queue_async_work(napi_env env, return napi_clear_last_error(env); } -napi_status NAPI_CDECL napi_cancel_async_work(napi_env env, +napi_status NAPI_CDECL napi_cancel_async_work(node_api_nogc_env env, napi_async_work work) { CHECK_ENV(env); CHECK_ARG(env, work); @@ -1311,10 +1316,12 @@ napi_create_threadsafe_function(napi_env env, size_t max_queue_size, size_t initial_thread_count, void* thread_finalize_data, - napi_finalize thread_finalize_cb, + node_api_nogc_finalize nogc_thread_finalize_cb, void* context, napi_threadsafe_function_call_js call_js_cb, napi_threadsafe_function* result) { + napi_finalize thread_finalize_cb = + reinterpret_cast(nogc_thread_finalize_cb); CHECK_ENV_NOT_IN_GC(env); CHECK_ARG(env, async_resource_name); RETURN_STATUS_IF_FALSE(env, initial_thread_count > 0, napi_invalid_arg); @@ -1396,20 +1403,21 @@ napi_status NAPI_CDECL napi_release_threadsafe_function( return reinterpret_cast(func)->Release(mode); } -napi_status NAPI_CDECL -napi_unref_threadsafe_function(napi_env env, napi_threadsafe_function func) { +napi_status NAPI_CDECL napi_unref_threadsafe_function( + node_api_nogc_env env, napi_threadsafe_function func) { CHECK_NOT_NULL(func); return reinterpret_cast(func)->Unref(); } -napi_status NAPI_CDECL -napi_ref_threadsafe_function(napi_env env, napi_threadsafe_function func) { +napi_status NAPI_CDECL napi_ref_threadsafe_function( + node_api_nogc_env env, napi_threadsafe_function func) { CHECK_NOT_NULL(func); return reinterpret_cast(func)->Ref(); } -napi_status NAPI_CDECL node_api_get_module_file_name(napi_env env, +napi_status NAPI_CDECL node_api_get_module_file_name(node_api_nogc_env nogc_env, const char** result) { + napi_env env = const_cast(nogc_env); CHECK_ENV(env); CHECK_ARG(env, result); diff --git a/src/node_api.h b/src/node_api.h index 49a23aed9c6e90..0074124dc6aa4f 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -131,7 +131,7 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_create_external_buffer(napi_env env, size_t length, void* data, - napi_finalize finalize_cb, + node_api_nogc_finalize finalize_cb, void* finalize_hint, napi_value* result); #endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED @@ -159,20 +159,20 @@ napi_create_async_work(napi_env env, napi_async_work* result); NAPI_EXTERN napi_status NAPI_CDECL napi_delete_async_work(napi_env env, napi_async_work work); -NAPI_EXTERN napi_status NAPI_CDECL napi_queue_async_work(napi_env env, +NAPI_EXTERN napi_status NAPI_CDECL napi_queue_async_work(node_api_nogc_env env, napi_async_work work); -NAPI_EXTERN napi_status NAPI_CDECL napi_cancel_async_work(napi_env env, +NAPI_EXTERN napi_status NAPI_CDECL napi_cancel_async_work(node_api_nogc_env env, napi_async_work work); // version management NAPI_EXTERN napi_status NAPI_CDECL -napi_get_node_version(napi_env env, const napi_node_version** version); +napi_get_node_version(node_api_nogc_env env, const napi_node_version** version); #if NAPI_VERSION >= 2 // Return the current libuv event loop for a given environment NAPI_EXTERN napi_status NAPI_CDECL -napi_get_uv_event_loop(napi_env env, struct uv_loop_s** loop); +napi_get_uv_event_loop(node_api_nogc_env env, struct uv_loop_s** loop); #endif // NAPI_VERSION >= 2 @@ -181,11 +181,11 @@ napi_get_uv_event_loop(napi_env env, struct uv_loop_s** loop); NAPI_EXTERN napi_status NAPI_CDECL napi_fatal_exception(napi_env env, napi_value err); -NAPI_EXTERN napi_status NAPI_CDECL -napi_add_env_cleanup_hook(napi_env env, napi_cleanup_hook fun, void* arg); +NAPI_EXTERN napi_status NAPI_CDECL napi_add_env_cleanup_hook( + node_api_nogc_env env, napi_cleanup_hook fun, void* arg); -NAPI_EXTERN napi_status NAPI_CDECL -napi_remove_env_cleanup_hook(napi_env env, napi_cleanup_hook fun, void* arg); +NAPI_EXTERN napi_status NAPI_CDECL napi_remove_env_cleanup_hook( + node_api_nogc_env env, napi_cleanup_hook fun, void* arg); NAPI_EXTERN napi_status NAPI_CDECL napi_open_callback_scope(napi_env env, @@ -209,7 +209,7 @@ napi_create_threadsafe_function(napi_env env, size_t max_queue_size, size_t initial_thread_count, void* thread_finalize_data, - napi_finalize thread_finalize_cb, + node_api_nogc_finalize thread_finalize_cb, void* context, napi_threadsafe_function_call_js call_js_cb, napi_threadsafe_function* result); @@ -228,18 +228,18 @@ napi_acquire_threadsafe_function(napi_threadsafe_function func); NAPI_EXTERN napi_status NAPI_CDECL napi_release_threadsafe_function( napi_threadsafe_function func, napi_threadsafe_function_release_mode mode); -NAPI_EXTERN napi_status NAPI_CDECL -napi_unref_threadsafe_function(napi_env env, napi_threadsafe_function func); +NAPI_EXTERN napi_status NAPI_CDECL napi_unref_threadsafe_function( + node_api_nogc_env env, napi_threadsafe_function func); -NAPI_EXTERN napi_status NAPI_CDECL -napi_ref_threadsafe_function(napi_env env, napi_threadsafe_function func); +NAPI_EXTERN napi_status NAPI_CDECL napi_ref_threadsafe_function( + node_api_nogc_env env, napi_threadsafe_function func); #endif // NAPI_VERSION >= 4 #if NAPI_VERSION >= 8 NAPI_EXTERN napi_status NAPI_CDECL -napi_add_async_cleanup_hook(napi_env env, +napi_add_async_cleanup_hook(node_api_nogc_env env, napi_async_cleanup_hook hook, void* arg, napi_async_cleanup_hook_handle* remove_handle); @@ -252,7 +252,7 @@ napi_remove_async_cleanup_hook(napi_async_cleanup_hook_handle remove_handle); #if NAPI_VERSION >= 9 NAPI_EXTERN napi_status NAPI_CDECL -node_api_get_module_file_name(napi_env env, const char** result); +node_api_get_module_file_name(node_api_nogc_env env, const char** result); #endif // NAPI_VERSION >= 9 diff --git a/test/js-native-api/common.h b/test/js-native-api/common.h index fc6f4cb6c2e1db..1308088e7872fb 100644 --- a/test/js-native-api/common.h +++ b/test/js-native-api/common.h @@ -2,6 +2,7 @@ #define JS_NATIVE_API_COMMON_H_ #include +#include // abort() // Empty value so that macros here are able to return NULL or void #define NODE_API_RETVAL_NOTHING // Intentionally blank #define @@ -22,6 +23,19 @@ } \ } while (0) +// The nogc version of GET_AND_THROW_LAST_ERROR. We cannot access any +// exceptions and we cannot fail by way of JS exception, so we abort. +#define FATALLY_FAIL_WITH_LAST_ERROR(env) \ + do { \ + const napi_extended_error_info* error_info; \ + napi_get_last_error_info((env), &error_info); \ + const char* err_message = error_info->error_message; \ + const char* error_message = \ + err_message != NULL ? err_message : "empty error message"; \ + fprintf(stderr, "%s\n", error_message); \ + abort(); \ + } while (0) + #define NODE_API_ASSERT_BASE(env, assertion, message, ret_val) \ do { \ if (!(assertion)) { \ @@ -33,6 +47,15 @@ } \ } while (0) +#define NODE_API_NOGC_ASSERT_BASE(assertion, message, ret_val) \ + do { \ + if (!(assertion)) { \ + fprintf(stderr, "assertion (" #assertion ") failed: " message); \ + abort(); \ + return ret_val; \ + } \ + } while (0) + // Returns NULL on failed assertion. // This is meant to be used inside napi_callback methods. #define NODE_API_ASSERT(env, assertion, message) \ @@ -43,6 +66,9 @@ #define NODE_API_ASSERT_RETURN_VOID(env, assertion, message) \ NODE_API_ASSERT_BASE(env, assertion, message, NODE_API_RETVAL_NOTHING) +#define NODE_API_NOGC_ASSERT_RETURN_VOID(assertion, message) \ + NODE_API_NOGC_ASSERT_BASE(assertion, message, NODE_API_RETVAL_NOTHING) + #define NODE_API_CALL_BASE(env, the_call, ret_val) \ do { \ if ((the_call) != napi_ok) { \ @@ -51,6 +77,14 @@ } \ } while (0) +#define NODE_API_NOGC_CALL_BASE(env, the_call, ret_val) \ + do { \ + if ((the_call) != napi_ok) { \ + FATALLY_FAIL_WITH_LAST_ERROR((env)); \ + return ret_val; \ + } \ + } while (0) + // Returns NULL if the_call doesn't return napi_ok. #define NODE_API_CALL(env, the_call) \ NODE_API_CALL_BASE(env, the_call, NULL) @@ -59,6 +93,9 @@ #define NODE_API_CALL_RETURN_VOID(env, the_call) \ NODE_API_CALL_BASE(env, the_call, NODE_API_RETVAL_NOTHING) +#define NODE_API_NOGC_CALL_RETURN_VOID(env, the_call) \ + NODE_API_NOGC_CALL_BASE(env, the_call, NODE_API_RETVAL_NOTHING) + #define NODE_API_CHECK_STATUS(the_call) \ do { \ napi_status status = (the_call); \ diff --git a/test/js-native-api/test_cannot_run_js/test_cannot_run_js.c b/test/js-native-api/test_cannot_run_js/test_cannot_run_js.c index 2cd25823c924c0..c495f8780d0141 100644 --- a/test/js-native-api/test_cannot_run_js/test_cannot_run_js.c +++ b/test/js-native-api/test_cannot_run_js/test_cannot_run_js.c @@ -20,6 +20,15 @@ static void Finalize(napi_env env, void* data, void* hint) { free(ref); } +static void NogcFinalize(node_api_nogc_env env, void* data, void* hint) { +#ifdef NAPI_EXPERIMENTAL + NODE_API_NOGC_CALL_RETURN_VOID( + env, node_api_post_finalizer(env, Finalize, data, hint)); +#else + Finalize(env, data, hint); +#endif +} + static napi_value CreateRef(napi_env env, napi_callback_info info) { size_t argc = 1; napi_value cb; @@ -30,7 +39,7 @@ static napi_value CreateRef(napi_env env, napi_callback_info info) { NODE_API_CALL(env, napi_typeof(env, cb, &value_type)); NODE_API_ASSERT( env, value_type == napi_function, "argument must be function"); - NODE_API_CALL(env, napi_add_finalizer(env, cb, ref, Finalize, NULL, ref)); + NODE_API_CALL(env, napi_add_finalizer(env, cb, ref, NogcFinalize, NULL, ref)); return cb; } diff --git a/test/js-native-api/test_finalizer/test_finalizer.c b/test/js-native-api/test_finalizer/test_finalizer.c index 378781b7042f96..b9b046484a5288 100644 --- a/test/js-native-api/test_finalizer/test_finalizer.c +++ b/test/js-native-api/test_finalizer/test_finalizer.c @@ -11,17 +11,17 @@ typedef struct { napi_ref js_func; } FinalizerData; -static void finalizerOnlyCallback(napi_env env, +static void finalizerOnlyCallback(node_api_nogc_env env, void* finalize_data, void* finalize_hint) { FinalizerData* data = (FinalizerData*)finalize_data; int32_t count = ++data->finalize_count; // It is safe to access instance data - NODE_API_CALL_RETURN_VOID(env, napi_get_instance_data(env, (void**)&data)); - NODE_API_ASSERT_RETURN_VOID(env, - count = data->finalize_count, - "Expected to be the same FinalizerData"); + NODE_API_NOGC_CALL_RETURN_VOID(env, + napi_get_instance_data(env, (void**)&data)); + NODE_API_NOGC_ASSERT_RETURN_VOID(count = data->finalize_count, + "Expected to be the same FinalizerData"); } static void finalizerCallingJSCallback(napi_env env, @@ -40,18 +40,20 @@ static void finalizerCallingJSCallback(napi_env env, } // Schedule async finalizer to run JavaScript-touching code. -static void finalizerWithJSCallback(napi_env env, +static void finalizerWithJSCallback(node_api_nogc_env env, void* finalize_data, void* finalize_hint) { - NODE_API_CALL_RETURN_VOID( + NODE_API_NOGC_CALL_RETURN_VOID( env, node_api_post_finalizer( env, finalizerCallingJSCallback, finalize_data, finalize_hint)); } -static void finalizerWithFailedJSCallback(napi_env env, +static void finalizerWithFailedJSCallback(node_api_nogc_env nogc_env, void* finalize_data, void* finalize_hint) { + // Intentionally cast to a napi_env to test the fatal failure. + napi_env env = (napi_env)nogc_env; napi_value obj; FinalizerData* data = (FinalizerData*)finalize_data; ++data->finalize_count; diff --git a/test/js-native-api/test_string/test_string.c b/test/js-native-api/test_string/test_string.c index b2046e3b873392..e18aa04ad1c5e2 100644 --- a/test/js-native-api/test_string/test_string.c +++ b/test/js-native-api/test_string/test_string.c @@ -88,7 +88,7 @@ static napi_value TestTwoByteImpl(napi_env env, return output; } -static void free_string(napi_env env, void* data, void* hint) { +static void free_string(node_api_nogc_env env, void* data, void* hint) { free(data); } diff --git a/test/node-api/test_reference_by_node_api_version/test_reference_by_node_api_version.c b/test/node-api/test_reference_by_node_api_version/test_reference_by_node_api_version.c index ae4fa7cb0e8856..f9110303d2ded4 100644 --- a/test/node-api/test_reference_by_node_api_version/test_reference_by_node_api_version.c +++ b/test/node-api/test_reference_by_node_api_version/test_reference_by_node_api_version.c @@ -4,12 +4,12 @@ static uint32_t finalizeCount = 0; -static void FreeData(napi_env env, void* data, void* hint) { - NODE_API_ASSERT_RETURN_VOID(env, data != NULL, "Expects non-NULL data."); +static void FreeData(node_api_nogc_env env, void* data, void* hint) { + NODE_API_NOGC_ASSERT_RETURN_VOID(data != NULL, "Expects non-NULL data."); free(data); } -static void Finalize(napi_env env, void* data, void* hint) { +static void Finalize(node_api_nogc_env env, void* data, void* hint) { ++finalizeCount; } @@ -61,7 +61,7 @@ static napi_value ToUInt32Value(napi_env env, uint32_t value) { static napi_status InitRefArray(napi_env env) { // valueRefs array has one entry per napi_valuetype napi_ref* valueRefs = malloc(sizeof(napi_ref) * ((int)napi_bigint + 1)); - return napi_set_instance_data(env, valueRefs, &FreeData, NULL); + return napi_set_instance_data(env, valueRefs, (napi_finalize)&FreeData, NULL); } static napi_value CreateExternal(napi_env env, napi_callback_info info) { diff --git a/test/node-api/test_threadsafe_function/test_uncaught_exception.c b/test/node-api/test_threadsafe_function/test_uncaught_exception.c index f8499d4fe4d680..c5eb4271dd490e 100644 --- a/test/node-api/test_threadsafe_function/test_uncaught_exception.c +++ b/test/node-api/test_threadsafe_function/test_uncaught_exception.c @@ -16,6 +16,18 @@ static void ThreadSafeFunctionFinalize(napi_env env, NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, js_func_ref)); } +static void ThreadSafeFunctionNogcFinalize(node_api_nogc_env env, + void* data, + void* hint) { +#ifdef NAPI_EXPERIMENTAL + NODE_API_NOGC_CALL_RETURN_VOID( + env, + node_api_post_finalizer(env, ThreadSafeFunctionFinalize, data, hint)); +#else + ThreadSafeFunctionFinalize(env, data, hint); +#endif +} + // Testing calling into JavaScript static napi_value CallIntoModule(napi_env env, napi_callback_info info) { size_t argc = 4; @@ -34,7 +46,7 @@ static napi_value CallIntoModule(napi_env env, napi_callback_info info) { 0, 1, finalize_func, - ThreadSafeFunctionFinalize, + ThreadSafeFunctionNogcFinalize, NULL, NULL, &tsfn)); diff --git a/tools/build-addons.mjs b/tools/build-addons.mjs index a96c18435ddc03..dc30c549fa5fbf 100755 --- a/tools/build-addons.mjs +++ b/tools/build-addons.mjs @@ -58,6 +58,8 @@ const jobs = []; for await (const dirent of await fs.opendir(directory)) { if (dirent.isDirectory()) { jobs.push(() => buildAddon(path.join(directory, dirent.name))); + } else if (dirent.isFile() && dirent.name === 'binding.gyp') { + jobs.push(() => buildAddon(directory)); } } await parallel(jobs, parallelization);