This fixes a resource leak detected by a Coverity scan.
PR-URL: https://github.com/nodejs/node/pull/13198
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Aleksei Koziatinskii <ak239spb@gmail.com>
`AsyncEvent` is not a good name given its semantics.
PR-URL: https://github.com/nodejs/node/pull/13192
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
In Writable, 'finish' was not emitted when using writev() and
cork() in the event of an Error during the write. This commit
makes it consistent with the write() path, which emits 'finish'.
Fixes: https://github.com/nodejs/node/issues/11121
PR-URL: https://github.com/nodejs/node/pull/13195
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Calvin Metcalf <calvin.metcalf@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/13182
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Original commit message:
Allow embedder to set promise internal field count
Asynchronous context tracking mechanisms in Node.js need to store some
state on all promise objects. This change will allow embedders to
configure the number of internal fields on promises as is already done
for ArrayBuffers.
BUG=v8:6435
Review-Url: https://codereview.chromium.org/2889863002
Cr-Commit-Position: refs/heads/master@{#45496}
PR-URL: https://github.com/nodejs/node/pull/13175
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Use `common.mustNotCall()` to confirm that noop callbacks are not run
when functions throw errors.
PR-URL: https://github.com/nodejs/node/pull/13183
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
root_cert_store is defined as extern in node_crypto.h but only used in
node_crypto.cc. It is then set using SSL_CTX_set_cert_store. The only
usages of SSL_CTX_get_cert_store are in node_crypto.cc which would all
be accessing the same X509_STORE through the root_cert_store pointer as
far as I can tell. Am I missing something here?
This commit suggests removing it from the header and making it static
in node_crypto.cc.
PR-URL: https://github.com/nodejs/node/pull/13194
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
If node is configured --without-inspector/--without-ssl this test will
fail with the following error:
Path: inspector/test-bindings
inspector.js:8
throw new Error('Inspector is not available');
^
Error: Inspector is not available
at inspector.js:8:9
at NativeModule.compile (bootstrap_node.js:549:7)
This commit skips this test if the inspector is disabled.
PR-URL: https://github.com/nodejs/node/pull/13186
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@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: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Add test to cover napi_has_named_property
PR-URL: https://github.com/nodejs/node/pull/13178
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
Make the style of "Note:" paragraphs consistent and document the
guidelines in `doc/STYLE_GUIDE.md`.
PR-URL: https://github.com/nodejs/node/pull/13133
Fixes: https://github.com/nodejs/node/issues/13131
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Use common.mustNotCall() to confirm that listener handles never run (as
no events are emitted).
PR-URL: https://github.com/nodejs/node/pull/13165
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
* add block-scoping
* use common.mustCall() on callbacks that should not execute
PR-URL: https://github.com/nodejs/node/pull/13164
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/13174
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
This commit adds test coverage for the case where a dgram socket
successfully binds, but the handle's send() function fails.
PR-URL: https://github.com/nodejs/node/pull/13158
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This commit attempts to fix the following TODO:
// TODO(bnoordhuis) Should perhaps also check whether
getauxval(AT_SECURE) is non-zero on Linux.
This can be manually tested at the moment using the following steps:
$ setcap cap_net_raw+ep out/Release/node
$ NODE_PENDING_DEPRECATION="1" out/Release/node -p
"process.binding('config').pendingDeprecation"
true
$ useradd test
$ su test
$ NODE_PENDING_DEPRECATION="1" out/Release/node -p
"process.binding('config').pendingDeprecation"
undefined
PR-URL: https://github.com/nodejs/node/pull/12548
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This commit attempts to fix an issue when building on windows using the
following command line options:
.\vcbuild.bat dll debug x64 vc2015
This will result in the following options passed to configure:
configure --debug --shared --dest-cpu=x64 --tag=
This commit excludes the dependency to openssl if node is configured
with --shared.
Also, FP_API to the categories to export in mkssldef when generating
the module definition (openssl.def) allowing the build to compile and
link successfully.
Fixes: https://github.com/nodejs/node/issues/12952
PR-URL: https://github.com/nodejs/node/pull/13078
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Refael Ackermann <refack@gmail.com>
This change provides unified tracking of asynchronous promise lifecycles
for both domains and async hooks.
PR-URL: https://github.com/nodejs/node/pull/13000
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: https://github.com/nodejs/node/pull/13177
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Original commit message:
[turbofan] Boost performance of Array.prototype.shift by 4x.
For small arrays, it's way faster to just move the elements instead of
doing the fairly complex and heavy-weight left-trimming. Crankshaft has
had this optimization for small arrays already; this CL more or less
ports this functionality to TurboFan, which yields a 4x speed-up when
using shift on small arrays (with up to 16 elements).
This should recover some of the regressions reported in the Node.js issues
https://github.com/nodejs/node/issues/12657
and discovered for the syncthrough module using
https://github.com/mcollina/syncthrough/blob/master/benchmarks/basic.js
as benchmark.
R=jarin@chromium.org
BUG=v8:6376
Review-Url: https://codereview.chromium.org/2874453002
Cr-Commit-Position: refs/heads/master@{#45216}
PR-URL: https://github.com/nodejs/node/pull/13162
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit adds coverage for several edge cases related to
util.format() format string placeholders.
PR-URL: https://github.com/nodejs/node/pull/13159
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Currently the following compiler warning is displayed when building:
../test/cctest/test_inspector_socket_server.cc:142:8: warning:
'ServerDone' overrides a member function but is not marked 'override'
[-Winconsistent-missing-override]
void ServerDone() {
^
../src/inspector_socket_server.h:30:16: note: overridden virtual
function is here
virtual void ServerDone() = 0;
^
This commit marks ServerDone with override to get rid of the warning.
PR-URL: https://github.com/nodejs/node/pull/13166
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>
PR-URL: https://github.com/nodejs/node/pull/13216
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
* track callback invocations with common.mustCall() and
common.mustNotCall()
* remove test in test/internet/test-dns.js that is duplicated in
test/parallel/test-dns.js
* move tests that might perform a DNS query from test
test/parallel/test-dns.js to test/internet/test-dns.js
PR-URL: https://github.com/nodejs/node/pull/13163
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Add `common.refreshTmpDir()` before using the tmp directory.
fs.writeSync no longer requires an integer for the position argument, so
change test from `assert.throws()` to `assert.doesNotThrow()`. The test
now passes.
Because it involves a 5 GB file, we're not going to activate the test,
although that is possible if we add checking for appropriate available
resources (definitely disk space, likely memory, maybe check that it's a
64-bit OS).
PR-URL: https://github.com/nodejs/node/pull/13147
Reviewed-By: James M Snell <jasnell@gmail.com>
Document --inspect-port, and fix the reporting for when it is misused.
The option requires an argument, but when the argument was omitted, the
error message incorrectly reported --inspect-port as being bad, as if
was not supported at all:
% node --inspect-port
node: bad option: --inspect-port
% node --none-such
node: bad option: --none-such
It is now correctly reported as requiring an argument:
% ./node --inspect-port
./node: --inspect-port requires an argument
PR-URL: https://github.com/nodejs/node/pull/12581
Reviewed-By: James M Snell <jasnell@gmail.com>
Really minor but I could not find an open PR for anything n-api where
this could be changed, so creating this so that it is not forgotten.
PR-URL: https://github.com/nodejs/node/pull/13190
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <mhdawson@ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Adds the ability to for write streams to have an _final method which acts
similarly to the _flush method that transform streams have but is called before
the finish event is emitted and if asynchronous delays the stream from
finishing. The `final` option may also be passed in order to set it.
PR-URL: https://github.com/nodejs/node/pull/12828
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Covert lib/dgram.js over to using lib/internal/errors.js
for generating Errors. See
[using-internal-errors.md](https://github.com/nodejs/node/blob/master/doc/guides/using-internal-errors.md)
for more details.
I have not addressed the cases that use errnoException() and
exceptionWithHostPort() helper methods as changing these would require
fixing the tests across all of the different files that use them. In
addition, these helpers already add a `code` to the Error and we'll
have to discuss how that interacts with the `code` used by
lib/internal/errors.js. I believe we should convert all users
of errnoException and exceptionWithHostPort in a PR dedicated to
that conversion.
PR-URL: https://github.com/nodejs/node/pull/12926
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben.bridgewater@fintura.de>
test-stream2-basic runs in a few seconds. It can be moved to parallel.
PR-URL: https://github.com/nodejs/node/pull/13146
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
In test/pummel/test-stream2-basic.js, check that noop functions are
called the expected number of times.
PR-URL: https://github.com/nodejs/node/pull/13146
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Fixed a regression that caused the callback passed to destroy()
to not be called if the stream was already destroyed.
This caused a regression on the ws module in CITGM introduced by
https://github.com/nodejs/node/pull/12925.
PR-URL: https://github.com/nodejs/node/pull/13156
Fixes: https://github.com/websockets/ws/issues/1118
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Calvin Metcalf <calvin.metcalf@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
* Wrap text at 80 characters.
* Use periods consistently.
PR-URL: https://github.com/nodejs/node/pull/13135
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <mhdawson@ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
This commit fixes an incorrect keyboard shortcut in
`doc/STYLE_GUIDE.md`: entering em-dashes is done via Alt+Shift+"-" on
macOS, not via Ctrl+Alt+"-". Besides that, Option is more canonical name
of Alt on Macs.
PR-URL: https://github.com/nodejs/node/pull/13134
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <mhdawson@ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Use common.mustCall() in test-fs-makeStatsCallback to confirm that the
callback is invoked.
PR-URL: https://github.com/nodejs/node/pull/13132
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <mhdawson@ibm.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
When a no-op message event handler is used in a test, make it clear what
is expected by using `common.mustCall()` and `common.mustNotCall()`.
PR-URL: https://github.com/nodejs/node/pull/13125
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <mhdawson@ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Add tests to validate that properties marked as static
are available through the class as opposed to instances
PR-URL: https://github.com/nodejs/node/pull/13124
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Hitesh Kanwathirtha <digitalinfinity@gmail.com>
Fix mismatch in title for napi_get_value_string_utf16
Fixes: https://github.com/nodejs/abi-stable-node/issues/243
PR-URL: https://github.com/nodejs/node/pull/13123
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This fixes a race condition in the watchdog timer used for vm timeouts.
The condition would terminate the main stack's execution instead of the
code running under the sandbox.
PR-URL: https://github.com/nodejs/node/pull/13074
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This option has been broken for almost a year when used with any of the
vm.runIn.. family of functions, except for syntax errors.
PR-URL: https://github.com/nodejs/node/pull/13074
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Eugene Ostroukhov
Anna Henningsen
Matteo Collina
Brian White
Vse Mozhet Byt
Matt Loring
Rich Trott
Daniel Bevenius
Michael Dawson
Alexey Orlenko
Ebrahim Byagowi
Daniel Pihlstrom
cjihrig
Timothy Gu
Sam Roberts
Ali Ijaz Sheikh
Calvin Metcalf
Jason Ginchereau
Michael Dawson
Marcel Laverdet