Do not assume that server handshake event happens before client, it is
not guaranteed.
PR-URL: https://github.com/nodejs/node/pull/25508
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Test assumed that server got the the connection before the client
destroys it, but that is not guaranteed. Also, the test was closing the
TCP connection 3 times, effectively:
1. on the server side, right after TLS connection occurs (if it does)
2. on the client side, internal to tls, when the cert is rejected
3. again on the client side, in the error event which is emitted by
the internal tls destroy from 2
This is too often, and the dependency on 1 occurring is fragile.
PR-URL: https://github.com/nodejs/node/pull/25508
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Test assumed server gets a handshake before the client destroyed it, and
didn't assert that dns.lookup() callback occurred.
PR-URL: https://github.com/nodejs/node/pull/25508
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Test for deep strict equality when prototype and toStringTag
have been modified in surprising ways.
PR-URL: https://github.com/nodejs/node/pull/25932
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Add some BigInt tests for test-util-isDeepStrictEqual to get 100%
coverage for lib/internal/util/comparisons.js.
PR-URL: https://github.com/nodejs/node/pull/25932
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Previously a GC prologue callback and a GC epilogue callback
are always unconditionally enabled during bootstrap when
the `performance` binding is loaded, even when the user does
not use the performance timeline API to enable GC tracking.
This patch makes the callback addition conditional and only
enables them when the user explicitly requests
`observer.observe(['gc'])` to avoid the overhead.
PR-URL: https://github.com/nodejs/node/pull/25853
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Allow undefined as a callback, but do not allow null.
PR-URL: https://github.com/nodejs/node/pull/25929
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Remove setting of a 'close' event handler on MessagePort through the use
of an `.onclose` property. We don't use this convention anywhere else in
our codebase for 'close' events, this feature is undocumented, and we
don't test it.
PR-URL: https://github.com/nodejs/node/pull/25904
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
* Confusing (but correct) regex using `A-z` character range by
accident
* Add the status argument to afterShutdown
PR-URL: https://github.com/nodejs/node/pull/25873
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/25906
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
when t0 and t1 are spawned with t0's outputstream [1, 2] is piped into
t1's input, a new pipe is created which uses a copy of the t0's fd.
This leaves the original copy in Node parent, unattended. Net result is
that when t0 produces data, it gets bifurcated into both the copies
Detect the passed handle to be of 'wrap' type and close after the
native spawn invocation by which time piping would have been over.
Fixes: https://github.com/nodejs/node/issues/9413
Fixes: https://github.com/nodejs/node/issues/18016
PR-URL: https://github.com/nodejs/node/pull/21209
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
The removed line does not add anything of value to the test. It was
removed to simplify the test.
PR-URL: https://github.com/nodejs/node/pull/25731
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
It is checked if an command buffer exists or not. This code branch
can only be reached if none exist, so there's no need to clear that
buffer again.
PR-URL: https://github.com/nodejs/node/pull/25731
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
In case no error has occurred during the evaluation of some code,
`undefined` has been returned in some cases as error argument instead
of `null`. This is fixed by this patch.
PR-URL: https://github.com/nodejs/node/pull/25731
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
The completion lists used a hand crafted list of global entries that
was redundant due to also using the actual global properties for tab
completion. Those entries ended up in an separated completion group
which did not seem useful.
PR-URL: https://github.com/nodejs/node/pull/25731
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Signed-off-by: gengjiawen <technicalcute@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/25849
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Signed-off-by: gengjiawen <technicalcute@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/25850
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Because the part that is shared by `process.reallyExit` and the
Node.js teardown is `WaitForInspectorDisconnect()`, move that
into node_internals.h instead, and move the C++ binding code
into `node_process_methods.cc` since that's the only place
it's needed.
PR-URL: https://github.com/nodejs/node/pull/25860
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Signed-off-by: gengjiawen <technicalcute@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/25889
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Some process methods are not supported in workers. This commit
adds stubs that throw more informative errors.
PR-URL: https://github.com/nodejs/node/pull/25587
Fixes: https://github.com/nodejs/node/issues/25448
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The const_cast used to be necessary for SSL_get_app_data() in OpenSSL
0.9.7, but node doesn't compile against OpenSSL versions that old.
However, now it's needed for the recently introduced
SSL_renegotiate_pending(), which is not const-correct as of 1.1.1a.
PR-URL: https://github.com/nodejs/node/pull/25861
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
- Don't use both break and return simultaneously.
- Use case:/UNREACHABLE() to enforce that all cases are handled, instead
of CHECK().
PR-URL: https://github.com/nodejs/node/pull/25861
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
`tls` shadows the global `tls` require, and isn't indicative of the
arument type.
PR-URL: https://github.com/nodejs/node/pull/25861
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Make it clear which of the multiple interfaces a TLSWrap method is
implementing by grouping and commenting the related methods.
PR-URL: https://github.com/nodejs/node/pull/25861
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Declaration is unused, it was added by mistake in 46c5c3388d.
PR-URL: https://github.com/nodejs/node/pull/25861
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Automated tooling for race condition detection reports this as a
possible problem. It’s unlikely that something bad would happen
here (beyond maybe calling `TriggerAsync()` twice), but adding this
should be okay.
PR-URL: https://github.com/nodejs/node/pull/25911
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Embedders may want to control whether a Node.js instance
controls the current process, similar to what we currently
have with `Worker`s.
Previously, the `isMainThread` flag had a bit of a double usage,
both for indicating whether we are (not) running a Worker and
whether we can modify per-process state.
PR-URL: https://github.com/nodejs/node/pull/25881
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Use `'close'` rather than `'exit'` to make sure that all stdio
has been captured by the time that the event handler is run.
PR-URL: https://github.com/nodejs/node/pull/25894
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This function is not only helpful for debugging crashes,
but can also be used as an ad-hoc debugging statement.
PR-URL: https://github.com/nodejs/node/pull/25905
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/25910
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The Windows ETW code is not written to be compatible with multi
threading, and in particular it relies on global state like a
single static `uv_async_t`. Adding that to multiple threads
would corrupt the corresponding loops' handle queues.
This addresses the flakiness of at least
`test-worker-exit-code` and very likely other flaky tests that
relate to Worker threads on Windows as well.
Fixes: https://github.com/nodejs/node/issues/25847
Fixes: https://github.com/nodejs/node/issues/25702
Fixes: https://github.com/nodejs/node/issues/24005
Fixes: https://github.com/nodejs/node/issues/23873
PR-URL: https://github.com/nodejs/node/pull/25907
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Use MaybeLocal::ToLocal and don't crash X509ToObject on error.
PR-URL: https://github.com/nodejs/node/pull/25717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
EC_KEY_key2buf returns an OPENSSL_malloc'd pointer so it shouldn't be
passed into Buffer::New, which expect a libc malloc'd pointer. Instead,
factor out the ECDH::GetPublicKey code which uses EC_POINT_point2oct.
This preserves the existing behavior where encoding failures are
silently ignored, but it is probably safe to CHECK fail them instead.
PR-URL: https://github.com/nodejs/node/pull/25717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Improve the material in the Collaborator Guide on unintended breaking
changes.
PR-URL: https://github.com/nodejs/node/pull/25887
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
OpenSSL is packaging its git and travis configuration files. Remove
them, Node.js has its own.
PR-URL: https://github.com/nodejs/node/pull/25689
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Libuv does not guarantee that handles have their close
callbacks called in the order in which they were added
(and in fact, currently calls them in reverse order).
This patch ensures that the `flush_signal_` handle
is no longer in use (i.e. its close callback has already
been run) when we signal to the main thread that
`~NodeTraceBuffer` may be destroyed.
The same applies for `~NodeTraceWriter`.
Credit for debugging goes to Gireesh Punathil.
Fixes: https://github.com/nodejs/node/issues/25512
Co-authored-by: Gireesh Punathil <gpunathi@in.ibm.com>
PR-URL: https://github.com/nodejs/node/pull/25896
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Don't throw on invalid property access if options is not provided, and
ensure callback is a function.
PR-URL: https://github.com/nodejs/node/pull/25876
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Signed-off-by: gengjiawen <technicalcute@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/25732
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Signed-off-by: gengjiawen <technicalcute@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/25820
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Currently, this test fails when configured --without-ssl:
=== release test-worker-cleanexit-with-moduleload ===
Path: parallel/test-worker-cleanexit-with-moduleload
events.js:173
throw er; // Unhandled 'error' event
^
internal/util.js:101
throw new ERR_NO_CRYPTO();
^
Error [ERR_NO_CRYPTO]:
Node.js is not compiled with OpenSSL crypto support
This commit as a check for crypto so that this test is skipped if there
is no crypto support.
PR-URL: https://github.com/nodejs/node/pull/25811
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
For `globals`, booleans are deprecated in favor strings
`'readable'`/`'writeable'`.
PR-URL: https://github.com/nodejs/node/pull/25877
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
