`node_napi_env__::New()` returns nullptr after throwing when an add-on
declares a Node-API version this binary does not support, but
`napi_module_register_by_symbol()` dereferenced the result without
checking it. Loading such an add-on segfaulted instead of surfacing the
error the version check had already produced, so `require()` could not
catch it.
Reproduced on v20.x, v22.x, v24.x and v26.8.2 with a ten-line add-on
whose only distinguishing content is a NAPI_VERSION above
NODE_API_SUPPORTED_VERSION_MAX. `main`, `v22.x-staging` and
`v24.x-staging` all lack the check.
The error path had no test coverage: the message text appears in exactly
one file in the repository, `src/node_api.cc`. A test is added next to
`test_null_init`, which covers the sibling early return in the same
function.
Prepared with assistance from a closed-source coding agent, named in the
pull request description. The design, review and validation are my own:
I verified the fix and the test against a local build, including
removing the four added lines and relinking to confirm the test fails
without them.
Refs: https://github.com/nodejs/node/issues/57233
Signed-off-by: Alexey Karimov <krassx@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66019
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Port the domain module from createHook (async_hooks) to
AsyncLocalStorage using the AsyncContextFrame-based implementation.
Key changes:
- Use AsyncLocalStorage for domain context propagation instead of
async_hooks.createHook()
- Lazy initialization that triggers AsyncContextFrame prototype swap
on first domain use
- Use enterWith instead of ALS.run() so domain context is NOT
automatically restored on exception - this matches the original
domain.run() behavior where exit() only runs on success
- Add ERR_ASYNC_RESOURCE_DOMAIN_REMOVED error for AsyncResource.domain
- Update DEP0097 to End-of-Life status
- Remove tests that relied on the removed MakeCallback domain property
The domain module now uses the AsyncContextFrame version of
AsyncLocalStorage directly for proper context propagation across
async boundaries.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/61095
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Instead of copying and writing files on the fly for SEA tests,
put the fixtures into a directory and copy them into tmpdir
for testing. This allows easier reproduction and debugging
when they do fail - we can just copy the entire fixture directory
and test directly from there.
PR-URL: https://github.com/nodejs/node/pull/61167
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Other threads can still hold a valid handle to the tsfn after
finalization if finalization was triggered by
- release with napi_tsfn_abort, or
- environment shutdown
Handle this by:
- protecting finalization itself with the mutex
- if necessary, delay deletion after finalization to when thread_count
drops to 0
- releasing all resources as soon as possible before deletion
Fixes: https://github.com/nodejs/node/issues/55706
PR-URL: https://github.com/nodejs/node/pull/55877
Co-Authored-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
There already is an existing requirement to have matching calls of
`napi_async_init()` and `napi_async_destroy()`, so expecting users
of this API to manually hold onto the resource for the duration of
the `napi_async_context`'s lifetime is unnecessary.
Weak callbacks are generally useful for when a corresponding C++
object should be cleaned up when a JS object is gargbage-collected,
but that is not the case here.
PR-URL: https://github.com/nodejs/node/pull/59828
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
The thread-safe function's finalizer is not called in conjunction with
the garbage collection of a JS value. In fact, it keeps a strong
reference to the JS function it is expected to call. Thus, it is safe
to make calls that affect GC state from its body.
PR-URL: https://github.com/nodejs/node/pull/51801
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
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 <legendecas@gmail.com>
Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
This (not particularly elegant) native addon tests the effect of
UV_THREADPOOL_SIZE on node-api. The test fails if Node.js allows more
than UV_THREADPOOL_SIZE async tasks to run concurrently, or if it limits
the number of concurrent async tasks to anything less than
UV_THREADPOOL_SIZE.
PR-URL: https://github.com/nodejs/node/pull/49165
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Introduce APIs that allow for the creation of JavaScript strings without
copying the underlying native string into the engine. The APIs fall back
to regular string creation if the engine's external string APIs are
unavailable. In this case, an optional boolean out-parameter indicates
that the string was copied, and the optional finalizer is called if
given.
PR-URL: https://github.com/nodejs/node/pull/48339
Fixes: https://github.com/nodejs/node/issues/48198
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Signed-off-by: Gabriel Schulhof <gabrielschulhof@gmail.com>
The HTML StructuredSerializeWithTransfer algorithm defines that when
an untransferable object is in the transfer list, a DataCloneError is
thrown.
An array buffer that is already transferred is also considered as
untransferable.
PR-URL: https://github.com/nodejs/node/pull/47604
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
The term "native module" dates back to some of the oldest code
in the code base. Within the context of Node.js core it usually
refers to modules that are native to Node.js (e.g. fs, http),
but it can cause confusion for people who don't work on this
part of the code base, as "native module" can also refer to
native addons - which is even the case in some of the API
docs and error messages.
This patch tries to make the usage of these terms more consistent.
Now within the context of Node.js core:
- JavaScript scripts that are built-in to Node.js are now referred
to as "built-in(s)". If they are available as modules,
they can also be referred to as "built-in module(s)".
- Dynamically-linked shared objects that are loaded into
the Node.js processes are referred to as "addons".
We will try to avoid using the term "native modules" because it could
be ambiguous.
Changes in this patch:
File names:
- node_native_module.h -> node_builtins.h,
- node_native_module.cc -> node_builtins.cc
C++ binding names:
- `native_module` -> `builtins`
`node::Environment`:
- `native_modules_without_cache` -> `builtins_without_cache`
- `native_modules_with_cache` -> `builtins_with_cache`
- `native_modules_in_snapshot` -> `builtins_in_cache`
- `native_module_require` -> `builtin_module_require`
`node::EnvSerializeInfo`:
- `native_modules` -> `builtins
`node::native_module::NativeModuleLoader`:
- `native_module` namespace -> `builtins` namespace
- `NativeModuleLoader` -> `BuiltinLoader`
- `NativeModuleRecordMap` -> `BuiltinSourceMap`
- `NativeModuleCacheMap` -> `BuiltinCodeCacheMap`
- `ModuleIds` -> `BuiltinIds`
- `ModuleCategories` -> `BuiltinCategories`
- `LoadBuiltinModuleSource` -> `LoadBuiltinSource`
`loader.js`:
- `NativeModule` -> `BuiltinModule` (the `NativeModule` name used in
`process.moduleLoadList` is kept for compatibility)
And other clarifications in the documentation and comments.
PR-URL: https://github.com/nodejs/node/pull/44135
Fixes: https://github.com/nodejs/node/issues/44036
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Antoine du Hamel