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


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

This commit removes the builtin async_wrap module from
lib/async_hooks.js.

The motivation for this is that lib/async_hooks.js requires
lib/internal/async_hooks which also binds async_wrap. Instead of
lib/async_hooks.js also binding async_wrap it now only has to require
the internal async_hooks and access it's exports.

There might be a very good reason for doing it the current way but the
reason is not obvious to me. Hopefully someone can shed some light on
this.

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


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Currently, there is an AsyncWrap constructor that is only used by
PromiseWrap. This constructor has a body which is very similar
to the other AsyncWrap constructor.

This commit suggests updating the private constructor that is used
by PromiseWrap and also have the second constructor delegate to this
one to avoid the code duplication.

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


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

This reverts commit 1708af36.

Numbers are much more difficult to read in blue and it would be good
to have a consistent output throughout all OS.

PR-URL: https://github.com/nodejs/node/pull/19256
Refs: https://github.com/nodejs/node/pull/18925


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

82bdf8fb fixed an issue by silently modifying the `start`
option for the case when only `end` is passed, in order to perform
reads from a specified range in the file.

However, that approach does not work for non-seekable files, since
a numeric `start` option means that positioned reads will be used
to read data from the file.

This patch fixes that, and instead ends reading after a specified
size by adjusting the read buffer size.

This way we avoid re-introducing the bug that 82bdf8fb fixed,
and align behaviour with the native file stream mechanism
introduced in https://github.com/nodejs/node/pull/18936 as well.

PR-URL: https://github.com/nodejs/node/pull/19329
Fixes: https://github.com/nodejs/node/issues/19240
Refs: https://github.com/nodejs/node/pull/18121


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Chen Gang <gangc.cxy@foxmail.com>

Before:

> Object(7n)
BigInt {}

After:

> Object(7n)
[BigInt: 7n]

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


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>

Add checks for a pending exception in napi_make_callback
after the callback has been invoked.  If there is a pending
exception then we need to avoid checking the result as that
will not be able to complete properly.

Add additional checks to the unit test for napi_make_callback
to catch this case.

PR-URL: https://github.com/nodejs/node/pull/19362
Fixes: https://github.com/nodejs/node-addon-api/issues/235


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

Emit the perf_hooks node timing milestones as trace events.

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


Reviewed-By: Anna Henningsen <anna@addaleax.net>

The test is unreliable on some Windows platforms in its current form.
Make it more robust by using `setInterval()` to repeat the flooding
until an error is triggered.

Fixes: https://github.com/nodejs/node/issues/18251

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


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

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


Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

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


Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Khaidi Chu <i@2333.moe>

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


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

Currently PromiseHook extracts the PromiseWrap from a Local<Promise> in
two places. This commit extracts that code into a function instead.

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


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

Move period incorrectly placed outside of parentheses to inside the
parentheses. The parenthetical in this case is a full sentence.

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


Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>

Added some simple tests to verify that the int64 API is correctly
handling numbers greater than 32-bits. This is a basic test, but
verifies that an implementer hasn't truncated back to 32-bits.

Refs: https://github.com/nodejs/node-chakracore/pull/496

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


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

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


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

Access permission on the target child is currently thrown
as an exception. Bring this under the runtime error definition,
much like ENOENT and friends.

PR-URL: https://github.com/nodejs/node/pull/19294
Fixes: https://github.com/nodejs/help/issues/990


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

- Make the HandleCleanup a struct, and make the queue a
  std::list, iterate over it in CleanupHandles() and clear
  it after that.
- Put the handle cleanup registration into a method and
  document that they will not be called in the one environemt
  per process setup.

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


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jackson Tian <shyvo1987@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

CleanupHandles() has not been called in our own code base anymore
after the v8 debug agent has been removed. It used to be in the
~Environment() destructor but then removed to avoid firing other
events after the exit event, given that we were not going to
clean up handles for the one environment per process setup.
Call it in FreeEnvironment so that embedders can clean up
the handles in the loop when creating multiple environments.

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


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jackson Tian <shyvo1987@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Refs: https://github.com/v8/v8/wiki/Untrusted-code-mitigations

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


Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benedikt Meurer <benedikt.meurer@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

Add a GYP flag similar to the one defined in BUILD.gn.

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


Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benedikt Meurer <benedikt.meurer@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

The previous commit deleted lib/module.js so that git
recognize the file move `lib/module.js` ->
`lib/internal/modules/cjs/loader.js`. This commit add the
redirection back.

PR-URL: https://github.com/nodejs/node/pull/19177
Refs: https://github.com/nodejs/node/pull/19112


Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Create `lib/internal/modules` and restructure the module loaders
to make the purpose of those files clearer.

