Since worker threads are complete Node.js environments, including the
ability to load native addons, and since those native addons can
allocate resources to be freed when objects go out of scope, and since,
upon worker thread exit, the engine does not invoke the weak callbacks
responsible for freeing resources which still have references, this
modification introduces tracking for weak references such that a list
of outstanding weak references is maintained. This list is traversed
during environment teardown. The callbacks for the remaining weak
references are called.
This change is also relevant for Node.js embedder scenarios, because in
those cases the process also outlives the `node::Environment` and
therefore weak callbacks should also be rendered as environment cleanup
hooks to ensure proper cleanup after native addons. This changes
introduces the means by which this can be accomplished.
A benchmark is included which measures the time it takes to execute the
weak reference callback for a given number of weak references.
Re: https://github.com/tc39/proposal-weakrefs/issues/125#issuecomment-535832130
PR-URL: https://github.com/nodejs/node/pull/28428
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
This removes `require('util')` from the `domain` module. There was
only a single simple type check used from the `util` module which
is now inlined instead.
PR-URL: https://github.com/nodejs/node/pull/29825
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Update tests to create more nested folders and files, like rimraf
package tests do. Each test will create 28 nested directories and
210 files. It will also create different types of files, like
symlinks.
PR-URL: https://github.com/nodejs/node/pull/29815
Reviewed-By: Ben Coe <bencoe@gmail.com>
* This is a semver-major change to rename the FSWatcher.start
function to FSWatcher._start to make it private
The motivation here is that it serves no purpose to the end user.
An instance of FSWatcher is returned when a user calls fs.watch,
which will call the start method. A user can't create an instance
of a FSWatcher directly. If the start method is called by a user
it is a noop since the watcher has already started. Calling start
after a watcher has closed is also a noop
PR-URL: https://github.com/nodejs/node/pull/29905
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
When it isn't nested, a new unordered list is created for the next item
PR-URL: https://github.com/nodejs/node/pull/29915
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Using `process.exit()` in these tests is unnecessary and may mask other
problems.
Refs: https://github.com/nodejs/node/pull/29873#pullrequestreview-298851847
PR-URL: https://github.com/nodejs/node/pull/29923
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
The two-week wait period for merging npm releases is one of those rule
exceptions that would be great to get rid of (in my opinion at least).
There are too many exceptions to our rules and they tend to be scattered
across multiple documents. People don't feel confident they know the
rules, thus hampering both project velocity and Collaborator confidence.
It also means I (and perhaps others?) get lots of pings about whether
this or that can land, etc.
This particular issue has come up a few times lately, and is
specifically calling for an exception-to-the-exception so that the
latest version of npm can be released along with Node.js 13.0.0.
Refs: https://github.com/nodejs/node/pull/29885#issuecomment-540730054
I propose here reducing the wait period from two weeks to one week. If,
after some amount of time, there seems to be no problems caused by this
change, we can consider further reducing the wait period to 48 hours to
align it with all other change requests. Even if you think that is going
too far, hopefully we can at least get it reduced to a week, as the
second week of the waiting period is usually just the PR sitting around
with an occasional ping from someone about whether/when it can land.
PR-URL: https://github.com/nodejs/node/pull/29922
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
WriteStream.open() and ReadStream.open() are undocumented internal
APIs that do not make sense to use in userland. File streams should
always be opened through their corresponding factory methods
(fs.createWriteStream() and fs.createReadStream()) or by passing a file
descriptor in options.
PR-URL: https://github.com/nodejs/node/pull/29061
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: João Reis <reis@janeasystems.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Set ClientRequest.reusedSocket property when reusing socket for request,
so user can handle retry base on wether the request is reusing a socket.
Refs: https://github.com/request/request/issues/3131
PR-URL: https://github.com/nodejs/node/pull/29715
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
During the addition of macOS support for large pages, a `memcpy` ended
up under the wrong preprocessor directive. As a result, the newly
allocated large pages were not initialized with a copy of the text
section.
Thanks to Suresh Srinivas <suresh.srinivas@intel.com>!
PR-URL: https://github.com/nodejs/node/pull/29914
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/29893
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Original commit message:
[build] Include string in v8.h
Explicitly #include<string> in v8.h, since std::string is referenced
in it. In the C++ STL shipped with Visual Studio 2019, none of the
headers included in v8.h ends up including the C++ string header, which
caused a compile error.
Bug: v8:9793
Change-Id: I84a133dd10dd6dcc7b70287af393e82cf0dc97df
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1834321
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64074}
Refs: 53e62affd3
PR-URL: https://github.com/nodejs/node/pull/29898
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gus Caplan <me@gus.host>
This makes sure that the `inspectOptions` are validated. This could
otherwise cause confusion.
Fixes: https://github.com/nodejs/node/issues/29726
PR-URL: https://github.com/nodejs/node/pull/29824
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Closing a FileHandle almost never fails, so it was hard to
notice before that `stream.emit(err)` would not emit an error
event due to the missing event name.
Destroying the stream with the error seems like the right thing
to do in that scenario.
PR-URL: https://github.com/nodejs/node/pull/29884
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
It is a common and unfortunate pattern to simply just forward
any and all options into the base constructor without taking
into account whether these options might conflict with basic
stream assumptions.
PR-URL: https://github.com/nodejs/node/pull/29857
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This commit removes unused using declarations in
src/node_worker.cc.
PR-URL: https://github.com/nodejs/node/pull/29883
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Release builds specifically set the `--with-intl=` configure option
instead of relying on the default. Explicitly set to `full-icu`.
Refs: https://github.com/nodejs/node/pull/29522
PR-URL: https://github.com/nodejs/node/pull/29887
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/29880
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Clearly state the modules version 72 is official node 12, similar as it
is done for other major node versions.
PR-URL: https://github.com/nodejs/node/pull/29877
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
process.exit() has a tendency to hide test failures because
it forces the process to exit. This test doesn't need the
process.exit(), so this commit removes it.
PR-URL: https://github.com/nodejs/node/pull/29873
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Some doc bits / fixes which were missing from
cbd8d715b2
PR-URL: https://github.com/nodejs/node/pull/29890
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This seems to make sense if we want to promote the use
of `fs.promises`, although it’s not strictly necessary.
PR-URL: https://github.com/nodejs/node/pull/29876
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Make test failures easier to diagnose through progress logging, and
reporting of dlerror().
PR-URL: https://github.com/nodejs/node/pull/29818
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
