This commit addresses the following warning:
../src/node_api.cc:28:19: warning: 'mark_arraybuffer_as_untransferable'
overrides a member function but is not marked 'override'
[-Winconsistent-missing-override]
v8::Maybe<bool> mark_arraybuffer_as_untransferable(
^
../src/js_native_api_v8.h:42:27: note: overridden virtual function is here
virtual v8::Maybe<bool> mark_arraybuffer_as_untransferable(
PR-URL: https://github.com/nodejs/node/pull/30549
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Modify the native `SetImmediate()` functions to take generic C++
callables as arguments. This makes passing arguments to the callback
easier, and in particular, it allows passing `std::unique_ptr`s
directly, which in turn makes sure that the data they point to is
deleted if the `Environment` is torn down before the callback can run.
PR-URL: https://github.com/nodejs/node/pull/28704
Reviewed-By: James M Snell <jasnell@gmail.com>
The C++ land `node::FatalException()` is not in fact fatal anymore.
It gives the user a chance to handle the uncaught exception
globally by listening to the `uncaughtException` event. This patch
renames it to `TriggerUncaughtException` in C++ to avoid the confusion.
In addition rename the JS land handler to `onGlobalUncaughtException`
to reflect its purpose - we have to keep the alias
`process._fatalException` and use that for now since it has been
monkey-patchable in the user land.
This patch also
- Adds more comments to the global uncaught exception handling routine
- Puts a few other C++ error handling functions into the `errors`
namespace
- Moves error-handling-related bindings to the `errors` binding.
Refs: https://github.com/nodejs/node/commit/2b252acea47af3ebeac3d7e68277f015667264cc
PR-URL: https://github.com/nodejs/node/pull/28257
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
We have a test that verifies that JS execution from the Buffer
finalizer is accepted, and that errors thrown are passed
down synchronously.
However, since the finalizer executes during GC, this is behaviour is
fundamentally invalid and, for good reasons, disallowed by the
JS engine. This leaves us with the options of either finding a way
to allow JS execution from the callback, or explicitly forbidding it on
the N-API side as well.
This commit implements the former option, since it is the more
backwards-compatible one, in the sense that the current situation
sometimes appears to work as well and we should not break that
behaviour if we don’t have to, but rather try to actually make it
work reliably.
Since GC timing is largely unobservable anyway, this commit moves
the callback into a `SetImmediate()`, as we do elsewhere in the code,
and a second pass callback is not an easily implemented option,
as the API is supposed to wrap around Node’s `Buffer` API.
In this case, exceptions are handled like other uncaught exceptions.
Two tests have to be adjusted to account for the timing difference.
This is unfortunate, but unavoidable if we want to conform to the
JS engine API contract and keep all tests.
Fixes: https://github.com/nodejs/node/issues/26754
PR-URL: https://github.com/nodejs/node/pull/28082
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The presence of the inline definitions in node_internals.h can cause all
files that include node_internals.h to depend on util-inl.h, even if
they never use ThreadPoolWork. Whether this happens depends on the
toolchain, gcc will strip unused definitions, clang won't.
PR-URL: https://github.com/nodejs/node/pull/27755
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
This patch:
- Moves the C++ binding/addon related code out of
node_internals.h/node.cc and into dedicated files
node_binding.h/node_binding.cc, and only puts the code resued
by other files into the header.
- Introduce a node::binding namespace so that code exposed to
other files can be easily recognized.
PR-URL: https://github.com/nodejs/node/pull/24701
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Create a class `PersistentToLocal` which contains three methods,
`Strong`, `Weak`, and `Default`:
* `Strong` returns a `Local` from a strong persistent reference,
* `Weak` returns a `Local` from a weak persistent reference, and
* `Default` decides based on `IsWeak()` which of the above two to call.
These replace `node::StrongPersistentToLocal()`,
`node::WeakPersistentToLocal()`, and `node::PersistentToLocal()`,
respectively.
PR-URL: https://github.com/nodejs/node/pull/24276
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Move the following code into a new node_errors.cc file and
declare them in node_errors.h for clarity and make it possible
to include them with node_errors.h.
- AppendExceptionLine()
- DecorateErrorStack()
- FatalError()
- OnFatalError()
- PrintErrorString()
- FatalException()
- ReportException()
- FatalTryCatch
And move the following definitions (declared elsewhere than
node_errors.h) to node_errors.cc:
- Abort() (in util.h)
- Assert() (in util.h)
PR-URL: https://github.com/nodejs/node/pull/24058
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Because instances of `napi_env` are created on a per-global-object
basis and because since most N-API functions refer to builtin JS
objects, `napi_env` is essentially in 1:1 correspondence with
`v8::Context`.
This was not clear from the implementation by itself, but has
emerged from conversations with the N-API team.
This patch changes the `napi_env` implementation to:
- Actually store the `v8::Context` it represents.
- Provide more direct access to the `node::Environment`
to which the `Context` belongs.
- Do not store the `uv_loop_t*` explicitly, since it can be
inferred from the `node::Environment` and we actually
have an N-API method for that.
- Replace calls to `isolate->GetCurrentContext()` with
the more appropriate `napi_env` `Context`.
- Implement a better (although not perfect) way of cleaning
up `napi_env` instances.
PR-URL: https://github.com/nodejs/node/pull/23689
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Make `Environment::GetCurrent()` return `nullptr` if the current
`Context` is not a Node.js context, and for the relevant usage of
this function, either:
- Switch to the better `GetCurrent(args)` variant
- Turn functions in to no-ops where it makes sense
- Make it a `CHECK`, i.e. an API requirement, where it make sense
- Leave a `TODO` comment for verifying what, if anything, is to be done
PR-URL: https://github.com/nodejs/node/pull/22819
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Add `napi_add_finalizer()`, which provides the ability to attach data
to an arbitrary object and be notified when that object is garbage-
collected so as to have an opportunity to delete the data previously
attached.
This differs from `napi_wrap()` in that it does not use up the private
slot on the object, and is therefore neither removable, nor retrievable
after the call to `napi_add_finalizer()`. It is assumed that the data
is accessible by other means, yet it must be tied to the lifetime of
the object. This is the case for data passed to a dynamically created
function which is itself heap-allocated and must therefore be freed
along with the function.
Fixes: https://github.com/nodejs/abi-stable-node/issues/313
PR-URL: https://github.com/nodejs/node/pull/22244
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Bundle a `uv_async_t`, a `uv_idle_t`, a `uv_mutex_t`, a `uv_cond_t`,
and a `v8::Persistent<v8::Function>` to make it possible to call into JS
from another thread. The API accepts a void data pointer and a callback
which will be invoked on the loop thread and which will receive the
`napi_value` representing the JavaScript function to call so as to
perform the call into JS. The callback is run inside a
`node::CallbackScope`.
A `std::queue<void*>` is used to store calls from the secondary
threads, and an idle loop is started by the `uv_async_t` callback on the
loop thread to drain the queue, calling into JS with each item.
Items can be added to the queue blockingly or non-blockingly.
The thread-safe function can be referenced or unreferenced, with the
same semantics as libuv handles.
Re: https://github.com/nodejs/help/issues/1035
Re: https://github.com/nodejs/node/issues/20964
Fixes: https://github.com/nodejs/node/issues/13512
PR-URL: https://github.com/nodejs/node/pull/17887
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Added a new struct CallbackBundle to eliminate all
GetInternalField() calls.
The principle is to store all required data inside a C++ struct,
and then store the pointer in the JavaScript object. Before this
change, the required data are stored in the JavaScript object in
3 or 4 seperate pointers. For every napi fun call, 3 of them
have to be fetched out, which are 3 GetInternalField() calls;
after this change, the C++ struct will be directly fetched out
by using v8::External::Value(), which is faster.
Profiling data show that GetInternalField() is slow.
On an i7-4770K (3.50GHz) box, a C++ V8-binding fun call is 8 ns,
before this change, napi fun call is 36 ns; after this change,
napi fun call is 20 ns.
The above data are measured using a modified benchmark in
'benchmark/misc/function_call'. The modification adds an indicator
of the average time of a "chatty" napi fun call (max 50M runs).
This change will speed up chatty case 1.8x (overall), and will cut
down the delay of napi mechanism to approx. 0.5x.
Background: a simple C++ binding function (e.g. receiving little
from JS, doing little and returning little to JS) is called
'chatty' case for JS<-->C++ fun call routine.
This improvement also applies to getter/setter fun calls.
PR-URL: https://github.com/nodejs/node/pull/21072
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Currently the following warnings are displayed when compiling:
../src/node_api.cc:3380:8:
warning: 'AfterThreadPoolWork' overrides a member function but is not
marked 'override' [-Winconsistent-missing-override]
void AfterThreadPoolWork(int status) {
^
../src/node_internals.h:513:16: note: overridden virtual function is
here
virtual void AfterThreadPoolWork(int status) = 0;
^
1 warning generated.
../src/node_zlib.cc:220:8:
warning: 'DoThreadPoolWork' overrides a member function but is not
marked 'override' [-Winconsistent-missing-override]
void DoThreadPoolWork() {
^
../src/node_internals.h:512:16: note: overridden virtual function is
here
virtual void DoThreadPoolWork() = 0;
^
../src/node_zlib.cc:224:8:
warning: 'AfterThreadPoolWork' overrides a member function but is
not marked 'override' [-Winconsistent-missing-override]
void AfterThreadPoolWork(int status) {
^
../src/node_internals.h:513:16: note: overridden virtual function is
here
virtual void AfterThreadPoolWork(int status) = 0;
^
2 warnings generated.
This commit adds override to the functions.
PR-URL: https://github.com/nodejs/node/pull/20663
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Instead of using the libuv mechanism directly, provide an internal
`ThreadPoolWork` wrapper that takes care of increasing/decreasing
the waiting request counter.
PR-URL: https://github.com/nodejs/node/pull/19377
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
This adds pairs of methods to the `Environment` class and to public APIs
which can add and remove cleanup handlers.
Unlike `AtExit`, this API targets addon developers rather than
embedders, giving them (and Node’s internals) the ability to register
per-`Environment` cleanup work.
We may want to replace `AtExit` with this API at some point.
Many thanks for Stephen Belanger for reviewing the original version of
this commit in the Ayo.js project.
Refs: https://github.com/ayojs/ayo/pull/82
PR-URL: https://github.com/nodejs/node/pull/19377
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
cjihrig