`uvimpl::Work::AfterThreadPoolWork()` and the thread-safe function's
`DispatchOne()` and `Finalize()` opened an
`AsyncResource::CallbackScope` with only a `HandleScope`.
`InternalCallbackScope` expects the resource's environment context to
be entered and otherwise asserts that `Environment::GetCurrent(isolate)`
is that environment, so with two environments on one isolate an addon's
async work completion or thread-safe function call aborted the process
whenever the other environment's context was current when the loop ran
the callback.
Enter the node-api env's context first, as `CallFinalizer()` and the
zlib and WebCrypto thread pool callbacks already do. In `Finalize()` the
scope covers only the finalizer call, since `MaybeDelete()` can free
the env whose persistent handle `context()` returns.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65406
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>
This is a follow-up to 234c26cca3. The Node-API interface does
not allow us to enforce that values are stored in a specific location,
e.g. on the stack or not; however, V8 requires `Local<>` handles
to be stored on the stack.
To circumvent this restriction, we add the ability to the async handle
stack to store either `Local<>*` pointers or `Global<>*` pointers, with
Node-API making use of the latter.
PR-URL: https://github.com/nodejs/node/pull/59828
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
There is a CHECK_NOT_NULL check before dereferencing node_env on
line 710 in the "if" block, but there is no CHECK_NOT_NULL check before
dereferencing node_env on line 721. Maybe it makes sense to put
CHECK_NOT_NULL right after calling the Environment::GetCurrent function.
PR-URL: https://github.com/nodejs/node/pull/58459
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <midawson@redhat.com>
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>
OnFatalError and OOMErrorHandler will not return control flow to the
calling function.
node::FatalError is an alias of node::OnFatalError. Replace all the
callsites with node::OnFatalError instead.
PR-URL: https://github.com/nodejs/node/pull/47695
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
napi_create_external_arraybuffer creates the arraybuffer with
node::Buffer, which already marks the buffer as untransferable.
The napi_env__::mark_arraybuffer_as_untransferable is not used
anymore.
PR-URL: https://github.com/nodejs/node/pull/47557
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Generalize the finalizer's second pass callback to make it
cancellable and simplify the code around the second pass callback.
With this change, it is determined that Reference::Finalize or
RefBase::Finalize are called once, either from the env's shutdown,
or from the env's second pass callback.
All existing node-api js tests should pass without a touch. The
js_native_api cctest is no longer applicable with this change,
just removing it.
PR-URL: https://github.com/nodejs/node/pull/44141
Refs: https://github.com/nodejs/node/issues/44071
Reviewed-By: Michael Dawson <midawson@redhat.com>
This overload accepts the current Environment* as an argument, unlike
the other constructor, which accepts an Isolate*. This is useful because
we can pass the current Environment* directly instead of recomputing it
from the Isolate* inside the constructor.
Signed-off-by: Darshan Sen <darshan.sen@postman.com>
PR-URL: https://github.com/nodejs/node/pull/39768
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Invoke threadsafe_function during the same tick and avoid marshalling
costs between threads and/or churning event loop if either:
1. There's a queued call already
2. `Push()` is called while the main thread was running
threadsafe_function
PR-URL: https://github.com/nodejs/node/pull/38506
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Unlike JS-only modules, native add-ons are always associated with a
dynamic shared object from which they are loaded. Being able to
retrieve its absolute path is important to native-only add-ons, i.e.
add-ons that are not themselves being loaded from a JS-only module
located in the same package as the native add-on itself.
Currently, the file name is obtained at environment construction time
from the JS `module.filename`. Nevertheless, the presence of `module`
is not required, because the file name could also be passed in via a
private property added onto `exports` from the `process.dlopen`
binding.
As an attempt at future-proofing, the file name is provided as a URL,
i.e. prefixed with the `file://` protocol.
Fixes: https://github.com/nodejs/node-addon-api/issues/449
PR-URL: https://github.com/nodejs/node/pull/37195
Co-authored-by: Michael Dawson <mdawson@devrus.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Shelley Vohr