PR-URL: https://github.com/nodejs/node/pull/17690


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: https://github.com/nodejs/node/pull/18607


Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Make sure that inspector ports in cluster are inside the valid range:
`[1024, 65535]`.
Fixes flaky `test-inspector-port-zero-cluster`.

PR-URL: https://github.com/nodejs/node/pull/18696
Fixes: https://github.com/nodejs/node/issues/18303


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: https://github.com/nodejs/node/pull/18893


Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/18908


Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.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>

Currently the following deprecation warning is produced when compiling
node_perf.cc:

./src/node_perf.cc:91:11:
warning: 'MakeCallback' is deprecated: Use MakeCallback(...,
      async_context) [-Wdeprecated-declarations]
    node::MakeCallback(env->isolate(),
          ^
../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 adds an async_context to the call and checks the maybe
result.

PR-URL: https://github.com/nodejs/node/pull/18877


Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>

PR-URL: https://github.com/nodejs/node/pull/18875


Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

When building the node with `--shared` option, we need
to verify the symbols in shared lib instead of executable.

Refs: https://github.com/nodejs/node/issues/18535



Signed-off-by: Yihong Wang <yh.wang@ibm.com>
PR-URL: https://github.com/nodejs/node/pull/18806
Refs: https://github.com/nodejs/node/issues/18535


Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

PR-URL: https://github.com/nodejs/node/pull/18854


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Shutting down the connection is what `_final` is there for.

PR-URL: https://github.com/nodejs/node/pull/18608


Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Don't try to update the internal field pointer of the JS object in the
destructor.

The garbage collector invokes the destructor when the object is
collected and is not necessarily in a valid state anymore.

PR-URL: https://github.com/nodejs/node/pull/18656


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

The previous commit made persistent handles auto-reset on destruction.
This commit removes the Reset() calls that are now no longer necessary.

PR-URL: https://github.com/nodejs/node/pull/18656


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Replace v8::Persistent with node::Persistent, a specialization that
resets the persistent handle on destruction.  Prevents accidental
resource leaks when forgetting to call .Reset() manually.

I'm fairly confident this commit fixes a number of resource leaks that
have gone undiagnosed so far.

PR-URL: https://github.com/nodejs/node/pull/18656


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/18847


Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matheus Marchini <matheus@sthima.com>

Note that the CI run for the exercise can be minimal.

PR-URL: https://github.com/nodejs/node/pull/18846


Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matheus Marchini <matheus@sthima.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

* Sync format schemes with the current doc state.
* Lowercase primitive types.
* Fix typos and unify the style.
* Remove tautological info.

PR-URL: https://github.com/nodejs/node/pull/18874


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/18566


Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>

TTY tests should almost never be placed in `/parallel/`. Skipping TTY
tests there due to missing tty fds just means they will never be run,
ever, on any system.

This moves the tty-get-color-depth test to `/pseudo-tty/` where the test
runner will actually make a pty fd.

Refs: https://github.com/nodejs/node/pull/17615
PR-URL: https://github.com/nodejs/node/pull/18800


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

This utility is fairly generic and likely useful for more than one test.

PR-URL: https://github.com/nodejs/node/pull/18800


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

PR-URL: https://github.com/nodejs/node/pull/18850


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/18849


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/18845


Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Refs: https://github.com/nodejs/node/issues/11135

PR-URL: https://github.com/nodejs/node/pull/18843


Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matheus Marchini <matheus@sthima.com>

PR-URL: https://github.com/nodejs/node/pull/18873


Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

PR-URL: https://github.com/nodejs/node/pull/18872


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

When using a debug build (on Windows specifically) the error case for
tls_wrap causes an assert to fire because the index being passed is
outside the bounds of the vector.

The fix is to switch to iterators.

PR-URL: https://github.com/nodejs/node/pull/18830


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

There are currently 3 places in Timers where the exact same code
appears. Instead create a helper function that does the same job
of setting asyncId & triggerAsyncId, as well as calling emitInit.

PR-URL: https://github.com/nodejs/node/pull/18825


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Currently the cctest target depend on the node_core_target_name
target. But it is the node_lib_target_name target that compiles the
sources now which means that if a source file in src is updated the
cctest executable will not be re-linked against it, but will remain
unchanged. The code will still be compiled, just not linked which
means that if you are debugging you'll not see the changes and also a
warning will be displayed about this issue.

This commit changes the cctest target to depend on node_lib_target_name.

PR-URL: https://github.com/nodejs/node/pull/18576


Reviewed-By: Matheus Marchini <matheus@sthima.com>
Reviewed-By: Yihong Wang <yh.wang@ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

PR-URL: https://github.com/nodejs/node/pull/18780


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/18780


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/18780


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Currenlty when configuring --debug-http2
/test/parallel/test-http2-getpackedsettings.js will segment fault:

$ out/Debug/node test/parallel/test-http2-getpackedsettings.js
Segmentation fault: 11

This is happening because the settings is created with the Environment in
PackSettings:
Http2Session::Http2Settings settings(env);
This will cause the session to be set to nullptr. When the init
function is later called the expanded DEBUG_HTTP2SESSION macro will
cause the segment fault when the session is dereferenced.

PR-URL: https://github.com/nodejs/node/pull/18815


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

When the async uv_fs_* call errors out synchronously in AsyncDestCall,
the after callbacks (e.g. AfterNoArgs) would delete the req_wrap
in FSReqAfterScope, and AsyncDestCall would set those req_wrap to
nullptr afterwards. But when it returns to the top-layer bindings,
the bindings all call `req_wrap->SetReturnValue()` again without
checking if `req_wrap` is nullptr, causing a segfault.

This has not been caught in any of the tests because we usually do a
lot of argument checking in the JS layer before invoking the uv_fs_*
functions, so it's rare to get a synchronous error from them.

Currently we never need the binding to return the wrap to JS layer,
so we can just call `req_wrap->SetReturnValue()` to return undefined
for normal FSReqWrap and the promise for FSReqPromise in AsyncDestCall
instead of doing this in the top-level bindings.

PR-URL: https://github.com/nodejs/node/pull/18811


Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/18739


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matheus Marchini <matheus@sthima.com>

1. Adds fixer method
2. Extends test

PR-URL: https://github.com/nodejs/node/pull/16646
Refs: https://github.com/nodejs/node/issues/16636


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

1. Fixer for crypto-check.js
2. Extends tests

PR-URL: https://github.com/nodejs/node/pull/16647
Refs: https://github.com/nodejs/node/issues/16636


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

This brings the behaviour in line with the documentation.

PR-URL: https://github.com/nodejs/node/pull/16944


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>

Use arrow functions and common.mustCall() and add a description.

PR-URL: https://github.com/nodejs/node/pull/18714


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>

Like man(7), mdoc(7) is a macro package for marking up computer manuals.
The main difference is that mdoc is semantic rather than presentational,
providing authors with a full DSL to abstract page markup from low-level
Roff commands. By contrast, `man` is minimalist and leaves formatting to
the author, who is expected to have a working amount of Roff knowledge.

Therefore the use of `mdoc` for marking up Node's manpage is a decidedly
better choice than bare `man`. Less room is left for error and mandoc(1)
offers very robust error-checking and linting.

PR-URL: https://github.com/nodejs/node/pull/18559


Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>