Add `HandleScope`s to the public C++ API for embedders/addons,
since these methods create V8 handles that should not leak into
the outer scopes.
In particular, for some of the methods it was not clear from
the function signatures that these functions previously
needed to be invoked with a `HandleScope`.
PR-URL: https://github.com/nodejs/node/pull/24285
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Currently there are a number of compiler warnings like the following:
../binding.cc:6:41:
warning: 'NewFromUtf8' is deprecated:
Use maybe version [-Wdeprecated-declarations]
args.GetReturnValue().Set(v8::String::NewFromUtf8(isolate, "world"));
^
/node/deps/v8/include/v8.h:2883:10:
note: 'NewFromUtf8' has been explicitly marked deprecated here
static V8_DEPRECATE_SOON(
^
/node/deps/v8/include/v8config.h:341:29:
note: expanded from macro 'V8_DEPRECATE_SOON'
declarator __attribute__((deprecated(message)))
^
This commit updates the code to use the maybe versions.
PR-URL: https://github.com/nodejs/node/pull/24216
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Throw an exception instead of crashing when attempting to create
`Buffer` objects from a Context that is not associated with
a Node.js `Environment`.
Possible alternatives for the future might be just returning
a plain `Uint8Array`, or working on providing `Buffer` for all
`Context`s.
PR-URL: https://github.com/nodejs/node/pull/23938
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/23784
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Assertion arguments should have the first value be the actual value,
while the second value be the expected value.
PR-URL: https://github.com/nodejs/node/pull/23470
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
This commit removes `common.crashOnUnhandledRejection()` and adds
`common.disableCrashOnUnhandledRejection()`.
To reduce the risk of mistakes and make writing tests that involve
promises simpler, always install the unhandledRejection hook in tests
and provide a way to disable it for the rare cases where it's needed.
PR-URL: https://github.com/nodejs/node/pull/21849
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Introduces macros `NODE_MODULE_INITIALIZER` which expands to the name
of the special symbol that process.dlopen() will look for to initialize
an addon, and `NODE_MODULE_INIT()` which creates the boilerplate for
a context-aware module which can be loaded multiple times via the
special symbol mechanism.
Additionally, provides an example of using the new macro to construct
an addon which stores per-addon-instance data in a heap-allocated
structure that gets passed to each binding, rather than in a collection
of global static variables.
Re: https://github.com/nodejs/node/issues/21291#issuecomment-396729727
PR-URL: https://github.com/nodejs/node/pull/21318
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Refactor Timers to behave more similarly to Immediates by having
a single uv_timer_t handle which is stored on the Environment.
No longer expose timers in a public binding and instead make
it part of the internalBinding.
PR-URL: https://github.com/nodejs/node/pull/20894
Fixes: https://github.com/nodejs/node/issues/10154
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Right now, we crash the process if there are handles remaining
on the event loop when we exit (except for the main thread).
This does not provide a lot of information about causes, though;
in particular, we don’t show which handles are pending and
who own them.
This patch adds debug output to these cases to help with the
situation.
PR-URL: https://github.com/nodejs/node/pull/21238
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
`AsyncResource` is intended to be a base class, and since we don’t
know what API consumers will do with it in their own code,
it’s good practice to make its destructor virtual.
This should not be ABI-breaking since all class methods are inline.
PR-URL: https://github.com/nodejs/node/pull/20633
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
We need to look for a special symbol even if the module self-registers
when the module self-registers with the wrong version.
PR-URL: https://github.com/nodejs/node/pull/20186
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Currently, the following compiler warning is issued:
../binding.cc:26:27:
warning: 'Utf8Value' is deprecated [-Wdeprecated-declarations]
const String::Utf8Value filename(args[0]);
^
This commit updates the code to use the Isolate version.
PR-URL: https://github.com/nodejs/node/pull/19966
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The old error name and message were trying to be consistent with
ERR_BUFFER_TOO_LARGE but they were not really accurate.
The kStringMaxLength was measured in number of characters,
not number of bytes. The name ERR_STRING_TOO_LARGE also
seems a bit awkward. This patch tries to correct them before
they get released to users.
PR-URL: https://github.com/nodejs/node/pull/19864
Refs: https://github.com/nodejs/node/pull/19739
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Re-`require()` the addon after clearing its cache to ensure that it is
re-initialized via the well-known symbol.
PR-URL: https://github.com/nodejs/node/pull/19875
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Currently there are two compiler warnings generated from the addons test
callback-scope:
../binding.cc:44:10:
warning: 'Resolve' is deprecated [-Wdeprecated-declarations]
local->Resolve(v8::Undefined(isolate));
^
../../../../deps/v8/include/v8.h:3893:45:
note: 'Resolve' has been explicitly marked deprecated here
V8_DEPRECATED("Use maybe version", void Resolve(Local<Value> value));
^
../binding.cc:52:54:
warning: 'New' is deprecated [-Wdeprecated-declarations]
persistent.Reset(isolate, v8::Promise::Resolver::New(isolate));
^
../../../../deps/v8/include/v8.h:3880:42:
note: 'New' has been explicitly marked deprecated here
Local<Resolver> New(Isolate* isolate));
This commit updates the test to use non-deprecated functions.
PR-URL: https://github.com/nodejs/node/pull/19252
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Directory symlinks in Windows require the 'dir' flag to be passed to
create the symlink correctly.
PR-URL: https://github.com/nodejs/node/pull/19049
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Look for symbol `node_register_module_v${NODE_MODULE_VERSION}` if the
add-on didn't self-register. This can be used to create add-ons that
support multiple Node.js versions from a single shared object.
PR-URL: https://github.com/nodejs/node/pull/18934
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matheus Marchini <matheus@sthima.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Currently, the make-callback-domain-warning addon generates the
following warning:
../binding.cc:22:9:
warning: 'MakeCallback' is deprecated: Use MakeCallback(...,
async_context) [-Wdeprecated-declarations]
node::MakeCallback(isolate, recv, method, 0, nullptr);
^
../../../../src/node.h:172:50:
note: 'MakeCallback' has been explicitly marked
deprecated here
NODE_EXTERN v8::Local<v8::Value> MakeCallback(
^
1 warning generated.
This commit fixes this warning.
PR-URL: https://github.com/nodejs/node/pull/18877
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Add punctuation and comments about code that should not throw.
Also remove a obsolete test and refactor some tests.
PR-URL: https://github.com/nodejs/node/pull/18669
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
There is actually no reason to use `assert.doesNotThrow()` in the
tests. If a test throws, just let the error bubble up right away
instead of first catching it and then rethrowing it.
PR-URL: https://github.com/nodejs/node/pull/18669
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Users of MakeCallback that adds the domain property to carry context,
should start using the async_context variant of MakeCallback or the
AsyncResource class.
PR-URL: https://github.com/nodejs/node/pull/17417
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
For tests that use anonymous namespaces, some tagged the close
of the namespace with 'namespace' while others used
'anonymous namespace'. It was suggested I should use
'anonymous namespace' in a recent PR review so make all of the
tests consistent with this.
PR-URL: https://github.com/nodejs/node/pull/18583
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
I mistakenly introduced user defined variables using uppercase
characters, reading the gyp documentation they state:
"Predefined variables. By convention, these are named with
CAPITAL_LETTERS. Predefined variables are set automatically by GYP"
and also "By convention, user-defined variables are named with
lowercase_letters."
This commit renames the user defined variables to lowercase to follow
the above mentioned convention.
PR-URL: https://github.com/nodejs/node/pull/16238
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Move the majority of C++ domain-related code into JS land by introducing
a top level domain callback which handles entering & exiting the domain.
Move the rest of the domain necessities into their own file that creates
an internal binding, to avoid exposing domain-related code on the
process object.
Modify an existing test slightly to better test domain-related code.
PR-URL: https://github.com/nodejs/node/pull/18291
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
This reverts commit d9b59def72.
Breaks downloadable source tarball builds as we remove some files prior
to creating a tarball but those files are included in the comprehensive
list of dependencies listed in .deps.
Ref: https://github.com/nodejs/node/pull/17407
PR-URL: https://github.com/nodejs/node/pull/18287
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
* Get rid of recursive `make` when building the node binary. An earlier
commit makes GYP write out rules that we can use for proper dependency
tracking.
* Use module name 'binding' in addons.md and addons-napi/*/binding.gyp.
This massively simplifies the logic for generating the build rules.
* Check in auto-generated add-on tests from `doc/api/addons.md`. The
files change rarely and generating them dynamically causes no end of
race conditions and special-casing during the build.
PR-URL: https://github.com/nodejs/node/pull/17407
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Refine the static and shared lib build process in order
to integrate static and shared lib verfication into CI.
When building both static and shared lib, we still build
node executable now and it uses the shared and static lib.
Signed-off-by: Yihong Wang <yh.wang@ibm.com>
Fixes: https://github.com/nodejs/node/issues/14158
PR-URL: https://github.com/nodejs/node/pull/17604
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/17957
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Using port 0 to request an open port from the operating system is
sufficient in openssl-client-cert-engine/test.js.
PR-URL: https://github.com/nodejs/node/pull/17563
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/17247
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Add a utility functions for addons to use when they need
a reference to the current event loop.
Currently, `uv_default_loop()` works if the embedder is the
single-threaded default node executable, but even without
the presence of e.g. workers that might not really an API
guarantee for when Node is embedded.
PR-URL: https://github.com/nodejs/node/pull/17109
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
The motivation for this commit is to enable projects embedding Node.js
and building with --enable-static to be able to run the test suite and
linter.
Currently when building with --enable-static no node executable
will be created which means that the tests (apart from the cctest) and
linter cannot be run.
This is currently a work in progress and works on MacOS but I need to
run the CI, and manually on different environments to verify that it
works as expected.
PR-URL: https://github.com/nodejs/node/pull/14986
Refs: https://github.com/nodejs/node/issues/14158
Refs: https://github.com/nodejs/node/pull/14892
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Add an option 'clientCertEngine' to `tls.createSecureContext()` which gets
wired up to OpenSSL function `SSL_CTX_set_client_cert_engine`. The option
is passed through from `https.request()` as well. This allows using a custom
OpenSSL engine to provide the client certificate.
The custom message means that the values involved are not reported.
Remove the custom message for a more detailed error message.
PR-URL: https://github.com/nodejs/node/pull/16824
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
use `common.buildType` instead of
`process.config.target_defaults.default_configuration` in
repl-domain-abort addon test.
PR-URL: https://github.com/nodejs/node/pull/16538
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Update tests to use module name macro
PR-URL: https://github.com/nodejs/node/pull/16185
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
