PR-URL: https://github.com/nodejs/node/pull/49912
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Previously to throw errors from C++ land, sync versions of the fs
were created by copying C++ code from the original implementation
and moving JS code to a separate file. This can lead to several
problems:
1. By moving code to a new file for the sake of moving, it would
be harder to use git blame to trace changes and harder to backport
changes to older branches.
2. Scattering the async and sync versions of fs methods in
different files makes it harder to keep them in sync and
share code in the prologues and epilogues.
3. Having two copies of code doing almost the same thing results
in duplication and can be prone to out-of-sync problems when the
prologue and epilogue get updated.
4. There is a minor cost to startup in adding an additional file.
This can add up even with the help of snapshots.
This patch moves the JS code back to lib/fs.js to stop 1, 2 & 4
and introduces C++ helpers SyncCallAndThrowIf() and
SyncCallAndThrowOnError() so that the original implementations
can be easily tweaked to allow throwing from C++ and stop 3.
PR-URL: https://github.com/nodejs/node/pull/49913
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
There is an `if` statement that likely should have been
an `else` in the original PR.
Refs: https://github.com/nodejs/node/pull/48915
PR-URL: https://github.com/nodejs/node/pull/49943
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Raz Luvaton <rluvaton@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
This prohibits invalid values (< -1 and non-integers) and
allows `filehandle.read()` to handle position up to `2n ** 63n - 1n`
PR-URL: https://github.com/nodejs/node/pull/42835
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/49869
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
V8 will soon support typed arrays as large as the maximum array buffer
length. This patch replaces hardcoded values related to
Buffer.kMaxLength with the actual constant.
It also fixes a test that was passing by accident.
Refs: 44b2995900
PR-URL: https://github.com/nodejs/node/pull/49876
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
PR-URL: https://github.com/nodejs/node/pull/49834
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
PR-URL: https://github.com/nodejs/node/pull/49692
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/49633
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
In #39175, better ESM errors were introduced. This commit tweaks the
language in the error slightly to make it clear that there are three
different options to resolve the error.
Refs: https://github.com/nodejs/node/pull/39175
PR-URL: https://github.com/nodejs/node/pull/49521
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Previously when managing the importModuleDynamically callback of
vm.compileFunction(), we use an ID number as the host defined option
and maintain a per-Environment ID -> CompiledFnEntry map to retain
the top-level referrer function returned by vm.compileFunction() in
order to pass it back to the callback, but it would leak because with
how we used v8::Persistent to maintain this reference, V8 would not
be able to understand the cycle and would just think that the
CompiledFnEntry was supposed to live forever. We made an attempt
to make that reference known to V8 by making the CompiledFnEntry weak
and using a private symbol to make CompiledFnEntry strongly
references the top-level referrer function in
https://github.com/nodejs/node/pull/46785, but that turned out to be
unsound, because the there's no guarantee that the top-level function
must be alive while import() can still be initiated from that
function, since V8 could discard the top-level function and only keep
inner functions alive, so relying on the top-level function to keep
the CompiledFnEntry alive could result in use-after-free which caused
a revert of that fix.
With this patch we use a symbol in the host defined options instead of
a number, because with the stage-3 symbol-as-weakmap-keys proposal
we could directly use that symbol to keep the referrer alive using a
WeakMap. As a bonus this also keeps the other kinds of referrers
alive as long as import() can still be initiated from that
Script/Module, so this also fixes the long-standing crash caused by
vm.Script being GC'ed too early when its importModuleDynamically
callback still needs it.
PR-URL: https://github.com/nodejs/node/pull/48510
Refs: https://github.com/nodejs/node/issues/44211
Refs: https://github.com/nodejs/node/issues/42080
Refs: https://github.com/nodejs/node/issues/47096
Refs: https://github.com/nodejs/node/issues/43205
Refs: https://github.com/nodejs/node/issues/38695
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
If the libuv operations invoked by `readdir`/`opendir` return
`uv_dirent_t` values where the `type` is `UV_DIRENT_UNKNOWN` then a
further `lstat` is issued to fully construct the `Dirent` values. In the
recursive versions of these functions, the `path` parameter was
incorrectly assumed to be the path to the entry when it should be the
path to the directory containing the entry.
Fixes#49499.
PR-URL: https://github.com/nodejs/node/pull/49603
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
