PR-URL: https://github.com/nodejs/node/pull/57940
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Co-authored-by: RafaelGSS <rafael.nunu@hotmail.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/57892
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
This commit adds an isTransaction getter to the DatabaseSync
class for determining if the database is currently within a
transaction.
Fixes: https://github.com/nodejs/node/issues/57922
PR-URL: https://github.com/nodejs/node/pull/57925
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Modified to check the format of the netmask instead
of just checking that each part of the netmask is even
PR-URL: https://github.com/nodejs/node/pull/57324
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
And do not lose time trying to regenerate the project and rebuild
before running the tests.
PR-URL: https://github.com/nodejs/node/pull/57919
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
This change adds proper tracking of HTTP / 2 server sessions
to ensure they are gracefully closed when the server is
shut down.It implements:
- A new kSessions symbol for tracking active sessions
- Adding/removing sessions from a SafeSet in the server
- A closeAllSessions helper function to close active sessions
- Updates to Http2Server and Http2SecureServer close methods
Breaking Change: any client trying to create new requests
on existing connections will not be able to do so once
server close is initiated
Refs: https://datatracker.ietf.org/doc/html/rfc7540\#section-9.1
Refs: https://nodejs.org/api/http.html\#serverclosecallback
- improve HTTP/2 server shutdown to prevent race conditions
1. Fix server shutdown race condition
- Stop listening for new connections before closing existing ones
- Ensure server.close() properly completes in all scenarios
2. Improve HTTP/2 tests
- Replace setTimeout with event-based flow control
- Simplify test logic for better readability
- Add clear state tracking for event ordering
- Improve assertions to verify correct shutdown sequence
This eliminates a race condition where new sessions could connect
between the time existing sessions are closed and the server stops
listening, potentially preventing the server from fully shutting down.
- fix cross-platform test timing issues
Fix test-http2-server-http1-client.js failure on Ubuntu
by deferring server.close() to next event loop cycle.
The issue only affected Ubuntu where session close occurs
before error emission, causing the test to miss errors
when HTTP/1 clients connect to HTTP/2 servers.
Using setImmediate() ensures error events fire before
server close across all platforms while maintaining
recent session handling improvements.
PR-URL: https://github.com/nodejs/node/pull/57586
Fixes: https://github.com/nodejs/node/issues/57611
Refs: https://datatracker.ietf.org/doc/html/rfc7540#section-9.1
Refs: https://nodejs.org/api/http.html#serverclosecallback
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/57834
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
The latest performance optimization did not take into account that
an object may have a property called constructor. This is addressed
in this PR by adding a new fast path and using fallbacks.
PR-URL: https://github.com/nodejs/node/pull/57876
Reviewed-By: Jordan Harband <ljharb@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
They are already set by `common.gypi`.
PR-URL: https://github.com/nodejs/node/pull/57907
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/57868
Reviewed-By: Darshan Sen <raisinten@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: Qingyu Deng <i@ayase-lab.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
The `execve` syscall does exist on IBM i but
it has caveats that make it not usable in Node.js context.
These changes disable building with `execve` like Windows does.
PR-URL: https://github.com/nodejs/node/pull/57883
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
This marks the whole devtools integration section as in active
development.
PR-URL: https://github.com/nodejs/node/pull/57886
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Adds a variant of AsyncHooksGetExecutionAsyncId that takes a V8 Context
and returns the async ID belonging to the Environment (if any) of that
Context. Sometimes we want to use Isolate::GetEnteredOrMicrotaskContext
insteads of Isolate::GetCurrentContext (e.g. recording the async ID in
a V8 GC prologue callback) when current context is not set.
PR-URL: https://github.com/nodejs/node/pull/57820
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Adds worker.getHeapStatistics() so that the heap usage of the worker
could be observer from the parent thread.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/57888
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
PR-URL: https://github.com/nodejs/node/pull/57879
Reviewed-By: Jordan Harband <ljharb@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/57857
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
A follow up of https://github.com/nodejs/node/pull/57578 to
replace all std::vector<v8::Local<T>> to use v8::LocalVector<T>
PR-URL: https://github.com/nodejs/node/pull/57642
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
According to V8's public API documentation, local handles
(i.e., objects of type v8::Local<T>) "should never be allocated
on the heap". This replaces the usage of heap-allocated data
structures containing instances of `v8::Local`, specifically the
`std::vector<v8::Local<v8::String>>` with recently
introduced `v8::LocalVector<T>`.
This is first of the series of commits to replace all
`std::vector<v8::Local<T>>` to use `v8::LocalVector<T>`.
PR-URL: https://github.com/nodejs/node/pull/57578
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/57889
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Qingyu Deng <i@ayase-lab.com>
An `std::string_view v` is a `const char* v.data()` along with an
`std::size_t v.size()` that guarantees that `v.size()` contiguous
elements of type `char` can be accessed relative to the pointer
`v.data()`.
One of the main reasons behind the existence of `std::string_view` is
the ability to operate on `char` sequences without requiring null
termination, which otherwise often requires expensive copies of strings
to be made. As a consequence, it is generally incorrect to assume that
`v.data()` points to a null-terminated sequence of `char`, and the only
way to obtain a null-terminated string from an `std::string_view` is to
make a copy. It is not even possible to check if the sequence pointed to
by `v.data()` is null-terminated because the null character would be at
position `v.data() + v.size()`, which is outside of the range that `v`
guarantees safe access to. (A default-constructed `std::string_view`
even sets its own data pointer to a `nullptr`, which is fine because it
only needs to guarantee safe access to zero elements, i.e., to no
elements).
In `deps/ncrypto` and `src/crypto`, there are various APIs that consume
`std::string_view v` arguments but then ignore `v.size()` and treat
`v.data()` as a C-style string of type `const char*`. However, that is
not what call sites would expect from functions that explicitly ask for
`std::string_view` arguments, since it makes assumptions beyond the
guarantees provided by `std::string_view` and leads to undefined
behavior unless the given view either contains an embedded null
character or the `char` at address `v.data() + v.size()` is a null
character. This is not a reasonable assumption for `std::string_view` in
general, and it also defeats the purpose of `std::string_view` for the
most part since, when `v.size()` is being ignored, it is essentially
just a `const char*`.
Constructing an `std::string_view` from a `const char*` is also not
"free" but requires computing the length of the C-style string (unless
the length can be computed at compile time, e.g., because the value is
just a string literal). Repeated conversion between `const char*` as
used by OpenSSL and `std::string_view` as used by ncrypto thus incurs
the additional overhead of computing the length of the string whenever
an `std::string_view` is constructed from a `const char*`. (This seems
negligible compared to the safety argument though.)
Similarly, returning a `const char*` pointer to a C-style string as an
`std::string_view` has two downsides: the function must compute the
length of the string in order to construct the view, and the caller
can no longer assume that the return value is null-terminated and thus
cannot pass the returned view to functions that require their arguments
to be null terminated. (And, for the reasons explained above, the caller
also cannot check if the value is null-terminated without potentially
invoking undefined behavior.)
C++20 unfortunately does not have a type similar to Rust's `CStr` or
GSL `czstring`. Therefore, this commit changes many occurrences of
`std::string_view` back to `const char*`, which is conventional for
null-terminated C-style strings and does not require computing the
length of strings.
There are _a lot_ of occurrences of `std::string_view` in ncrypto and
for each one, we need to evaluate if it is safe and a good abstraction.
I tried to do so, but I might have changed too few or too many, so
please feel free to give feedback on individual occurrences.
PR-URL: https://github.com/nodejs/node/pull/57816
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Previously we have been using the variant of SnapshotCreator that
only passes the external references instead of
v8::Isolate::CreateParams and it's about to be deprecated.
Switch to using the new constructor that takes a fully CreateParams
instead.
This also makes sure that the snapshot building script is using
the Node.js array buffer allocator instead of a separate default
one that was previously used by the old constructor. The zero fill
toggle in the Node.js array buffer allocator would still be ignored
during snapshot building, however, until we fixes the array buffer
allocator and let V8 own the toggle backing store instead, because
otherwise the snapshot would contain the external toggle address
and become unreproducible.
PR-URL: https://github.com/nodejs/node/pull/55337
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
increase the z-index of the header element to make sure that
opened pickers from it (such as the node version picker) are
on top of other content
PR-URL: https://github.com/nodejs/node/pull/57851
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/57848
Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/57841
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/57825
Reviewed-By: Jordan Harband <ljharb@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
The fourth positional argument of `createCipherBase` is `true` when
called from within the `Cipheriv` constructor and `false`when called
from within the `Decipheriv` constructor. This value is then passed to
the `CipherBase::New()` method, which treats it as follows:
args[0]->IsTrue() ? kCipher : kDecipher
However, the current name of said positional argument is `decipher` and
thus indicates the exact opposite: when the argument is set to true, the
instance is *not* a `Decipheriv` object. Therefore, this commit renames
the argument to `isEncrypt`, which matches the actual semantics.
PR-URL: https://github.com/nodejs/node/pull/57843
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
