This PR fixes a few different things:
The timing of 'prefinish' depends on whether or not
_final is defined. In on case the event is emitted
synchronously with end() and otherwise asynchronously.
_final is currently unecessarily called asynchronously
which forces implementors to use 'prefinish' as a hack
to emulate synchronous behaviour. Furthermore, this hack
is subtly broken due to the above issue.
Refs: https://github.com/nodejs/node/issues/31401
Refs: https://github.com/nodejs/node/pull/32763#discussion_r407041983
PR-URL: https://github.com/nodejs/node/pull/32780
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
For targets that are strings that do not start with `./` or `/` the
error will now have additional information about what the programming
error is.
Closes: https://github.com/nodejs/node/issues/32034
PR-URL: https://github.com/nodejs/node/pull/32052
Fixes: https://github.com/nodejs/node/issues/32034
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Signed-off-by: Myles Borins <myles.borins@gmail.com>
* cleanup constants in http2 binding
The error constants were just doing some weird things. Cleanup
and improve maintainability.
* simplify settings to reduce duplicate code
* improve style consistency and correctness
Use snake_case for getters and setters at c++ level,
avoid unnecessary use of enums,
use consistent style for exported vs. internal constants,
avoid unnecessary memory info reporting,
use setters/getters for flags_ for improved code readability
* make EmitStatistics function private
* un-nest Http2Settings and Http2Ping
* refactoring and cleanup of Http2Settings and Http2Ping
* avoid ** syntax for readability
The **session and **stream syntax for getting the underlying
nghttp2 pointers is not ideal for readability
* use const references for Http2Priority
* remove unnecessary GetStream function
* refactor Http2Scope to use BaseObjectPtr
* move utility function to anonymous namespace
* refactor and simplify Origins
* Use an AllocatedBuffer instead of MaybeStackBuffer
* Use a const reference instead of pointer
* use BaseObjectPtr for Http2Streams map
* move MemoryInfo impl to cc
Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/32884
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
We introduce status `napi_would_deadlock` to be used as a return status
by `napi_call_threadsafe_function` if the call is made with
`napi_tsfn_blocking` on the main thread and the queue is full.
Fixes: https://github.com/nodejs/node/issues/32615
Signed-off-by: Gabriel Schulhof <gabriel.schulhof@intel.com>
PR-URL: https://github.com/nodejs/node/pull/32860
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Convert callback functions that are anonymous
to arrow functions for better readability.
Also adjusted the `this` object in places where
that was required.
PR-URL: https://github.com/nodejs/node/pull/32875
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
To skip some tests on IBMi PASE, we use
uname to detect the true os name, but
on Windows machines there is no uname
available.
PR-URL: https://github.com/nodejs/node/pull/32833
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Suggest using `--trace-warnings` or `--trace-deprecation` the first
time a warning is emitted without a stack trace, similar to how
we suggest `--trace-uncaught` when printing uncaught exceptions
without a stack trace.
PR-URL: https://github.com/nodejs/node/pull/32797
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: https://github.com/nodejs/node/pull/32727
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/32751
Reviewed-By: Adrian Estrada <edsadr@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Currently, the following warnings are emitted during compilation:
In file included from ../src/string_bytes.h:29,
from ../src/string_bytes.cc:22:
../deps/v8/include/v8.h:
In instantiation of ‘void v8::PersistentBase<T>::SetWeak(
P*,typename v8::WeakCallbackInfo<P>::Callback, v8::WeakCallbackType)
[with P = node::BaseObject;
T = v8::Object;
typename v8::WeakCallbackInfo<P>::Callback =
void (*)(const v8::WeakCallbackInfo<node::BaseObject>&)]’:
../src/base_object-inl.h:135:42: required from here
../deps/v8/include/v8.h:10732:16: warning:
cast between incompatible function types from
‘v8::WeakCallbackInfo<node::BaseObject>::Callback’ {aka
‘void (*)(const v8::WeakCallbackInfo<node::BaseObject>&)’} to
‘Callback’ {aka ‘void (*)(const v8::WeakCallbackInfo<void>&)’}
[-Wcast-function-type]
10732 | reinterpret_cast<Callback>(callback), type);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
And the same from test/cctest/test_aliased_buffer.cc
We have an open pull request against v8 for this:
https://chromium-review.googlesource.com/c/v8/v8/+/2080361
PR-URL: https://github.com/nodejs/node/pull/32679
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Avoid storing data that depends on command line options on internal
bindings. This is generally a cleaner way of accessing CLI options.
PR-URL: https://github.com/nodejs/node/pull/32760
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
This removes a footgun in which users could attempt to transfer the
pooled ArrayBuffer underlying a regular `Buffer`, which would lead to
all `Buffer`s that share the same pool being rendered unusable as well,
and potentially break creation of new pooled `Buffer`s.
This disables this kind of transfer.
Refs: https://github.com/nodejs/node/issues/32752
PR-URL: https://github.com/nodejs/node/pull/32759
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Git for Windows includes `C:\Program Files\Git\bin\bash.exe`,
which spawns ..\usr\bin\bash.exe, so copying that executable
won't work.
However, if a symlink is used to test paths with spaces,
this executable will still work.
PR-URL: https://github.com/nodejs/node/pull/32129
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Use the same type check we use in `MoveToContext()` in
`ReceiveMessage()`.
Fixes: https://github.com/nodejs/node/issues/32742
PR-URL: https://github.com/nodejs/node/pull/32745
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Function setTimeout in net.Socket should return this,
not undefined, as doc said.
PR-URL: https://github.com/nodejs/node/pull/32722
Refs: https://nodejs.org/api/net.html#net_socket_settimeout_timeout_callback
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
This simplifies the code significantly, and removes the dependency of
the inspector code on the availability of a `MultiIsolatePlatform`
(by removing the dependency on a platform altogether).
It also fixes a memory leak that occurs when `RequestInterrupt()`
is used, but the interrupt handler is never called before the Isolate
is destroyed.
One downside is that this leads to a slight change in timing, because
inspector messages are not dispatched in a re-entrant way. This means
having to harden one test to account for that possibility by making
sure that the stack is always clear through a `setImmediate()`.
This does not affect the assertion made by the test, which is that
messages will not be delivered synchronously while other code is
executing.
https://github.com/nodejs/node/issues/32415
PR-URL: https://github.com/nodejs/node/pull/32523
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
We introduce status `napi_would_deadlock` to be used as a return status
by `napi_call_threadsafe_function` if the call is made with
`napi_tsfn_blocking` on the main thread and the queue is full.
PR-URL: https://github.com/nodejs/node/pull/32689
Fixes: https://github.com/nodejs/node/issues/32615
Signed-off-by: Gabriel Schulhof <gabriel.schulhof@intel.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: David Carlier <devnexen@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
PR-URL: https://github.com/nodejs/node/pull/32699
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
The readv_sync.txt test file is currenly saved in the root directory.
Save it in the dedicated temporary directory.
PR-URL: https://github.com/nodejs/node/pull/32670
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/32508
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
stdio (stderr & stdout) should for compatibility
reasons not be closed/end():ed. However, this
causes pipeline with a stdio destination to
never finish. This commit fixes this issue at
a performance cost.
Refs: https://github.com/nodejs/node/issues/7606
Fixes: https://github.com/nodejs/node/issues/32363
PR-URL: https://github.com/nodejs/node/pull/32373
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This commit remove the `id` and `type` arguments from the mustCall
function on init.
PR-URL: https://github.com/nodejs/node/pull/32630
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Add test cases that confirm that `--expose-internals` and
`--expose_internals` are disallowed in the NODE_OPTIONS environment
variable.
PR-URL: https://github.com/nodejs/node/pull/32554
Refs: https://github.com/nodejs/node/pull/32542
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Cover the scenario fixed through
https://github.com/nodejs/node/pull/31621
Unfortunately there is no easy way to test this, in a
cross-platform manner. So the approach is:
- open a child process with ulimit restriction on file descriptors
- in the child process, start few workers - more than the fd limit
- make sure some workers fail, with the expected error type.
- skip the test in windows, as there is no ulimit there.
Refs: https://github.com/nodejs/node/pull/31621
PR-URL: https://github.com/nodejs/node/pull/31929
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
daemon1024
bcoe
Guy Bedford
Robert Nagy
Myles Borins
James M Snell
Andrey Pechkurov
Gabriel Schulhof
Vadzim Zieńka
Nimit
Qinhui Chen
Xu Meng
Anna Henningsen
Sam Roberts
Jesus Hernandez
Julian Duque
Daniel Bevenius
Gerhard Stoebich
unknown
Stephen Belanger
John Kleinschmidt
龙腾道
himself65
Agustin Daguerre
gengjiawen
Luigi Pinca
Juan José Arboleda
Harshitha KP