Also make it clear in the code that the object exported by
`lib/internal/modules/cjs/loader.js` is `CJSModule` instead of the
ambiguous `Module`.

Before:

```
lib
├── ...
├── internal
│       ├── loaders
│       │     ├── CreateDynamicModule.js
│       │     ├── DefaultResolve.js
│       │     ├── Loader.js
│       │     ├── ModuleJob.js
│       │     ├── ModuleMap.js
│       │     └── Translators.js
│       └── module.js
└── module.js
```

After:

```
lib
├── ...
├── internal
│       ├── ...
│       └── modules
│              ├── cjs
│              │     ├── helpers.js
│              │     └── loader.js
│              └── esm
│                    ├── CreateDynamicModule.js
│                    ├── DefaultResolve.js
│                    ├── Loader.js
│                    ├── ModuleJob.js
│                    ├── ModuleMap.js
│                    └── Translators.js
└── module.js # deleted in this commit to work with git file mode
```

PR-URL: https://github.com/nodejs/node/pull/19177
Refs: https://github.com/nodejs/node/pull/19112


Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Create `lib/internal/bootstrap/` and put bootstrappers there:

Before:

```
lib/internal
├── ...
├── bootstrap_loaders.js
└── bootstrap_node.js
```

After:

```
lib/internal
├── ...
└── bootstrap
    ├── loaders.js
    └── node.js
```

PR-URL: https://github.com/nodejs/node/pull/19177
Refs: https://github.com/nodejs/node/pull/19112


Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Otherwise the debug log output might be mixed up with
the expected errors and the assertion matching the error
message would fail.

PR-URL: https://github.com/nodejs/node/pull/19177
Refs: https://github.com/nodejs/node/pull/19112


Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

The `StreamBase` interface changed, so that `OnStreamRead()`
and `OnStreamAlloc()` are not guaranteed to be emitted in the
same tick any more.

This means that, while it isn’t causing any trouble right now,
we should not assume that it’s safe to return a static buffer
in the HTTP parser’s `OnStreamAlloc()` method.

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


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

`requests = 1000000` took about 10 minutes per run for me
and doesn’t seem to add much value on its own.

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


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

This allows V8 to avoid preparing a execution context
for the constructor, to give a (kinda) small but noticeable
perf gain.

Benchmarks (only this commit):

    $ ./node benchmark/compare.js --new ./node --old ./node-master --filter net-c2s.js --set len=10 --set type=asc --runs 360 net | Rscript benchmark/compare.R
    [01:15:27|% 100| 1/1 files | 720/720 runs | 1/1 configs]: Done
                                             confidence improvement accuracy (*)   (**)  (***)
     net/net-c2s.js dur=5 type='asc' len=10        ***      0.69 %       ±0.31% ±0.41% ±0.53%

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


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

Requiring `respondWithFile()` to only work with regular files
is an artificial restriction on Node’s side and has become unnecessary.

Offsets or lengths cannot be specified for those files,
but that is an inherent property of other file types.

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


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

This resolves the issue of using synchronous I/O for
`respondWithFile()` and `respondWithFD()`, and enables
scenarios in which the underlying file does not need
to be a regular file.

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


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

Provide a way to create pipes between native `StreamBase` instances
that acts more directly than a `.pipe()` call would.

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


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

Add `AsyncScope` for cases where the async_hooks `before` and
`after` callbacks should be called, to track async context,
but no actual JS is called in between and we can therefore
skip things like draining the microtask or `nextTick` queues.

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


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

Add a `OnStreamWantsWrite()` event that allows streams to
ask for more input data if they want some.

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


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

This enables accessing files using a more standard pattern.

Once some more refactoring has been performed on the other existing
`StreamBase` streams, this could also be used to implement `fs`
streams in a more standard manner.

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


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

Put `HandleScope`s and `Context::Scope`s where they are used,
and don’t create one for native stream callbacks automatically.

This is slightly less convenient but means that stream listeners
that don’t actually call back into JS don’t have to pay the
(small) cost of setting these up.

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


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

Currently there are two tests that specify a third argument, a
deprecation code string, when calling common.expectWarning. The
function only takes two arguments and this third argument is not used.

This commit removes the deprecation code.

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


Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Jackson Tian <shyvo1987@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>

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


Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>

* easily -> easy
* was -> is
* add a missing comma

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


Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

* Remove unnecessary *Note:* designations.
* Various small edits for tone, punctuation, etc.

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


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

test-cluster-send-handle-large-payload is susceptible to failure if it
is competing for resources. Move to `sequential` directory so it is not
competing with other tests.

Fixes: https://github.com/nodejs/node/issues/14844

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


Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>