diff --git a/.github/workflows/auto-start-ci.yml b/.github/workflows/auto-start-ci.yml index 6a45b2d692b94e..ecc7f6d2e7dbe3 100644 --- a/.github/workflows/auto-start-ci.yml +++ b/.github/workflows/auto-start-ci.yml @@ -55,10 +55,10 @@ jobs: with: node-version: ${{ env.NODE_VERSION }} - - name: Install node-core-utils - run: npm install -g node-core-utils + - name: Install @node-core/utils + run: npm install -g @node-core/utils - - name: Setup node-core-utils + - name: Setup @node-core/utils run: | ncu-config set username ${{ secrets.JENKINS_USER }} ncu-config set token "${{ secrets.GH_USER_TOKEN }}" diff --git a/.github/workflows/commit-queue.yml b/.github/workflows/commit-queue.yml index 8cf3978c3f23ef..322d483d6fff7a 100644 --- a/.github/workflows/commit-queue.yml +++ b/.github/workflows/commit-queue.yml @@ -74,15 +74,15 @@ jobs: uses: actions/setup-node@5e21ff4d9bc1a8cf6de233a3057d20ec6b3fb69d # v3.8.1 with: node-version: ${{ env.NODE_VERSION }} - - name: Install node-core-utils - run: npm install -g node-core-utils@latest + - name: Install @node-core/utils + run: npm install -g @node-core/utils - name: Set variables run: | echo "REPOSITORY=$(echo ${{ github.repository }} | cut -d/ -f2)" >> $GITHUB_ENV echo "OWNER=${{ github.repository_owner }}" >> $GITHUB_ENV - - name: Configure node-core-utils + - name: Configure @node-core/utils run: | ncu-config set branch ${GITHUB_REF_NAME} ncu-config set upstream origin diff --git a/.github/workflows/tools.yml b/.github/workflows/tools.yml index 880586e2879cbe..187352cc542561 100644 --- a/.github/workflows/tools.yml +++ b/.github/workflows/tools.yml @@ -23,6 +23,7 @@ on: - corepack - doc - eslint + - github_reporter - googletest - histogram - icu diff --git a/.github/workflows/update-v8.yml b/.github/workflows/update-v8.yml index fb123a5b069a72..1a6c87aa528d33 100644 --- a/.github/workflows/update-v8.yml +++ b/.github/workflows/update-v8.yml @@ -33,8 +33,8 @@ jobs: uses: actions/setup-node@5e21ff4d9bc1a8cf6de233a3057d20ec6b3fb69d # v3.8.1 with: node-version: ${{ env.NODE_VERSION }} - - name: Install node-core-utils - run: npm install -g node-core-utils@latest + - name: Install @node-core/utils + run: npm install -g @node-core/utils - name: Check and download new V8 version run: | ./tools/dep_updaters/update-v8-patch.sh > temp-output diff --git a/.gitpod.yml b/.gitpod.yml index b674e800f4c015..1a56acf40f1746 100644 --- a/.gitpod.yml +++ b/.gitpod.yml @@ -1,7 +1,7 @@ # Ref: https://github.com/gitpod-io/gitpod/issues/6283#issuecomment-1001043454 tasks: - init: ./configure && timeout 50m make -j16 || true - - init: pnpm i -g node-core-utils + - init: pnpm i -g @node-core/utils # Ref: https://www.gitpod.io/docs/prebuilds#github-specific-configuration github: diff --git a/README.md b/README.md index 76eb5c3c60ec6d..a9eafa7595ef93 100644 --- a/README.md +++ b/README.md @@ -721,6 +721,8 @@ maintaining the Node.js project. **Akhil Marsonya** <> (he/him) * [meixg](https://github.com/meixg) - **Xuguang Mei** <> (he/him) +* [mertcanaltin](https://github.com/mertcanaltin) - + **Mert Can Altin** <> * [Mesteery](https://github.com/Mesteery) - **Mestery** <> (he/him) * [preveen-stack](https://github.com/preveen-stack) - diff --git a/benchmark/fs/bench-unlinkSync.js b/benchmark/fs/bench-unlinkSync.js new file mode 100644 index 00000000000000..8b992198c8d368 --- /dev/null +++ b/benchmark/fs/bench-unlinkSync.js @@ -0,0 +1,43 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const tmpdir = require('../../test/common/tmpdir'); +tmpdir.refresh(); + +const bench = common.createBenchmark(main, { + type: ['existing', 'non-existing'], + n: [1e3], +}); + +function main({ n, type }) { + let files; + + switch (type) { + case 'existing': + files = []; + + // Populate tmpdir with mock files + for (let i = 0; i < n; i++) { + const path = tmpdir.resolve(`unlinksync-bench-file-${i}`); + fs.writeFileSync(path, 'bench'); + files.push(path); + } + break; + case 'non-existing': + files = new Array(n).fill(tmpdir.resolve(`.non-existing-file-${Date.now()}`)); + break; + default: + new Error('Invalid type'); + } + + bench.start(); + for (let i = 0; i < n; i++) { + try { + fs.unlinkSync(files[i]); + } catch { + // do nothing + } + } + bench.end(n); +} diff --git a/benchmark/fs/readFileSync.js b/benchmark/fs/readFileSync.js index b81bdce8f27f69..800ab31450f43a 100644 --- a/benchmark/fs/readFileSync.js +++ b/benchmark/fs/readFileSync.js @@ -6,12 +6,21 @@ const fs = require('fs'); const bench = common.createBenchmark(main, { encoding: ['undefined', 'utf8'], path: ['existing', 'non-existing'], - n: [60e1], + hasFileDescriptor: ['true', 'false'], + n: [1e4], }); -function main({ n, encoding, path }) { +function main({ n, encoding, path, hasFileDescriptor }) { const enc = encoding === 'undefined' ? undefined : encoding; - const file = path === 'existing' ? __filename : '/tmp/not-found'; + let file; + let shouldClose = false; + + if (hasFileDescriptor === 'true') { + shouldClose = path === 'existing'; + file = path === 'existing' ? fs.openSync(__filename) : -1; + } else { + file = path === 'existing' ? __filename : '/tmp/not-found'; + } bench.start(); for (let i = 0; i < n; ++i) { try { @@ -21,4 +30,7 @@ function main({ n, encoding, path }) { } } bench.end(n); + if (shouldClose) { + fs.closeSync(file); + } } diff --git a/benchmark/perf_hooks/timerfied.js b/benchmark/perf_hooks/timerfied.js new file mode 100644 index 00000000000000..50be0a47fc1b5a --- /dev/null +++ b/benchmark/perf_hooks/timerfied.js @@ -0,0 +1,36 @@ +'use strict'; + +const assert = require('assert'); +const common = require('../common.js'); + +const { + PerformanceObserver, + performance, +} = require('perf_hooks'); + +function randomFn() { + return Math.random(); +} + +const bench = common.createBenchmark(main, { + n: [1e5], + observe: ['function'], +}); + +let _result; + +function main({ n, observe }) { + const obs = new PerformanceObserver(() => { + bench.end(n); + }); + obs.observe({ entryTypes: [observe], buffered: true }); + + const timerfied = performance.timerify(randomFn); + + bench.start(); + for (let i = 0; i < n; i++) + _result = timerfied(); + + // Avoid V8 deadcode (elimination) + assert.ok(_result); +} diff --git a/benchmark/url/whatwg-url-validity.js b/benchmark/url/whatwg-url-validity.js new file mode 100644 index 00000000000000..6ba22336408fa1 --- /dev/null +++ b/benchmark/url/whatwg-url-validity.js @@ -0,0 +1,23 @@ +'use strict'; +const common = require('../common.js'); +const url = require('url'); +const URL = url.URL; + +const bench = common.createBenchmark(main, { + type: ['valid', 'invalid'], + e: [1e5], +}); + +// This benchmark is used to compare the `Invalid URL` path of the URL parser +function main({ type, e }) { + const url = type === 'valid' ? 'https://www.nodejs.org' : 'www.nodejs.org'; + bench.start(); + for (let i = 0; i < e; i++) { + try { + new URL(url); + } catch { + // do nothing + } + } + bench.end(e); +} diff --git a/common.gypi b/common.gypi index c6d968c5e7447d..d783c7f970237a 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.15', + 'v8_embedder_string': '-node.16', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/include/v8-object.h b/deps/v8/include/v8-object.h index d805dbe9e7d818..1acc0dea60b17e 100644 --- a/deps/v8/include/v8-object.h +++ b/deps/v8/include/v8-object.h @@ -20,6 +20,8 @@ class Function; class FunctionTemplate; template class PropertyCallbackInfo; +class Module; +class UnboundScript; /** * A private symbol @@ -480,6 +482,21 @@ class V8_EXPORT Object : public Value { /** Sets the value in an internal field. */ void SetInternalField(int index, Local value); + /** + * Warning: These are Node.js-specific extentions used to avoid breaking + * changes in Node.js v20.x. They do not exist in V8 upstream and will + * not exist in Node.js v21.x. Node.js embedders and addon authors should + * not use them from v20.x. + */ +#ifndef NODE_WANT_INTERNALS + V8_DEPRECATED("This extention should only be used by Node.js core") +#endif + void SetInternalFieldForNodeCore(int index, Local value); +#ifndef NODE_WANT_INTERNALS + V8_DEPRECATED("This extention should only be used by Node.js core") +#endif + void SetInternalFieldForNodeCore(int index, Local value); + /** * Gets a 2-byte-aligned native pointer from an internal field. This field * must have been set by SetAlignedPointerInInternalField, everything else diff --git a/deps/v8/src/api/api.cc b/deps/v8/src/api/api.cc index e286ccd254497a..a91dfc271c06f2 100644 --- a/deps/v8/src/api/api.cc +++ b/deps/v8/src/api/api.cc @@ -6338,14 +6338,33 @@ Local v8::Object::SlowGetInternalField(int index) { return Utils::ToLocal(value); } -void v8::Object::SetInternalField(int index, v8::Local value) { - i::Handle obj = Utils::OpenHandle(this); +template +void SetInternalFieldImpl(v8::Object* receiver, int index, v8::Local value) { + i::Handle obj = Utils::OpenHandle(receiver); const char* location = "v8::Object::SetInternalField()"; if (!InternalFieldOK(obj, index, location)) return; i::Handle val = Utils::OpenHandle(*value); i::Handle::cast(obj)->SetEmbedderField(index, *val); } +void v8::Object::SetInternalField(int index, v8::Local value) { + SetInternalFieldImpl(this, index, value); +} + +/** + * These are Node.js-specific extentions used to avoid breaking changes in + * Node.js v20.x. + */ +void v8::Object::SetInternalFieldForNodeCore(int index, + v8::Local value) { + SetInternalFieldImpl(this, index, value); +} + +void v8::Object::SetInternalFieldForNodeCore(int index, + v8::Local value) { + SetInternalFieldImpl(this, index, value); +} + void* v8::Object::SlowGetAlignedPointerFromInternalField(int index) { i::Handle obj = Utils::OpenHandle(this); const char* location = "v8::Object::GetAlignedPointerFromInternalField()"; diff --git a/deps/v8/src/flags/flags.cc b/deps/v8/src/flags/flags.cc index e41b71f85ec657..78d7b82354cb04 100644 --- a/deps/v8/src/flags/flags.cc +++ b/deps/v8/src/flags/flags.cc @@ -10,6 +10,7 @@ #include #include #include +#include #include #include "src/base/functional.h" @@ -103,7 +104,12 @@ struct Flag { const char* cmt_; // A comment about the flags purpose. bool owns_ptr_; // Does the flag own its string value? SetBy set_by_ = SetBy::kDefault; + // Name of the flag implying this flag, if any. const char* implied_by_ = nullptr; +#ifdef DEBUG + // Pointer to the flag implying this flag, if any. + const Flag* implied_by_ptr_ = nullptr; +#endif FlagType type() const { return type_; } @@ -113,6 +119,17 @@ struct Flag { bool PointsTo(const void* ptr) const { return valptr_ == ptr; } +#ifdef DEBUG + bool ImpliedBy(const void* ptr) const { + const Flag* current = this->implied_by_ptr_; + while (current != nullptr) { + if (current->PointsTo(ptr)) return true; + current = current->implied_by_ptr_; + } + return false; + } +#endif + bool bool_variable() const { return GetValue(); } void set_bool_variable(bool value, SetBy set_by) { @@ -333,6 +350,15 @@ struct Flag { if (IsAnyImplication(new_set_by)) { DCHECK_NOT_NULL(implied_by); implied_by_ = implied_by; +#ifdef DEBUG + // This only works when implied_by is a flag_name or !flag_name, but it + // can also be a condition e.g. flag_name > 3. Since this is only used for + // checks in DEBUG mode, we will just ignore the more complex conditions + // for now - that will just lead to a nullptr which won't be followed. + implied_by_ptr_ = static_cast( + FindFlagByName(implied_by[0] == '!' ? implied_by + 1 : implied_by)); + DCHECK_NE(implied_by_ptr_, this); +#endif } return change_flag; } @@ -534,15 +560,70 @@ uint32_t ComputeFlagListHash() { std::ostringstream modified_args_as_string; if (COMPRESS_POINTERS_BOOL) modified_args_as_string << "ptr-compr"; if (DEBUG_BOOL) modified_args_as_string << "debug"; + +#ifdef DEBUG + // These two sets are used to check that we don't leave out any flags + // implied by --predictable in the list below. + std::set flags_implied_by_predictable; + std::set flags_ignored_because_of_predictable; +#endif + for (const Flag& flag : flags) { if (flag.IsDefault()) continue; +#ifdef DEBUG + if (flag.ImpliedBy(&v8_flags.predictable)) { + flags_implied_by_predictable.insert(flag.name()); + } +#endif // We want to be able to flip --profile-deserialization without // causing the code cache to get invalidated by this hash. if (flag.PointsTo(&v8_flags.profile_deserialization)) continue; - // Skip v8_flags.random_seed to allow predictable code caching. + // Skip v8_flags.random_seed and v8_flags.predictable to allow predictable + // code caching. if (flag.PointsTo(&v8_flags.random_seed)) continue; + if (flag.PointsTo(&v8_flags.predictable)) continue; + + // The following flags are implied by --predictable (some negated). + if (flag.PointsTo(&v8_flags.concurrent_sparkplug) || + flag.PointsTo(&v8_flags.concurrent_recompilation) || +#ifdef V8_ENABLE_MAGLEV + flag.PointsTo(&v8_flags.maglev_deopt_data_on_background) || + flag.PointsTo(&v8_flags.maglev_build_code_on_background) || +#endif + flag.PointsTo(&v8_flags.parallel_scavenge) || + flag.PointsTo(&v8_flags.concurrent_marking) || + flag.PointsTo(&v8_flags.concurrent_array_buffer_sweeping) || + flag.PointsTo(&v8_flags.parallel_marking) || + flag.PointsTo(&v8_flags.concurrent_sweeping) || + flag.PointsTo(&v8_flags.parallel_compaction) || + flag.PointsTo(&v8_flags.parallel_pointer_update) || + flag.PointsTo(&v8_flags.memory_reducer) || + flag.PointsTo(&v8_flags.cppheap_concurrent_marking) || + flag.PointsTo(&v8_flags.cppheap_incremental_marking) || + flag.PointsTo(&v8_flags.single_threaded_gc)) { +#ifdef DEBUG + if (flag.ImpliedBy(&v8_flags.predictable)) { + flags_ignored_because_of_predictable.insert(flag.name()); + } +#endif + continue; + } modified_args_as_string << flag; } + +#ifdef DEBUG + for (const char* name : flags_implied_by_predictable) { + if (flags_ignored_because_of_predictable.find(name) == + flags_ignored_because_of_predictable.end()) { + PrintF( + "%s should be added to the list of " + "flags_ignored_because_of_predictable\n", + name); + UNREACHABLE(); + } + } +#endif + std::string args(modified_args_as_string.str()); // Generate a hash that is not 0. uint32_t hash = static_cast(base::hash_range( diff --git a/doc/api/cli.md b/doc/api/cli.md index f50b22f729c283..936fd097c1333f 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -1089,7 +1089,7 @@ Silence deprecation warnings. added: v18.0.0 --> -Disable experimental support for the [Fetch API][]. +Disable exposition of [Fetch API][] on the global scope. ### `--no-experimental-global-customevent` diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index aca38c89379437..4f76a4489e4540 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -3406,6 +3406,20 @@ Type: Documentation-only The [`util.toUSVString()`][] API is deprecated. Please use [`String.prototype.toWellFormed`][] instead. +### DEP0176: `fs.F_OK`, `fs.R_OK`, `fs.W_OK`, `fs.X_OK` + + + +Type: Documentation-only + +`F_OK`, `R_OK`, `W_OK` and `X_OK` getters exposed directly on `node:fs` are +deprecated. Get them from `fs.constants` or `fs.promises.constants` instead. + [NIST SP 800-38D]: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf [RFC 6066]: https://tools.ietf.org/html/rfc6066#section-3 [RFC 8247 Section 2.4]: https://www.rfc-editor.org/rfc/rfc8247#section-2.4 diff --git a/doc/api/fs.md b/doc/api/fs.md index 499711f5240af3..d223fa5fb7ca5f 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -1820,6 +1820,10 @@ concurrent modifications on the same file or data corruption may occur. -> Stability: 1 - Experimental. Disable this API with the [`--no-experimental-fetch`][] -> CLI flag. +> Stability: 2 - Stable A browser-compatible implementation of the [`fetch()`][] function. @@ -503,13 +506,16 @@ added: - v17.6.0 - v16.15.0 changes: + - version: + - REPLACEME + pr-url: https://github.com/nodejs/node/pull/45684 + description: No longer experimental. - version: v18.0.0 pr-url: https://github.com/nodejs/node/pull/41811 description: No longer behind `--experimental-global-fetch` CLI flag. --> -> Stability: 1 - Experimental. Disable this API with the [`--no-experimental-fetch`][] -> CLI flag. +> Stability: 2 - Stable A browser-compatible implementation of {FormData}. @@ -539,13 +545,16 @@ added: - v17.5.0 - v16.15.0 changes: + - version: + - REPLACEME + pr-url: https://github.com/nodejs/node/pull/45684 + description: No longer experimental. - version: v18.0.0 pr-url: https://github.com/nodejs/node/pull/41811 description: No longer behind `--experimental-global-fetch` CLI flag. --> -> Stability: 1 - Experimental. Disable this API with the [`--no-experimental-fetch`][] -> CLI flag. +> Stability: 2 - Stable A browser-compatible implementation of {Headers}. @@ -776,13 +785,16 @@ added: - v17.5.0 - v16.15.0 changes: + - version: + - REPLACEME + pr-url: https://github.com/nodejs/node/pull/45684 + description: No longer experimental. - version: v18.0.0 pr-url: https://github.com/nodejs/node/pull/41811 description: No longer behind `--experimental-global-fetch` CLI flag. --> -> Stability: 1 - Experimental. Disable this API with the [`--no-experimental-fetch`][] -> CLI flag. +> Stability: 2 - Stable A browser-compatible implementation of {Response}. @@ -793,13 +805,16 @@ added: - v17.5.0 - v16.15.0 changes: + - version: + - REPLACEME + pr-url: https://github.com/nodejs/node/pull/45684 + description: No longer experimental. - version: v18.0.0 pr-url: https://github.com/nodejs/node/pull/41811 description: No longer behind `--experimental-global-fetch` CLI flag. --> -> Stability: 1 - Experimental. Disable this API with the [`--no-experimental-fetch`][] -> CLI flag. +> Stability: 2 - Stable A browser-compatible implementation of {Request}. @@ -999,7 +1014,6 @@ A browser-compatible implementation of [`WritableStreamDefaultWriter`][]. [CommonJS module]: modules.md [ECMAScript module]: esm.md [Web Crypto API]: webcrypto.md -[`--no-experimental-fetch`]: cli.md#--no-experimental-fetch [`--no-experimental-global-customevent`]: cli.md#--no-experimental-global-customevent [`--no-experimental-global-webcrypto`]: cli.md#--no-experimental-global-webcrypto [`AbortController`]: https://developer.mozilla.org/en-US/docs/Web/API/AbortController diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 10b6dac61c4490..feb9fa6061ca4e 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -6245,6 +6245,13 @@ napi_create_threadsafe_function(napi_env env, [`napi_threadsafe_function_call_js`][] provides more details. * `[out] result`: The asynchronous thread-safe JavaScript function. +**Change History:** + +* Experimental (`NAPI_EXPERIMENTAL` is defined): + + Uncaught exceptions thrown in `call_js_cb` are handled with the + [`'uncaughtException'`][] event, instead of being ignored. + ### `napi_get_threadsafe_function_context` -> Stability: 1 - Experimental. +> Stability: 2 - Stable An implementation of the [WHATWG Streams Standard][]. diff --git a/doc/contributing/backporting-to-release-lines.md b/doc/contributing/backporting-to-release-lines.md index 58f6a54192e12a..851e4e255442d1 100644 --- a/doc/contributing/backporting-to-release-lines.md +++ b/doc/contributing/backporting-to-release-lines.md @@ -48,7 +48,7 @@ line. ### Automated -1. Make sure you have [`node-core-utils`][] installed +1. Make sure you have [`@node-core/utils`][] installed 2. Run the [`git node backport`][] command @@ -132,6 +132,6 @@ original pull request with `backported-to-v20.x`. [Release Plan]: https://github.com/nodejs/Release#release-plan [Release Schedule]: https://github.com/nodejs/Release#release-schedule +[`@node-core/utils`]: https://github.com/nodejs/node-core-utils [`git node backport`]: https://github.com/nodejs/node-core-utils/blob/main/docs/git-node.md#git-node-backport -[`node-core-utils`]: https://github.com/nodejs/node-core-utils [`node-test-pull-request`]: https://ci.nodejs.org/job/node-test-pull-request/build diff --git a/doc/contributing/collaborator-guide.md b/doc/contributing/collaborator-guide.md index a4218acc7d4fd0..2ecf14a082dc3b 100644 --- a/doc/contributing/collaborator-guide.md +++ b/doc/contributing/collaborator-guide.md @@ -555,22 +555,23 @@ See the [commit queue guide][commit-queue.md]. ### Using `git-node` -In most cases, using [the `git-node` command][git-node] of [`node-core-utils`][] -is enough to land a pull request. If you discover a problem when using -this tool, please file an issue [to the issue tracker][node-core-utils-issues]. +In most cases, using [the `git-node` command][git-node] of +[`@node-core/utils`][] is enough to land a pull request. If you discover a +problem when using this tool, please file an issue +[to the issue tracker][node-core-utils-issues]. Quick example: ```bash -npm install -g node-core-utils +npm install -g @node-core/utils git node land $PRID ``` -To use `node-core-utils`, you will need a GitHub access token. If you do not -have one, `node-core-utils` will create one for you the first time you use it. +To use `@node-core/utils`, you will need a GitHub access token. If you do not +have one, `@node-core/utils` will create one for you the first time you use it. To do this, it will ask for your GitHub password and two-factor authentication code. If you wish to create the token yourself in advance, see -[the `node-core-utils` guide][node-core-utils-credentials]. +[the `@node-core/utils` guide][node-core-utils-credentials]. ### Technical HOWTO @@ -959,7 +960,7 @@ need to be attached anymore, as only important bugfixes will be included. [TSC]: https://github.com/nodejs/TSC [`--pending-deprecation`]: ../api/cli.md#--pending-deprecation [`--throw-deprecation`]: ../api/cli.md#--throw-deprecation -[`node-core-utils`]: https://github.com/nodejs/node-core-utils +[`@node-core/utils`]: https://github.com/nodejs/node-core-utils [backporting guide]: backporting-to-release-lines.md [commit message guidelines]: pull-requests.md#commit-message-guidelines [commit-example]: https://github.com/nodejs/node/commit/b636ba8186 diff --git a/doc/contributing/commit-queue.md b/doc/contributing/commit-queue.md index 4730d0889e99aa..cece9ea84e94f8 100644 --- a/doc/contributing/commit-queue.md +++ b/doc/contributing/commit-queue.md @@ -7,8 +7,8 @@ _tl;dr: You can land pull requests by adding the `commit-queue` label to it._ Commit Queue is an experimental feature for the project which simplifies the landing process by automating it via GitHub Actions. With it, collaborators can land pull requests by adding the `commit-queue` label to a PR. All -checks will run via node-core-utils, and if the pull request is ready to land, -the Action will rebase it and push to `main`. +checks will run via `@node-core/utils`, and if the pull request is ready to +land, the Action will rebase it and push to `main`. This document gives an overview of how the Commit Queue works, as well as implementation details, reasoning for design choices, and current limitations. @@ -76,7 +76,7 @@ reasons: commit, meaning we wouldn't be able to use it for already opened PRs without rebasing them first. -`node-core-utils` is configured with a personal token and +`@node-core/utils` is configured with a personal token and a Jenkins token from [@nodejs-github-bot](https://github.com/nodejs/github-bot). `octokit/graphql-action` is used to fetch all pull requests with the diff --git a/doc/contributing/maintaining/maintaining-V8.md b/doc/contributing/maintaining/maintaining-V8.md index 0fe9b393fc0f15..740af7f228f694 100644 --- a/doc/contributing/maintaining/maintaining-V8.md +++ b/doc/contributing/maintaining/maintaining-V8.md @@ -122,7 +122,7 @@ some manual steps and is recommended. Here are the steps for the bug mentioned above: -1. Install `git-node` by installing [`node-core-utils`][]. +1. Install `git-node` by installing [`@node-core/utils`][]. 2. Install the prerequisites for [`git-node-v8`][]. 3. Find the commit hash linked-to in the issue (in this case a51f429). 4. Checkout a branch off the appropriate _vY.x-staging_ branch (e.g. @@ -277,7 +277,7 @@ that Node.js may be floating (or else cause a merge conflict). #### Applying minor updates with `git-node` (recommended) -1. Install [`git-node`][] by installing [`node-core-utils`][]. +1. Install [`git-node`][] by installing [`@node-core/utils`][]. 2. Install the prerequisites for [`git-node-v8`][]. 3. Run `git node v8 minor` to apply a minor update. @@ -384,8 +384,8 @@ This would require some tooling to: [V8MergingPatching]: https://v8.dev/docs/merge-patch [V8TemplateMergeRequest]: https://bugs.chromium.org/p/v8/issues/entry?template=Node.js%20merge%20request [V8TemplateUpstreamBug]: https://bugs.chromium.org/p/v8/issues/entry?template=Node.js%20upstream%20bug +[`@node-core/utils`]: https://github.com/nodejs/node-core-utils#Install [`git-node-v8-backport`]: https://github.com/nodejs/node-core-utils/blob/main/docs/git-node.md#git-node-v8-backport-sha [`git-node-v8-minor`]: https://github.com/nodejs/node-core-utils/blob/main/docs/git-node.md#git-node-v8-minor [`git-node-v8`]: https://github.com/nodejs/node-core-utils/blob/HEAD/docs/git-node.md#git-node-v8 [`git-node`]: https://github.com/nodejs/node-core-utils/blob/HEAD/docs/git-node.md#git-node-v8 -[`node-core-utils`]: https://github.com/nodejs/node-core-utils#Install diff --git a/doc/contributing/releases.md b/doc/contributing/releases.md index 8b4bdb3c487104..bf34bff850c678 100644 --- a/doc/contributing/releases.md +++ b/doc/contributing/releases.md @@ -563,7 +563,7 @@ ecosystem. Use `ncu-ci` to compare `vx.x` run (10) and proposal branch (11) ```bash -npm i -g node-core-utils +npm i -g @node-core/utils ncu-ci citgm 10 11 ``` @@ -1052,7 +1052,7 @@ _In whatever form you do this..._ ### Marking a release line as LTS The process of marking a release line as LTS has been automated using -[node-core-utils](https://github.com/nodejs/node-core-utils). +[`@node-core/utils`](https://github.com/nodejs/node-core-utils). Start by checking out the staging branch for the release line that is going to be marked as LTS, e.g: @@ -1061,10 +1061,10 @@ be marked as LTS, e.g: git checkout v1.x-staging ``` -Next, make sure you have **node-core-utils** installed: +Next, make sure you have **`@node-core/utils`** installed: ```bash -npm i -g node-core-utils +npm i -g @node-core/utils ``` Run the prepare LTS release command: @@ -1110,7 +1110,7 @@ current LTS codename in its release line changelog file. The `test/parallel/test-process-release.js` file might also need to be updated. -In case you can not run the automated `node-core-utils` command and you are +In case you can not run the automated `@node-core/utils` command and you are currently running these steps manually it's a good idea to refer to previous LTS proposal PRs and make sure all required changes are covered. diff --git a/lib/fs.js b/lib/fs.js index 015e424efbdf4c..29f356a57cd22e 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -437,13 +437,11 @@ function tryReadSync(fd, isUserFd, buffer, pos, len) { function readFileSync(path, options) { options = getOptions(options, { flag: 'r' }); - const isUserFd = isFd(path); // File descriptor ownership - - // TODO(@anonrig): Do not handle file descriptor ownership for now. - if (!isUserFd && (options.encoding === 'utf8' || options.encoding === 'utf-8')) { + if (options.encoding === 'utf8' || options.encoding === 'utf-8') { return syncFs.readFileUtf8(path, options.flag); } + const isUserFd = isFd(path); // File descriptor ownership const fd = isUserFd ? path : fs.openSync(path, options.flag, 0o666); const stats = tryStatSync(fd, isUserFd); @@ -1852,10 +1850,7 @@ function unlink(path, callback) { * @returns {void} */ function unlinkSync(path) { - path = getValidatedPath(path); - const ctx = { path }; - binding.unlink(pathModule.toNamespacedPath(path), undefined, ctx); - handleErrorFromBinding(ctx); + return syncFs.unlink(path); } /** diff --git a/lib/internal/errors.js b/lib/internal/errors.js index a8c2a9ea15db04..4e332e1ce18d16 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1370,8 +1370,13 @@ E('ERR_INVALID_SYNC_FORK_INPUT', E('ERR_INVALID_THIS', 'Value of "this" must be of type %s', TypeError); E('ERR_INVALID_TUPLE', '%s must be an iterable %s tuple', TypeError); E('ERR_INVALID_URI', 'URI malformed', URIError); -E('ERR_INVALID_URL', function(input) { +E('ERR_INVALID_URL', function(input, base = null) { this.input = input; + + if (base != null) { + this.base = base; + } + // Don't include URL in message. // (See https://github.com/nodejs/node/pull/38614) return 'Invalid URL'; diff --git a/lib/internal/fs/sync.js b/lib/internal/fs/sync.js index 7fd356833b11a2..fbcc2ad2e25b2a 100644 --- a/lib/internal/fs/sync.js +++ b/lib/internal/fs/sync.js @@ -9,7 +9,7 @@ const { getStatFsFromBinding, getValidatedFd, } = require('internal/fs/utils'); -const { parseFileMode } = require('internal/validators'); +const { parseFileMode, isInt32 } = require('internal/validators'); const binding = internalBinding('fs'); @@ -19,7 +19,9 @@ const binding = internalBinding('fs'); * @return {string} */ function readFileUtf8(path, flag) { - path = pathModule.toNamespacedPath(getValidatedPath(path)); + if (!isInt32(path)) { + path = pathModule.toNamespacedPath(getValidatedPath(path)); + } return binding.readFileUtf8(path, stringToFlags(flag)); } @@ -86,6 +88,11 @@ function close(fd) { return binding.closeSync(fd); } +function unlink(path) { + path = pathModule.toNamespacedPath(getValidatedPath(path)); + return binding.unlinkSync(path); +} + module.exports = { readFileUtf8, exists, @@ -95,4 +102,5 @@ module.exports = { statfs, open, close, + unlink, }; diff --git a/lib/internal/modules/esm/create_dynamic_module.js b/lib/internal/modules/esm/create_dynamic_module.js index 26ccd38be1ad6f..95ff24277aa06e 100644 --- a/lib/internal/modules/esm/create_dynamic_module.js +++ b/lib/internal/modules/esm/create_dynamic_module.js @@ -45,8 +45,9 @@ import.meta.done(); if (imports.length) reflect.imports = { __proto__: null }; - const { setCallbackForWrap } = require('internal/modules/esm/utils'); - setCallbackForWrap(m, { + const { registerModule } = require('internal/modules/esm/utils'); + registerModule(m, { + __proto__: null, initializeImportMeta: (meta, wrap) => { meta.exports = reflect.exports; if (reflect.imports) diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index fb76e90c497faa..109ec9be8aee69 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -187,9 +187,10 @@ class ModuleLoader { ) { const evalInstance = (url) => { const { ModuleWrap } = internalBinding('module_wrap'); - const { setCallbackForWrap } = require('internal/modules/esm/utils'); + const { registerModule } = require('internal/modules/esm/utils'); const module = new ModuleWrap(url, undefined, source, 0, 0); - setCallbackForWrap(module, { + registerModule(module, { + __proto__: null, initializeImportMeta: (meta, wrap) => this.importMetaInitialize(meta, { url }), importModuleDynamically: (specifier, { url }, importAssertions) => { return this.import(specifier, url, importAssertions); diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 80228e895fafcf..b143cd0ad34d0e 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -116,8 +116,9 @@ translators.set('module', async function moduleStrategy(url, source, isMain) { maybeCacheSourceMap(url, source); debug(`Translating StandardModule ${url}`); const module = new ModuleWrap(url, undefined, source, 0, 0); - const { setCallbackForWrap } = require('internal/modules/esm/utils'); - setCallbackForWrap(module, { + const { registerModule } = require('internal/modules/esm/utils'); + registerModule(module, { + __proto__: null, initializeImportMeta: (meta, wrap) => this.importMetaInitialize(meta, { url }), importModuleDynamically, }); diff --git a/lib/internal/modules/esm/utils.js b/lib/internal/modules/esm/utils.js index 64bc21a47c7845..98578438302445 100644 --- a/lib/internal/modules/esm/utils.js +++ b/lib/internal/modules/esm/utils.js @@ -7,6 +7,11 @@ const { ObjectFreeze, } = primordials; +const { + privateSymbols: { + host_defined_option_symbol, + }, +} = internalBinding('util'); const { ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING, ERR_INVALID_ARG_VALUE, @@ -21,16 +26,8 @@ const { setImportModuleDynamicallyCallback, setInitializeImportMetaObjectCallback, } = internalBinding('module_wrap'); -const { - getModuleFromWrap, -} = require('internal/vm/module'); const assert = require('internal/assert'); -const callbackMap = new SafeWeakMap(); -function setCallbackForWrap(wrap, data) { - callbackMap.set(wrap, data); -} - let defaultConditions; function getDefaultConditions() { assert(defaultConditions !== undefined); @@ -73,21 +70,75 @@ function getConditionsSet(conditions) { return getDefaultConditionsSet(); } -function initializeImportMetaObject(wrap, meta) { - if (callbackMap.has(wrap)) { - const { initializeImportMeta } = callbackMap.get(wrap); +/** + * @callback ImportModuleDynamicallyCallback + * @param {string} specifier + * @param {ModuleWrap|ContextifyScript|Function|vm.Module} callbackReferrer + * @param {object} assertions + * @returns { Promise } + */ + +/** + * @callback InitializeImportMetaCallback + * @param {object} meta + * @param {ModuleWrap|ContextifyScript|Function|vm.Module} callbackReferrer + */ + +/** + * @typedef {{ + * callbackReferrer: ModuleWrap|ContextifyScript|Function|vm.Module + * initializeImportMeta? : InitializeImportMetaCallback, + * importModuleDynamically? : ImportModuleDynamicallyCallback + * }} ModuleRegistry + */ + +/** + * @type {WeakMap} + */ +const moduleRegistries = new SafeWeakMap(); + +/** + * V8 would make sure that as long as import() can still be initiated from + * the referrer, the symbol referenced by |host_defined_option_symbol| should + * be alive, which in term would keep the settings object alive through the + * WeakMap, and in turn that keeps the referrer object alive, which would be + * passed into the callbacks. + * The reference goes like this: + * [v8::internal::Script] (via host defined options) ----1--> [idSymbol] + * [callbackReferrer] (via host_defined_option_symbol) ------2------^ | + * ^----------3---- (via WeakMap)------ + * 1+3 makes sure that as long as import() can still be initiated, the + * referrer wrap is still around and can be passed into the callbacks. + * 2 is only there so that we can get the id symbol to configure the + * weak map. + * @param {ModuleWrap|ContextifyScript|Function} referrer The referrer to + * get the id symbol from. This is different from callbackReferrer which + * could be set by the caller. + * @param {ModuleRegistry} registry + */ +function registerModule(referrer, registry) { + const idSymbol = referrer[host_defined_option_symbol]; + // To prevent it from being GC'ed. + registry.callbackReferrer ??= referrer; + moduleRegistries.set(idSymbol, registry); +} + +// The native callback +function initializeImportMetaObject(symbol, meta) { + if (moduleRegistries.has(symbol)) { + const { initializeImportMeta, callbackReferrer } = moduleRegistries.get(symbol); if (initializeImportMeta !== undefined) { - meta = initializeImportMeta(meta, getModuleFromWrap(wrap) || wrap); + meta = initializeImportMeta(meta, callbackReferrer); } } } -async function importModuleDynamicallyCallback(wrap, specifier, assertions) { - if (callbackMap.has(wrap)) { - const { importModuleDynamically } = callbackMap.get(wrap); +// The native callback +async function importModuleDynamicallyCallback(symbol, specifier, assertions) { + if (moduleRegistries.has(symbol)) { + const { importModuleDynamically, callbackReferrer } = moduleRegistries.get(symbol); if (importModuleDynamically !== undefined) { - return importModuleDynamically( - specifier, getModuleFromWrap(wrap) || wrap, assertions); + return importModuleDynamically(specifier, callbackReferrer, assertions); } } throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING(); @@ -142,7 +193,7 @@ async function initializeHooks() { } module.exports = { - setCallbackForWrap, + registerModule, initializeESM, initializeHooks, getDefaultConditions, diff --git a/lib/internal/perf/performance_entry.js b/lib/internal/perf/performance_entry.js index 036bfc173bd024..aa97a652626606 100644 --- a/lib/internal/perf/performance_entry.js +++ b/lib/internal/perf/performance_entry.js @@ -2,7 +2,6 @@ const { ObjectDefineProperties, - ReflectConstruct, Symbol, } = primordials; @@ -25,14 +24,17 @@ const kEntryType = Symbol('PerformanceEntry.EntryType'); const kStartTime = Symbol('PerformanceEntry.StartTime'); const kDuration = Symbol('PerformanceEntry.Duration'); const kDetail = Symbol('NodePerformanceEntry.Detail'); +const kSkipThrow = Symbol('kSkipThrow'); function isPerformanceEntry(obj) { return obj?.[kName] !== undefined; } class PerformanceEntry { - constructor() { - throw new ERR_ILLEGAL_CONSTRUCTOR(); + constructor(skipThrowSymbol = undefined) { + if (skipThrowSymbol !== kSkipThrow) { + throw new ERR_ILLEGAL_CONSTRUCTOR(); + } } get name() { @@ -92,9 +94,11 @@ function initPerformanceEntry(entry, name, type, start, duration) { } function createPerformanceEntry(name, type, start, duration) { - return ReflectConstruct(function PerformanceEntry() { - initPerformanceEntry(this, name, type, start, duration); - }, [], PerformanceEntry); + const entry = new PerformanceEntry(kSkipThrow); + + initPerformanceEntry(entry, name, type, start, duration); + + return entry; } /** @@ -119,10 +123,12 @@ class PerformanceNodeEntry extends PerformanceEntry { } function createPerformanceNodeEntry(name, type, start, duration, detail) { - return ReflectConstruct(function PerformanceNodeEntry() { - initPerformanceEntry(this, name, type, start, duration); - this[kDetail] = detail; - }, [], PerformanceNodeEntry); + const entry = new PerformanceNodeEntry(kSkipThrow); + + initPerformanceEntry(entry, name, type, start, duration); + entry[kDetail] = detail; + + return entry; } module.exports = { diff --git a/lib/internal/streams/writable.js b/lib/internal/streams/writable.js index 718d4e48478677..595aadc23c8bec 100644 --- a/lib/internal/streams/writable.js +++ b/lib/internal/streams/writable.js @@ -74,6 +74,110 @@ function nop() {} const kOnFinished = Symbol('kOnFinished'); +const kObjectMode = 1 << 0; +const kEnded = 1 << 1; +const kConstructed = 1 << 2; +const kSync = 1 << 3; +const kErrorEmitted = 1 << 4; +const kEmitClose = 1 << 5; +const kAutoDestroy = 1 << 6; +const kDestroyed = 1 << 7; +const kClosed = 1 << 8; +const kCloseEmitted = 1 << 9; +const kFinalCalled = 1 << 10; +const kNeedDrain = 1 << 11; +const kEnding = 1 << 12; +const kFinished = 1 << 13; +const kDecodeStrings = 1 << 14; +const kWriting = 1 << 15; +const kBufferProcessing = 1 << 16; +const kPrefinished = 1 << 17; +const kAllBuffers = 1 << 18; +const kAllNoop = 1 << 19; + +// TODO(benjamingr) it is likely slower to do it this way than with free functions +function makeBitMapDescriptor(bit) { + return { + enumerable: false, + get() { return (this.state & bit) !== 0; }, + set(value) { + if (value) this.state |= bit; + else this.state &= ~bit; + }, + }; +} +ObjectDefineProperties(WritableState.prototype, { + // Object stream flag to indicate whether or not this stream + // contains buffers or objects. + objectMode: makeBitMapDescriptor(kObjectMode), + + // if _final has been called. + finalCalled: makeBitMapDescriptor(kFinalCalled), + + // drain event flag. + needDrain: makeBitMapDescriptor(kNeedDrain), + + // At the start of calling end() + ending: makeBitMapDescriptor(kEnding), + + // When end() has been called, and returned. + ended: makeBitMapDescriptor(kEnded), + + // When 'finish' is emitted. + finished: makeBitMapDescriptor(kFinished), + + // Has it been destroyed. + destroyed: makeBitMapDescriptor(kDestroyed), + + // Should we decode strings into buffers before passing to _write? + // this is here so that some node-core streams can optimize string + // handling at a lower level. + decodeStrings: makeBitMapDescriptor(kDecodeStrings), + + // A flag to see when we're in the middle of a write. + writing: makeBitMapDescriptor(kWriting), + + // A flag to be able to tell if the onwrite cb is called immediately, + // or on a later tick. We set this to true at first, because any + // actions that shouldn't happen until "later" should generally also + // not happen before the first write call. + sync: makeBitMapDescriptor(kSync), + + // A flag to know if we're processing previously buffered items, which + // may call the _write() callback in the same tick, so that we don't + // end up in an overlapped onwrite situation. + bufferProcessing: makeBitMapDescriptor(kBufferProcessing), + + // Stream is still being constructed and cannot be + // destroyed until construction finished or failed. + // Async construction is opt in, therefore we start as + // constructed. + constructed: makeBitMapDescriptor(kConstructed), + + // Emit prefinish if the only thing we're waiting for is _write cbs + // This is relevant for synchronous Transform streams. + prefinished: makeBitMapDescriptor(kPrefinished), + + // True if the error was already emitted and should not be thrown again. + errorEmitted: makeBitMapDescriptor(kErrorEmitted), + + // Should close be emitted on destroy. Defaults to true. + emitClose: makeBitMapDescriptor(kEmitClose), + + // Should .destroy() be called after 'finish' (and potentially 'end'). + autoDestroy: makeBitMapDescriptor(kAutoDestroy), + + // Indicates whether the stream has finished destroying. + closed: makeBitMapDescriptor(kClosed), + + // True if close has been emitted or would have been emitted + // depending on emitClose. + closeEmitted: makeBitMapDescriptor(kCloseEmitted), + + allBuffers: makeBitMapDescriptor(kAllBuffers), + allNoop: makeBitMapDescriptor(kAllNoop), +}); + function WritableState(options, stream, isDuplex) { // Duplex streams are both readable and writable, but share // the same options object. @@ -83,13 +187,12 @@ function WritableState(options, stream, isDuplex) { if (typeof isDuplex !== 'boolean') isDuplex = stream instanceof Stream.Duplex; - // Object stream flag to indicate whether or not this stream - // contains buffers or objects. - this.objectMode = !!(options && options.objectMode); + // Bit map field to store WritableState more effciently with 1 bit per field + // instead of a V8 slot per field. + this.state = kSync | kConstructed | kEmitClose | kAutoDestroy; - if (isDuplex) - this.objectMode = this.objectMode || - !!(options && options.writableObjectMode); + if (options && options.objectMode) this.state |= kObjectMode; + if (isDuplex && options && options.writableObjectMode) this.state |= kObjectMode; // The point at which write() starts returning false // Note: 0 is a valid value, means that we always return false if @@ -98,26 +201,13 @@ function WritableState(options, stream, isDuplex) { getHighWaterMark(this, options, 'writableHighWaterMark', isDuplex) : getDefaultHighWaterMark(false); - // if _final has been called. - this.finalCalled = false; + if (!options || options.decodeStrings !== false) this.state |= kDecodeStrings; - // drain event flag. - this.needDrain = false; - // At the start of calling end() - this.ending = false; - // When end() has been called, and returned. - this.ended = false; - // When 'finish' is emitted. - this.finished = false; + // Should close be emitted on destroy. Defaults to true. + if (options && options.emitClose === false) this.state &= ~kEmitClose; - // Has it been destroyed - this.destroyed = false; - - // Should we decode strings into buffers before passing to _write? - // this is here so that some node-core streams can optimize string - // handling at a lower level. - const noDecode = !!(options && options.decodeStrings === false); - this.decodeStrings = !noDecode; + // Should .destroy() be called after 'end' (and potentially 'finish'). + if (options && options.autoDestroy === false) this.state &= ~kAutoDestroy; // Crypto is kind of old and crusty. Historically, its default string // encoding is 'binary' so we have to make this configurable. @@ -136,23 +226,9 @@ function WritableState(options, stream, isDuplex) { // socket or file. this.length = 0; - // A flag to see when we're in the middle of a write. - this.writing = false; - // When true all writes will be buffered until .uncork() call. this.corked = 0; - // A flag to be able to tell if the onwrite cb is called immediately, - // or on a later tick. We set this to true at first, because any - // actions that shouldn't happen until "later" should generally also - // not happen before the first write call. - this.sync = true; - - // A flag to know if we're processing previously buffered items, which - // may call the _write() callback in the same tick, so that we don't - // end up in an overlapped onwrite situation. - this.bufferProcessing = false; - // The callback that's passed to _write(chunk, cb). this.onwrite = onwrite.bind(undefined, stream); @@ -172,45 +248,18 @@ function WritableState(options, stream, isDuplex) { // this must be 0 before 'finish' can be emitted. this.pendingcb = 0; - // Stream is still being constructed and cannot be - // destroyed until construction finished or failed. - // Async construction is opt in, therefore we start as - // constructed. - this.constructed = true; - - // Emit prefinish if the only thing we're waiting for is _write cbs - // This is relevant for synchronous Transform streams. - this.prefinished = false; - - // True if the error was already emitted and should not be thrown again. - this.errorEmitted = false; - - // Should close be emitted on destroy. Defaults to true. - this.emitClose = !options || options.emitClose !== false; - - // Should .destroy() be called after 'finish' (and potentially 'end'). - this.autoDestroy = !options || options.autoDestroy !== false; - // Indicates whether the stream has errored. When true all write() calls // should return false. This is needed since when autoDestroy // is disabled we need a way to tell whether the stream has failed. this.errored = null; - // Indicates whether the stream has finished destroying. - this.closed = false; - - // True if close has been emitted or would have been emitted - // depending on emitClose. - this.closeEmitted = false; - this[kOnFinished] = []; } function resetBuffer(state) { state.buffered = []; state.bufferedIndex = 0; - state.allBuffers = true; - state.allNoop = true; + state.state |= kAllBuffers | kAllNoop; } WritableState.prototype.getBuffer = function getBuffer() { @@ -307,9 +356,9 @@ function _write(stream, chunk, encoding, cb) { if (chunk === null) { throw new ERR_STREAM_NULL_VALUES(); - } else if (!state.objectMode) { + } else if ((state.state & kObjectMode) === 0) { if (typeof chunk === 'string') { - if (state.decodeStrings !== false) { + if ((state.state & kDecodeStrings) !== 0) { chunk = Buffer.from(chunk, encoding); encoding = 'buffer'; } @@ -325,9 +374,9 @@ function _write(stream, chunk, encoding, cb) { } let err; - if (state.ending) { + if ((state.state & kEnding) !== 0) { err = new ERR_STREAM_WRITE_AFTER_END(); - } else if (state.destroyed) { + } else if ((state.state & kDestroyed) !== 0) { err = new ERR_STREAM_DESTROYED('write'); } @@ -354,7 +403,7 @@ Writable.prototype.uncork = function() { if (state.corked) { state.corked--; - if (!state.writing) + if ((state.state & kWriting) === 0) clearBuffer(this, state); } }; @@ -373,7 +422,7 @@ Writable.prototype.setDefaultEncoding = function setDefaultEncoding(encoding) { // in the queue, and wait our turn. Otherwise, call _write // If we return false, then we need a drain event, so set that flag. function writeOrBuffer(stream, state, chunk, encoding, callback) { - const len = state.objectMode ? 1 : chunk.length; + const len = (state.state & kObjectMode) !== 0 ? 1 : chunk.length; state.length += len; @@ -381,42 +430,40 @@ function writeOrBuffer(stream, state, chunk, encoding, callback) { const ret = state.length < state.highWaterMark; // We must ensure that previous needDrain will not be reset to false. if (!ret) - state.needDrain = true; + state.state |= kNeedDrain; - if (state.writing || state.corked || state.errored || !state.constructed) { + if ((state.state & kWriting) !== 0 || state.corked || state.errored || (state.state & kConstructed) === 0) { state.buffered.push({ chunk, encoding, callback }); - if (state.allBuffers && encoding !== 'buffer') { - state.allBuffers = false; + if ((state.state & kAllBuffers) !== 0 && encoding !== 'buffer') { + state.state &= ~kAllBuffers; } - if (state.allNoop && callback !== nop) { - state.allNoop = false; + if ((state.state & kAllNoop) !== 0 && callback !== nop) { + state.state &= ~kAllNoop; } } else { state.writelen = len; state.writecb = callback; - state.writing = true; - state.sync = true; + state.state |= kWriting | kSync; stream._write(chunk, encoding, state.onwrite); - state.sync = false; + state.state &= ~kSync; } // Return false if errored or destroyed in order to break // any synchronous while(stream.write(data)) loops. - return ret && !state.errored && !state.destroyed; + return ret && !state.errored && (state.state & kDestroyed) === 0; } function doWrite(stream, state, writev, len, chunk, encoding, cb) { state.writelen = len; state.writecb = cb; - state.writing = true; - state.sync = true; - if (state.destroyed) + state.state |= kWriting | kSync; + if ((state.state & kDestroyed) !== 0) state.onwrite(new ERR_STREAM_DESTROYED('write')); else if (writev) stream._writev(chunk, state.onwrite); else stream._write(chunk, encoding, state.onwrite); - state.sync = false; + state.state &= ~kSync; } function onwriteError(stream, state, er, cb) { @@ -434,7 +481,7 @@ function onwriteError(stream, state, er, cb) { function onwrite(stream, er) { const state = stream._writableState; - const sync = state.sync; + const sync = (state.state & kSync) !== 0; const cb = state.writecb; if (typeof cb !== 'function') { @@ -442,7 +489,7 @@ function onwrite(stream, er) { return; } - state.writing = false; + state.state &= ~kWriting; state.writecb = null; state.length -= state.writelen; state.writelen = 0; @@ -495,10 +542,9 @@ function afterWriteTick({ stream, state, count, cb }) { } function afterWrite(stream, state, count, cb) { - const needDrain = !state.ending && !stream.destroyed && state.length === 0 && - state.needDrain; + const needDrain = (state.state & (kEnding | kNeedDrain)) === kNeedDrain && !stream.destroyed && state.length === 0; if (needDrain) { - state.needDrain = false; + state.state &= ~kNeedDrain; stream.emit('drain'); } @@ -507,7 +553,7 @@ function afterWrite(stream, state, count, cb) { cb(null); } - if (state.destroyed) { + if ((state.state & kDestroyed) !== 0) { errorBuffer(state); } @@ -516,13 +562,13 @@ function afterWrite(stream, state, count, cb) { // If there's something in the buffer waiting, then invoke callbacks. function errorBuffer(state) { - if (state.writing) { + if ((state.state & kWriting) !== 0) { return; } for (let n = state.bufferedIndex; n < state.buffered.length; ++n) { const { chunk, callback } = state.buffered[n]; - const len = state.objectMode ? 1 : chunk.length; + const len = (state.state & kObjectMode) !== 0 ? 1 : chunk.length; state.length -= len; callback(state.errored ?? new ERR_STREAM_DESTROYED('write')); } @@ -538,13 +584,13 @@ function errorBuffer(state) { // If there's something in the buffer waiting, then process it. function clearBuffer(stream, state) { if (state.corked || - state.bufferProcessing || - state.destroyed || - !state.constructed) { + (state.state & (kDestroyed | kBufferProcessing)) !== 0 || + (state.state & kConstructed) === 0) { return; } - const { buffered, bufferedIndex, objectMode } = state; + const objectMode = (state.state & kObjectMode) !== 0; + const { buffered, bufferedIndex } = state; const bufferedLength = buffered.length - bufferedIndex; if (!bufferedLength) { @@ -553,20 +599,20 @@ function clearBuffer(stream, state) { let i = bufferedIndex; - state.bufferProcessing = true; + state.state |= kBufferProcessing; if (bufferedLength > 1 && stream._writev) { state.pendingcb -= bufferedLength - 1; - const callback = state.allNoop ? nop : (err) => { + const callback = (state.state & kAllNoop) !== 0 ? nop : (err) => { for (let n = i; n < buffered.length; ++n) { buffered[n].callback(err); } }; // Make a copy of `buffered` if it's going to be used by `callback` above, // since `doWrite` will mutate the array. - const chunks = state.allNoop && i === 0 ? + const chunks = (state.state & kAllNoop) !== 0 && i === 0 ? buffered : ArrayPrototypeSlice(buffered, i); - chunks.allBuffers = state.allBuffers; + chunks.allBuffers = (state.state & kAllBuffers) !== 0; doWrite(stream, state, true, state.length, chunks, '', callback); @@ -577,7 +623,7 @@ function clearBuffer(stream, state) { buffered[i++] = null; const len = objectMode ? 1 : chunk.length; doWrite(stream, state, false, len, chunk, encoding, callback); - } while (i < buffered.length && !state.writing); + } while (i < buffered.length && (state.state & kWriting) === 0); if (i === buffered.length) { resetBuffer(state); @@ -588,7 +634,7 @@ function clearBuffer(stream, state) { state.bufferedIndex = i; } } - state.bufferProcessing = false; + state.state &= ~kBufferProcessing; } Writable.prototype._write = function(chunk, encoding, cb) { @@ -630,26 +676,26 @@ Writable.prototype.end = function(chunk, encoding, cb) { if (err) { // Do nothing... - } else if (!state.errored && !state.ending) { + } else if (!state.errored && (state.state & kEnding) === 0) { // This is forgiving in terms of unnecessary calls to end() and can hide // logic errors. However, usually such errors are harmless and causing a // hard error can be disproportionately destructive. It is not always // trivial for the user to determine whether end() needs to be called // or not. - state.ending = true; + state.state |= kEnding; finishMaybe(this, state, true); - state.ended = true; - } else if (state.finished) { + state.state |= kEnded; + } else if ((state.state & kFinished) !== 0) { err = new ERR_STREAM_ALREADY_FINISHED('end'); - } else if (state.destroyed) { + } else if ((state.state & kDestroyed) !== 0) { err = new ERR_STREAM_DESTROYED('end'); } if (typeof cb === 'function') { if (err) { process.nextTick(cb, err); - } else if (state.finished) { + } else if ((state.state & kFinished) !== 0) { process.nextTick(cb, null); } else { state[kOnFinished].push(cb); @@ -660,16 +706,20 @@ Writable.prototype.end = function(chunk, encoding, cb) { }; function needFinish(state) { - return (state.ending && - !state.destroyed && - state.constructed && + return ( + // State is ended && constructed but not destroyed, finished, writing, errorEmitted or closedEmitted + (state.state & ( + kEnding | + kDestroyed | + kConstructed | + kFinished | + kWriting | + kErrorEmitted | + kCloseEmitted + )) === (kEnding | kConstructed) && state.length === 0 && !state.errored && - state.buffered.length === 0 && - !state.finished && - !state.writing && - !state.errorEmitted && - !state.closeEmitted); + state.buffered.length === 0); } function callFinal(stream, state) { @@ -688,9 +738,9 @@ function callFinal(stream, state) { for (let i = 0; i < onfinishCallbacks.length; i++) { onfinishCallbacks[i](err); } - errorOrDestroy(stream, err, state.sync); + errorOrDestroy(stream, err, (state.state & kSync) !== 0); } else if (needFinish(state)) { - state.prefinished = true; + state.state |= kPrefinished; stream.emit('prefinish'); // Backwards compat. Don't check state.sync here. // Some streams assume 'finish' will be emitted @@ -700,7 +750,7 @@ function callFinal(stream, state) { } } - state.sync = true; + state.state |= kSync; state.pendingcb++; try { @@ -709,16 +759,16 @@ function callFinal(stream, state) { onFinish(err); } - state.sync = false; + state.state &= ~kSync; } function prefinish(stream, state) { - if (!state.prefinished && !state.finalCalled) { - if (typeof stream._final === 'function' && !state.destroyed) { - state.finalCalled = true; + if ((state.state & (kPrefinished | kFinalCalled)) === 0) { + if (typeof stream._final === 'function' && (state.state & kDestroyed) === 0) { + state.state |= kFinalCalled; callFinal(stream, state); } else { - state.prefinished = true; + state.state |= kPrefinished; stream.emit('prefinish'); } } @@ -747,7 +797,7 @@ function finishMaybe(stream, state, sync) { function finish(stream, state) { state.pendingcb--; - state.finished = true; + state.state |= kFinished; const onfinishCallbacks = state[kOnFinished].splice(0); for (let i = 0; i < onfinishCallbacks.length; i++) { @@ -756,7 +806,7 @@ function finish(stream, state) { stream.emit('finish'); - if (state.autoDestroy) { + if ((state.state & kAutoDestroy) !== 0) { // In case of duplex streams we need a way to detect // if the readable side is ready for autoDestroy as well. const rState = stream._readableState; @@ -777,20 +827,21 @@ ObjectDefineProperties(Writable.prototype, { closed: { __proto__: null, get() { - return this._writableState ? this._writableState.closed : false; + return this._writableState ? (this._writableState.state & kClosed) !== 0 : false; }, }, destroyed: { __proto__: null, get() { - return this._writableState ? this._writableState.destroyed : false; + return this._writableState ? (this._writableState.state & kDestroyed) !== 0 : false; }, set(value) { // Backward compatibility, the user is explicitly managing destroyed. - if (this._writableState) { - this._writableState.destroyed = value; - } + if (!this._writableState) return; + + if (value) this._writableState.state |= kDestroyed; + else this._writableState.state &= ~kDestroyed; }, }, @@ -802,8 +853,8 @@ ObjectDefineProperties(Writable.prototype, { // where the writable side was disabled upon construction. // Compat. The user might manually disable writable side through // deprecated setter. - return !!w && w.writable !== false && !w.destroyed && !w.errored && - !w.ending && !w.ended; + return !!w && w.writable !== false && !w.errored && + (w.state & (kEnding | kEnded | kDestroyed)) === 0; }, set(val) { // Backwards compatible. @@ -816,14 +867,14 @@ ObjectDefineProperties(Writable.prototype, { writableFinished: { __proto__: null, get() { - return this._writableState ? this._writableState.finished : false; + return this._writableState ? (this._writableState.state & kFinished) !== 0 : false; }, }, writableObjectMode: { __proto__: null, get() { - return this._writableState ? this._writableState.objectMode : false; + return this._writableState ? (this._writableState.state & kObjectMode) !== 0 : false; }, }, @@ -837,7 +888,7 @@ ObjectDefineProperties(Writable.prototype, { writableEnded: { __proto__: null, get() { - return this._writableState ? this._writableState.ending : false; + return this._writableState ? (this._writableState.state & kEnding) !== 0 : false; }, }, @@ -846,7 +897,9 @@ ObjectDefineProperties(Writable.prototype, { get() { const wState = this._writableState; if (!wState) return false; - return !wState.destroyed && !wState.ending && wState.needDrain; + + // !destroyed && !ending && needDrain + return (wState.state & (kDestroyed | kEnding | kNeedDrain)) === kNeedDrain; }, }, @@ -885,8 +938,8 @@ ObjectDefineProperties(Writable.prototype, { get: function() { return !!( this._writableState.writable !== false && - (this._writableState.destroyed || this._writableState.errored) && - !this._writableState.finished + ((this._writableState.state & kDestroyed) !== 0 || this._writableState.errored) && + (this._writableState.state & kFinished) === 0 ); }, }, @@ -897,7 +950,7 @@ Writable.prototype.destroy = function(err, cb) { const state = this._writableState; // Invoke pending callbacks. - if (!state.destroyed && + if ((state.state & kDestroyed) === 0 && (state.bufferedIndex < state.buffered.length || state[kOnFinished].length)) { process.nextTick(errorBuffer, state); diff --git a/lib/internal/url.js b/lib/internal/url.js index 37f67e6792959c..8d5926e8fcb9df 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -772,13 +772,7 @@ class URL { base = `${base}`; } - const href = bindingUrl.parse(input, base); - - if (!href) { - throw new ERR_INVALID_URL(input); - } - - this.#updateContext(href); + this.#updateContext(bindingUrl.parse(input, base)); } [inspect.custom](depth, opts) { diff --git a/lib/internal/vm.js b/lib/internal/vm.js index b14ba13e7e4cfb..ba5e2324667374 100644 --- a/lib/internal/vm.js +++ b/lib/internal/vm.js @@ -100,9 +100,10 @@ function internalCompileFunction(code, params, options) { const { importModuleDynamicallyWrap } = require('internal/vm/module'); const wrapped = importModuleDynamicallyWrap(importModuleDynamically); const func = result.function; - const { setCallbackForWrap } = require('internal/modules/esm/utils'); - setCallbackForWrap(result.cacheKey, { - importModuleDynamically: (s, _k, i) => wrapped(s, func, i), + const { registerModule } = require('internal/modules/esm/utils'); + registerModule(func, { + __proto__: null, + importModuleDynamically: wrapped, }); } diff --git a/lib/internal/vm/module.js b/lib/internal/vm/module.js index 3d2d25064b62cd..c77ee9b107e0ad 100644 --- a/lib/internal/vm/module.js +++ b/lib/internal/vm/module.js @@ -11,7 +11,6 @@ const { ObjectSetPrototypeOf, ReflectApply, SafePromiseAllReturnVoid, - SafeWeakMap, Symbol, SymbolToStringTag, TypeError, @@ -69,7 +68,6 @@ const STATUS_MAP = { let globalModuleId = 0; const defaultModuleName = 'vm:module'; -const wrapToModuleMap = new SafeWeakMap(); const kWrap = Symbol('kWrap'); const kContext = Symbol('kContext'); @@ -120,17 +118,18 @@ class Module { }); } + let registry = { __proto__: null }; if (sourceText !== undefined) { this[kWrap] = new ModuleWrap(identifier, context, sourceText, options.lineOffset, options.columnOffset, options.cachedData); - const { setCallbackForWrap } = require('internal/modules/esm/utils'); - setCallbackForWrap(this[kWrap], { + registry = { + __proto__: null, initializeImportMeta: options.initializeImportMeta, importModuleDynamically: options.importModuleDynamically ? importModuleDynamicallyWrap(options.importModuleDynamically) : undefined, - }); + }; } else { assert(syntheticEvaluationSteps); this[kWrap] = new ModuleWrap(identifier, context, @@ -138,7 +137,11 @@ class Module { syntheticEvaluationSteps); } - wrapToModuleMap.set(this[kWrap], this); + // This will take precedence over the referrer as the object being + // passed into the callbacks. + registry.callbackReferrer = this; + const { registerModule } = require('internal/modules/esm/utils'); + registerModule(this[kWrap], registry); this[kContext] = context; } @@ -445,5 +448,4 @@ module.exports = { SourceTextModule, SyntheticModule, importModuleDynamicallyWrap, - getModuleFromWrap: (wrap) => wrapToModuleMap.get(wrap), }; diff --git a/lib/vm.js b/lib/vm.js index 515c7afb4aedb9..4b9bedec3f4934 100644 --- a/lib/vm.js +++ b/lib/vm.js @@ -105,8 +105,9 @@ class Script extends ContextifyScript { validateFunction(importModuleDynamically, 'options.importModuleDynamically'); const { importModuleDynamicallyWrap } = require('internal/vm/module'); - const { setCallbackForWrap } = require('internal/modules/esm/utils'); - setCallbackForWrap(this, { + const { registerModule } = require('internal/modules/esm/utils'); + registerModule(this, { + __proto__: null, importModuleDynamically: importModuleDynamicallyWrap(importModuleDynamically), }); diff --git a/onboarding.md b/onboarding.md index 1e912f0c8a4379..be393a64f3fc67 100644 --- a/onboarding.md +++ b/onboarding.md @@ -10,7 +10,7 @@ onboarding session. possible to add them to the organization if they are not using two-factor authentication. If they cannot receive SMS messages from GitHub, try [using a TOTP mobile app][]. -* Suggest the new Collaborator install [`node-core-utils`][] and +* Suggest the new Collaborator install [`@node-core/utils`][] and [set up the credentials][] for it. ## Fifteen minutes before the onboarding session @@ -230,7 +230,7 @@ needs to be pointed out separately during the onboarding. request. * Be sure to add the `PR-URL: ` and appropriate `Reviewed-By:` metadata. - * [`node-core-utils`][] automates the generation of metadata and the landing + * [`@node-core/utils`][] automates the generation of metadata and the landing process. See the documentation of [`git-node`][]. * [`core-validate-commit`][] automates the validation of commit messages. This will be run during `git node land --final` of the [`git-node`][] @@ -260,10 +260,10 @@ needs to be pointed out separately during the onboarding. [Labels]: doc/contributing/collaborator-guide.md#labels [Landing pull requests]: doc/contributing/collaborator-guide.md#landing-pull-requests [Publicizing or hiding organization membership]: https://help.github.com/articles/publicizing-or-hiding-organization-membership/ +[`@node-core/utils`]: https://github.com/nodejs/node-core-utils [`author-ready`]: doc/contributing/collaborator-guide.md#author-ready-pull-requests [`core-validate-commit`]: https://github.com/nodejs/core-validate-commit [`git-node`]: https://github.com/nodejs/node-core-utils/blob/HEAD/docs/git-node.md -[`node-core-utils`]: https://github.com/nodejs/node-core-utils [set up the credentials]: https://github.com/nodejs/node-core-utils#setting-up-github-credentials [static-analysis]: doc/contributing/static-analysis.md [two-factor authentication]: https://help.github.com/articles/securing-your-account-with-two-factor-authentication-2fa/ diff --git a/src/env-inl.h b/src/env-inl.h index 7802304b1891ae..793dc72e0dbad8 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -393,16 +393,6 @@ inline AliasedInt32Array& Environment::stream_base_state() { return stream_base_state_; } -inline uint32_t Environment::get_next_module_id() { - return module_id_counter_++; -} -inline uint32_t Environment::get_next_script_id() { - return script_id_counter_++; -} -inline uint32_t Environment::get_next_function_id() { - return function_id_counter_++; -} - ShouldNotAbortOnUncaughtScope::ShouldNotAbortOnUncaughtScope( Environment* env) : env_(env) { diff --git a/src/env.h b/src/env.h index c02fc6bd62dd78..afe67d2237ae69 100644 --- a/src/env.h +++ b/src/env.h @@ -746,14 +746,6 @@ class Environment : public MemoryRetainer { builtins::BuiltinLoader* builtin_loader(); std::unordered_multimap hash_to_module_map; - std::unordered_map id_to_module_map; - std::unordered_map - id_to_script_map; - std::unordered_map id_to_function_map; - - inline uint32_t get_next_module_id(); - inline uint32_t get_next_script_id(); - inline uint32_t get_next_function_id(); EnabledDebugList* enabled_debug_list() { return &enabled_debug_list_; } diff --git a/src/env_properties.h b/src/env_properties.h index 970e25d926dbb2..26e45695957b7a 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -21,6 +21,7 @@ V(arrow_message_private_symbol, "node:arrowMessage") \ V(contextify_context_private_symbol, "node:contextify:context") \ V(decorated_private_symbol, "node:decorated") \ + V(host_defined_option_symbol, "node:host_defined_option_symbol") \ V(napi_type_tag, "node:napi:type_tag") \ V(napi_wrapper, "node:napi:wrapper") \ V(untransferable_object_private_symbol, "node:untransferableObject") \ @@ -54,6 +55,7 @@ V(args_string, "args") \ V(asn1curve_string, "asn1Curve") \ V(async_ids_stack_string, "async_ids_stack") \ + V(base_string, "base") \ V(bits_string, "bits") \ V(block_list_string, "blockList") \ V(buffer_string, "buffer") \ @@ -338,7 +340,6 @@ V(blocklist_constructor_template, v8::FunctionTemplate) \ V(contextify_global_template, v8::ObjectTemplate) \ V(contextify_wrapper_template, v8::ObjectTemplate) \ - V(compiled_fn_entry_template, v8::ObjectTemplate) \ V(crypto_key_object_handle_constructor, v8::FunctionTemplate) \ V(env_proxy_template, v8::ObjectTemplate) \ V(env_proxy_ctor_template, v8::FunctionTemplate) \ diff --git a/src/module_wrap.cc b/src/module_wrap.cc index b96106a39744b9..a1b0f812391486 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -38,13 +38,13 @@ using v8::MaybeLocal; using v8::MicrotaskQueue; using v8::Module; using v8::ModuleRequest; -using v8::Number; using v8::Object; using v8::PrimitiveArray; using v8::Promise; using v8::ScriptCompiler; using v8::ScriptOrigin; using v8::String; +using v8::Symbol; using v8::UnboundModuleScript; using v8::Undefined; using v8::Value; @@ -57,9 +57,8 @@ ModuleWrap::ModuleWrap(Environment* env, Local synthetic_evaluation_step) : BaseObject(env, object), module_(env->isolate(), module), - id_(env->get_next_module_id()) { - env->id_to_module_map.emplace(id_, this); - + module_hash_(module->GetIdentityHash()) { + object->SetInternalFieldForNodeCore(kModuleSlot, module); object->SetInternalField(kURLSlot, url); object->SetInternalField(kSyntheticEvaluationStepsSlot, synthetic_evaluation_step); @@ -68,13 +67,13 @@ ModuleWrap::ModuleWrap(Environment* env, if (!synthetic_evaluation_step->IsUndefined()) { synthetic_ = true; } + MakeWeak(); + module_.SetWeak(); } ModuleWrap::~ModuleWrap() { HandleScope scope(env()->isolate()); - Local module = module_.Get(env()->isolate()); - env()->id_to_module_map.erase(id_); - auto range = env()->hash_to_module_map.equal_range(module->GetIdentityHash()); + auto range = env()->hash_to_module_map.equal_range(module_hash_); for (auto it = range.first; it != range.second; ++it) { if (it->second == this) { env()->hash_to_module_map.erase(it); @@ -102,14 +101,6 @@ ModuleWrap* ModuleWrap::GetFromModule(Environment* env, return nullptr; } -ModuleWrap* ModuleWrap::GetFromID(Environment* env, uint32_t id) { - auto module_wrap_it = env->id_to_module_map.find(id); - if (module_wrap_it == env->id_to_module_map.end()) { - return nullptr; - } - return module_wrap_it->second; -} - // new ModuleWrap(url, context, source, lineOffset, columnOffset) // new ModuleWrap(url, context, exportNames, syntheticExecutionFunction) void ModuleWrap::New(const FunctionCallbackInfo& args) { @@ -154,8 +145,8 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { Local host_defined_options = PrimitiveArray::New(isolate, HostDefinedOptions::kLength); - host_defined_options->Set(isolate, HostDefinedOptions::kType, - Number::New(isolate, ScriptType::kModule)); + Local id_symbol = Symbol::New(isolate, url); + host_defined_options->Set(isolate, HostDefinedOptions::kID, id_symbol); ShouldNotAbortOnUncaughtScope no_abort_scope(env); TryCatchScope try_catch(env); @@ -235,6 +226,11 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { return; } + if (that->SetPrivate(context, env->host_defined_option_symbol(), id_symbol) + .IsNothing()) { + return; + } + // Use the extras object as an object whose GetCreationContext() will be the // original `context`, since the `Context` itself strictly speaking cannot // be stored in an internal field. @@ -249,9 +245,6 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { env->hash_to_module_map.emplace(module->GetIdentityHash(), obj); - host_defined_options->Set(isolate, HostDefinedOptions::kID, - Number::New(isolate, obj->id())); - that->SetIntegrityLevel(context, IntegrityLevel::kFrozen); args.GetReturnValue().Set(that); } @@ -586,35 +579,16 @@ static MaybeLocal ImportModuleDynamically( Local object; - int type = options->Get(context, HostDefinedOptions::kType) - .As() - ->Int32Value(context) - .ToChecked(); - uint32_t id = options->Get(context, HostDefinedOptions::kID) - .As() - ->Uint32Value(context) - .ToChecked(); - if (type == ScriptType::kScript) { - contextify::ContextifyScript* wrap = env->id_to_script_map.find(id)->second; - object = wrap->object(); - } else if (type == ScriptType::kModule) { - ModuleWrap* wrap = ModuleWrap::GetFromID(env, id); - object = wrap->object(); - } else if (type == ScriptType::kFunction) { - auto it = env->id_to_function_map.find(id); - CHECK_NE(it, env->id_to_function_map.end()); - object = it->second->object(); - } else { - UNREACHABLE(); - } + Local id = + options->Get(context, HostDefinedOptions::kID).As(); Local assertions = createImportAssertionContainer(env, isolate, import_assertions); Local import_args[] = { - object, - Local(specifier), - assertions, + id, + Local(specifier), + assertions, }; Local result; @@ -658,7 +632,13 @@ void ModuleWrap::HostInitializeImportMetaObjectCallback( Local wrap = module_wrap->object(); Local callback = env->host_initialize_import_meta_object_callback(); - Local args[] = { wrap, meta }; + Local id; + if (!wrap->GetPrivate(context, env->host_defined_option_symbol()) + .ToLocal(&id)) { + return; + } + DCHECK(id->IsSymbol()); + Local args[] = {id, meta}; TryCatchScope try_catch(env); USE(callback->Call( context, Undefined(env->isolate()), arraysize(args), args)); diff --git a/src/module_wrap.h b/src/module_wrap.h index a3d3386763af85..6435bad40936fe 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -26,15 +26,14 @@ enum ScriptType : int { }; enum HostDefinedOptions : int { - kType = 8, - kID = 9, - kLength = 10, + kID = 8, + kLength = 9, }; class ModuleWrap : public BaseObject { public: enum InternalFields { - kModuleWrapBaseField = BaseObject::kInternalFieldCount, + kModuleSlot = BaseObject::kInternalFieldCount, kURLSlot, kSyntheticEvaluationStepsSlot, kContextObjectSlot, // Object whose creation context is the target Context @@ -55,9 +54,7 @@ class ModuleWrap : public BaseObject { tracker->TrackField("resolve_cache", resolve_cache_); } - inline uint32_t id() { return id_; } v8::Local context() const; - static ModuleWrap* GetFromID(node::Environment*, uint32_t id); SET_MEMORY_INFO_NAME(ModuleWrap) SET_SELF_SIZE(ModuleWrap) @@ -109,7 +106,7 @@ class ModuleWrap : public BaseObject { contextify::ContextifyContext* contextify_context_ = nullptr; bool synthetic_ = false; bool linked_ = false; - uint32_t id_; + int module_hash_; }; } // namespace loader diff --git a/src/node_api.cc b/src/node_api.cc index 7537dc20b2bd82..368f05f3f4a261 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -82,9 +82,8 @@ void node_napi_env__::trigger_fatal_exception(v8::Local local_err) { node::errors::TriggerUncaughtException(isolate, local_err, local_msg); } -// option enforceUncaughtExceptionPolicy is added for not breaking existing -// running n-api add-ons, and should be deprecated in the next major Node.js -// release. +// The option enforceUncaughtExceptionPolicy is added for not breaking existing +// running Node-API add-ons. template void node_napi_env__::CallbackIntoModule(T&& call) { CallIntoModule(call, [](napi_env env_, v8::Local local_err) { @@ -93,19 +92,24 @@ void node_napi_env__::CallbackIntoModule(T&& call) { return; } node::Environment* node_env = env->node_env(); - if (!node_env->options()->force_node_api_uncaught_exceptions_policy && + // If the module api version is less than NAPI_VERSION_EXPERIMENTAL, + // and the option --force-node-api-uncaught-exceptions-policy is not + // specified, emit a warning about the uncaught exception instead of + // triggering uncaught exception event. + if (env->module_api_version < NAPI_VERSION_EXPERIMENTAL && + !node_env->options()->force_node_api_uncaught_exceptions_policy && !enforceUncaughtExceptionPolicy) { ProcessEmitDeprecationWarning( node_env, "Uncaught N-API callback exception detected, please run node " - "with option --force-node-api-uncaught-exceptions-policy=true" + "with option --force-node-api-uncaught-exceptions-policy=true " "to handle those exceptions properly.", "DEP0168"); return; } // If there was an unhandled exception in the complete callback, // report it as a fatal exception. (There is no JavaScript on the - // callstack that can possibly handle it.) + // call stack that can possibly handle it.) env->trigger_fatal_exception(local_err); }); } diff --git a/src/node_contextify.cc b/src/node_contextify.cc index a557f5bd9f3b35..75208c7293863e 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -60,7 +60,6 @@ using v8::MicrotasksPolicy; using v8::Name; using v8::NamedPropertyHandlerConfiguration; using v8::Nothing; -using v8::Number; using v8::Object; using v8::ObjectTemplate; using v8::PrimitiveArray; @@ -73,11 +72,11 @@ using v8::Script; using v8::ScriptCompiler; using v8::ScriptOrigin; using v8::String; +using v8::Symbol; using v8::Uint32; using v8::UnboundScript; using v8::Value; using v8::WeakCallbackInfo; -using v8::WeakCallbackType; // The vm module executes code in a sandboxed environment with a different // global object than the rest of the code. This is achieved by applying @@ -817,10 +816,9 @@ void ContextifyScript::New(const FunctionCallbackInfo& args) { Local host_defined_options = PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength); - host_defined_options->Set(isolate, loader::HostDefinedOptions::kType, - Number::New(isolate, loader::ScriptType::kScript)); - host_defined_options->Set(isolate, loader::HostDefinedOptions::kID, - Number::New(isolate, contextify_script->id())); + Local id_symbol = Symbol::New(isolate, filename); + host_defined_options->Set( + isolate, loader::HostDefinedOptions::kID, id_symbol); ScriptOrigin origin(isolate, filename, @@ -857,13 +855,23 @@ void ContextifyScript::New(const FunctionCallbackInfo& args) { "ContextifyScript::New"); return; } + contextify_script->script_.Reset(isolate, v8_script); + contextify_script->script_.SetWeak(); + contextify_script->object()->SetInternalFieldForNodeCore(kUnboundScriptSlot, + v8_script); std::unique_ptr new_cached_data; if (produce_cached_data) { new_cached_data.reset(ScriptCompiler::CreateCodeCache(v8_script)); } + if (contextify_script->object() + ->SetPrivate(context, env->host_defined_option_symbol(), id_symbol) + .IsNothing()) { + return; + } + if (StoreCodeCacheResult(env, args.This(), compile_options, @@ -1116,17 +1124,11 @@ bool ContextifyScript::EvalMachine(Local context, } ContextifyScript::ContextifyScript(Environment* env, Local object) - : BaseObject(env, object), - id_(env->get_next_script_id()) { + : BaseObject(env, object) { MakeWeak(); - env->id_to_script_map.emplace(id_, this); -} - - -ContextifyScript::~ContextifyScript() { - env()->id_to_script_map.erase(id_); } +ContextifyScript::~ContextifyScript() {} void ContextifyContext::CompileFunction( const FunctionCallbackInfo& args) { @@ -1196,18 +1198,12 @@ void ContextifyContext::CompileFunction( data + cached_data_buf->ByteOffset(), cached_data_buf->ByteLength()); } - // Get the function id - uint32_t id = env->get_next_function_id(); - // Set host_defined_options Local host_defined_options = PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength); + Local id_symbol = Symbol::New(isolate, filename); host_defined_options->Set( - isolate, - loader::HostDefinedOptions::kType, - Number::New(isolate, loader::ScriptType::kFunction)); - host_defined_options->Set( - isolate, loader::HostDefinedOptions::kID, Number::New(isolate, id)); + isolate, loader::HostDefinedOptions::kID, id_symbol); ScriptOrigin origin(isolate, filename, @@ -1272,21 +1268,14 @@ void ContextifyContext::CompileFunction( } return; } - - Local cache_key; - if (!env->compiled_fn_entry_template()->NewInstance( - context).ToLocal(&cache_key)) { + if (fn->SetPrivate(context, env->host_defined_option_symbol(), id_symbol) + .IsNothing()) { return; } - CompiledFnEntry* entry = new CompiledFnEntry(env, cache_key, id, fn); - env->id_to_function_map.emplace(id, entry); Local result = Object::New(isolate); if (result->Set(parsing_context, env->function_string(), fn).IsNothing()) return; - if (result->Set(parsing_context, env->cache_key_string(), cache_key) - .IsNothing()) - return; if (result ->Set(parsing_context, env->source_map_url_string(), @@ -1311,25 +1300,6 @@ void ContextifyContext::CompileFunction( args.GetReturnValue().Set(result); } -void CompiledFnEntry::WeakCallback( - const WeakCallbackInfo& data) { - CompiledFnEntry* entry = data.GetParameter(); - delete entry; -} - -CompiledFnEntry::CompiledFnEntry(Environment* env, - Local object, - uint32_t id, - Local fn) - : BaseObject(env, object), id_(id), fn_(env->isolate(), fn) { - fn_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter); -} - -CompiledFnEntry::~CompiledFnEntry() { - env()->id_to_function_map.erase(id_); - fn_.ClearWeak(); -} - static void StartSigintWatchdog(const FunctionCallbackInfo& args) { int ret = SigintWatchdogHelper::GetInstance()->Start(); args.GetReturnValue().Set(ret == 0); @@ -1381,14 +1351,6 @@ void CreatePerIsolateProperties(IsolateData* isolate_data, SetMethodNoSideEffect( isolate, target, "watchdogHasPendingSigint", WatchdogHasPendingSigint); - { - Local tpl = FunctionTemplate::New(isolate); - tpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "CompiledFnEntry")); - tpl->InstanceTemplate()->SetInternalFieldCount( - CompiledFnEntry::kInternalFieldCount); - - isolate_data->set_compiled_fn_entry_template(tpl->InstanceTemplate()); - } SetMethod(isolate, target, "measureMemory", MeasureMemory); } diff --git a/src/node_contextify.h b/src/node_contextify.h index 2bcc15b5f55ad3..d1dddbf374d563 100644 --- a/src/node_contextify.h +++ b/src/node_contextify.h @@ -128,6 +128,11 @@ class ContextifyContext : public BaseObject { class ContextifyScript : public BaseObject { public: + enum InternalFields { + kUnboundScriptSlot = BaseObject::kInternalFieldCount, + kInternalFieldCount + }; + SET_NO_MEMORY_INFO() SET_MEMORY_INFO_NAME(ContextifyScript) SET_SELF_SIZE(ContextifyScript) @@ -151,32 +156,8 @@ class ContextifyScript : public BaseObject { v8::MicrotaskQueue* microtask_queue, const v8::FunctionCallbackInfo& args); - inline uint32_t id() { return id_; } - private: v8::Global script_; - uint32_t id_; -}; - -class CompiledFnEntry final : public BaseObject { - public: - SET_NO_MEMORY_INFO() - SET_MEMORY_INFO_NAME(CompiledFnEntry) - SET_SELF_SIZE(CompiledFnEntry) - - CompiledFnEntry(Environment* env, - v8::Local object, - uint32_t id, - v8::Local fn); - ~CompiledFnEntry(); - - bool IsNotIndicativeOfMemoryLeakAtExit() const override { return true; } - - private: - uint32_t id_; - v8::Global fn_; - - static void WeakCallback(const v8::WeakCallbackInfo& data); }; v8::Maybe StoreCodeCacheResult( diff --git a/src/node_file.cc b/src/node_file.cc index 39d8ac078b435b..59780dec1c4b6d 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -1702,6 +1702,27 @@ static void Unlink(const FunctionCallbackInfo& args) { } } +static void UnlinkSync(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + + const int argc = args.Length(); + CHECK_GE(argc, 1); + + BufferValue path(env->isolate(), args[0]); + CHECK_NOT_NULL(*path); + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, permission::PermissionScope::kFileSystemWrite, path.ToStringView()); + + uv_fs_t req; + auto make = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); }); + FS_SYNC_TRACE_BEGIN(unlink); + int err = uv_fs_unlink(nullptr, &req, *path, nullptr); + FS_SYNC_TRACE_END(unlink); + if (err < 0) { + return env->ThrowUVException(err, "unlink", nullptr, *path); + } +} + static void RMDir(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -2190,7 +2211,7 @@ static void OpenSync(const FunctionCallbackInfo& args) { uv_fs_t req; auto make = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); }); FS_SYNC_TRACE_BEGIN(open); - int err = uv_fs_open(nullptr, &req, *path, flags, mode, nullptr); + auto err = uv_fs_open(nullptr, &req, *path, flags, mode, nullptr); FS_SYNC_TRACE_END(open); if (err < 0) { return env->ThrowUVException(err, "open", nullptr, path.out()); @@ -2581,30 +2602,41 @@ static void ReadFileUtf8(const FunctionCallbackInfo& args) { CHECK_GE(args.Length(), 2); - BufferValue path(env->isolate(), args[0]); - CHECK_NOT_NULL(*path); - CHECK(args[1]->IsInt32()); const int flags = args[1].As()->Value(); - if (CheckOpenPermissions(env, path, flags).IsNothing()) return; - + uv_file file; uv_fs_t req; - auto defer_req_cleanup = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); }); - FS_SYNC_TRACE_BEGIN(open); - uv_file file = uv_fs_open(nullptr, &req, *path, flags, 438, nullptr); - FS_SYNC_TRACE_END(open); - if (req.result < 0) { - // req will be cleaned up by scope leave. - return env->ThrowUVException(req.result, "open", nullptr, path.out()); + bool is_fd = args[0]->IsInt32(); + + // Check for file descriptor + if (is_fd) { + file = args[0].As()->Value(); + } else { + BufferValue path(env->isolate(), args[0]); + CHECK_NOT_NULL(*path); + if (CheckOpenPermissions(env, path, flags).IsNothing()) return; + + FS_SYNC_TRACE_BEGIN(open); + file = uv_fs_open(nullptr, &req, *path, flags, O_RDONLY, nullptr); + FS_SYNC_TRACE_END(open); + if (req.result < 0) { + uv_fs_req_cleanup(&req); + // req will be cleaned up by scope leave. + return env->ThrowUVException(req.result, "open", nullptr, path.out()); + } } - auto defer_close = OnScopeLeave([file]() { - uv_fs_t close_req; - CHECK_EQ(0, uv_fs_close(nullptr, &close_req, file, nullptr)); - uv_fs_req_cleanup(&close_req); + auto defer_close = OnScopeLeave([file, is_fd, &req]() { + if (!is_fd) { + FS_SYNC_TRACE_BEGIN(close); + CHECK_EQ(0, uv_fs_close(nullptr, &req, file, nullptr)); + FS_SYNC_TRACE_END(close); + } + uv_fs_req_cleanup(&req); }); + std::string result{}; char buffer[8192]; uv_buf_t buf = uv_buf_init(buffer, sizeof(buffer)); @@ -2615,7 +2647,7 @@ static void ReadFileUtf8(const FunctionCallbackInfo& args) { if (req.result < 0) { FS_SYNC_TRACE_END(read); // req will be cleaned up by scope leave. - return env->ThrowUVException(req.result, "read", nullptr, path.out()); + return env->ThrowUVException(req.result, "read", nullptr); } if (r <= 0) { break; @@ -3385,15 +3417,15 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data, Isolate* isolate = isolate_data->isolate(); SetMethod(isolate, target, "access", Access); - SetMethodNoSideEffect(isolate, target, "accessSync", AccessSync); + SetMethod(isolate, target, "accessSync", AccessSync); SetMethod(isolate, target, "close", Close); SetMethod(isolate, target, "closeSync", CloseSync); - SetMethodNoSideEffect(isolate, target, "existsSync", ExistsSync); + SetMethod(isolate, target, "existsSync", ExistsSync); SetMethod(isolate, target, "open", Open); SetMethod(isolate, target, "openSync", OpenSync); SetMethod(isolate, target, "openFileHandle", OpenFileHandle); SetMethod(isolate, target, "read", Read); - SetMethodNoSideEffect(isolate, target, "readFileUtf8", ReadFileUtf8); + SetMethod(isolate, target, "readFileUtf8", ReadFileUtf8); SetMethod(isolate, target, "readBuffers", ReadBuffers); SetMethod(isolate, target, "fdatasync", Fdatasync); SetMethod(isolate, target, "fsync", Fsync); @@ -3414,12 +3446,13 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data, SetMethod(isolate, target, "symlink", Symlink); SetMethod(isolate, target, "readlink", ReadLink); SetMethod(isolate, target, "unlink", Unlink); + SetMethod(isolate, target, "unlinkSync", UnlinkSync); SetMethod(isolate, target, "writeBuffer", WriteBuffer); SetMethod(isolate, target, "writeBuffers", WriteBuffers); SetMethod(isolate, target, "writeString", WriteString); SetMethod(isolate, target, "realpath", RealPath); SetMethod(isolate, target, "copyFile", CopyFile); - SetMethodNoSideEffect(isolate, target, "copyFileSync", CopyFileSync); + SetMethod(isolate, target, "copyFileSync", CopyFileSync); SetMethod(isolate, target, "chmod", Chmod); SetMethod(isolate, target, "fchmod", FChmod); @@ -3539,6 +3572,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(Symlink); registry->Register(ReadLink); registry->Register(Unlink); + registry->Register(UnlinkSync); registry->Register(WriteBuffer); registry->Register(WriteBuffers); registry->Register(WriteString); diff --git a/src/node_url.cc b/src/node_url.cc index 666492ca47cee3..89fcfec20f5685 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -227,6 +227,35 @@ void BindingData::Format(const FunctionCallbackInfo& args) { .ToLocalChecked()); } +void BindingData::ThrowInvalidURL(node::Environment* env, + std::string_view input, + std::optional base) { + Local err = ERR_INVALID_URL(env->isolate(), "Invalid URL"); + DCHECK(err->IsObject()); + + auto err_object = err.As(); + + USE(err_object->Set(env->context(), + env->input_string(), + v8::String::NewFromUtf8(env->isolate(), + input.data(), + v8::NewStringType::kNormal, + input.size()) + .ToLocalChecked())); + + if (base.has_value()) { + USE(err_object->Set(env->context(), + env->base_string(), + v8::String::NewFromUtf8(env->isolate(), + base.value().c_str(), + v8::NewStringType::kNormal, + base.value().size()) + .ToLocalChecked())); + } + + env->isolate()->ThrowException(err); +} + void BindingData::Parse(const FunctionCallbackInfo& args) { CHECK_GE(args.Length(), 1); CHECK(args[0]->IsString()); // input @@ -235,15 +264,16 @@ void BindingData::Parse(const FunctionCallbackInfo& args) { Realm* realm = Realm::GetCurrent(args); BindingData* binding_data = realm->GetBindingData(); Isolate* isolate = realm->isolate(); + std::optional base_{}; Utf8Value input(isolate, args[0]); ada::result base; ada::url_aggregator* base_pointer = nullptr; if (args[1]->IsString()) { - base = - ada::parse(Utf8Value(isolate, args[1]).ToString()); + base_ = Utf8Value(isolate, args[1]).ToString(); + base = ada::parse(*base_); if (!base) { - return args.GetReturnValue().Set(false); + return ThrowInvalidURL(realm->env(), input.ToStringView(), base_); } base_pointer = &base.value(); } @@ -251,7 +281,7 @@ void BindingData::Parse(const FunctionCallbackInfo& args) { ada::parse(input.ToStringView(), base_pointer); if (!out) { - return args.GetReturnValue().Set(false); + return ThrowInvalidURL(realm->env(), input.ToStringView(), base_); } binding_data->UpdateComponents(out->get_components(), out->type); diff --git a/src/node_url.h b/src/node_url.h index c485caa2eb0343..f3aa136a5b538d 100644 --- a/src/node_url.h +++ b/src/node_url.h @@ -76,6 +76,9 @@ class BindingData : public SnapshotableObject { const ada::scheme::type type); static v8::CFunction fast_can_parse_methods_[]; + static void ThrowInvalidURL(Environment* env, + std::string_view input, + std::optional base); }; std::string FromFilePath(const std::string_view file_path); diff --git a/test/common/gc.js b/test/common/gc.js new file mode 100644 index 00000000000000..af637af7bedcd6 --- /dev/null +++ b/test/common/gc.js @@ -0,0 +1,70 @@ +'use strict'; + +// TODO(joyeecheung): merge ongc.js and gcUntil from common/index.js +// into this. + +// This function can be used to check if an object factor leaks or not, +// but it needs to be used with care: +// 1. The test should be set up with an ideally small +// --max-old-space-size or --max-heap-size, which combined with +// the maxCount parameter can reproduce a leak of the objects +// created by fn(). +// 2. This works under the assumption that if *none* of the objects +// created by fn() can be garbage-collected, the test would crash due +// to OOM. +// 3. If *any* of the objects created by fn() can be garbage-collected, +// it is considered leak-free. The FinalizationRegistry is used to +// terminate the test early once we detect any of the object is +// garbage-collected to make the test less prone to false positives. +// This may be especially important for memory management relying on +// emphemeron GC which can be inefficient to deal with extremely fast +// heap growth. +// Note that this can still produce false positives. When the test using +// this function still crashes due to OOM, inspect the heap to confirm +// if a leak is present (e.g. using heap snapshots). +// The generateSnapshotAt parameter can be used to specify a count +// interval to create the heap snapshot which may enforce a more thorough GC. +// This can be tried for code paths that require it for the GC to catch up +// with heap growth. However this type of forced GC can be in conflict with +// other logic in V8 such as bytecode aging, and it can slow down the test +// significantly, so it should be used scarcely and only as a last resort. +async function checkIfCollectable( + fn, maxCount = 4096, generateSnapshotAt = Infinity, logEvery = 128) { + let anyFinalized = false; + let count = 0; + + const f = new FinalizationRegistry(() => { + anyFinalized = true; + }); + + async function createObject() { + const obj = await fn(); + f.register(obj); + if (count++ < maxCount && !anyFinalized) { + setImmediate(createObject, 1); + } + // This can force a more thorough GC, but can slow the test down + // significantly in a big heap. Use it with care. + if (count % generateSnapshotAt === 0) { + // XXX(joyeecheung): This itself can consume a bit of JS heap memory, + // but the other alternative writeHeapSnapshot can run into disk space + // not enough problems in the CI & be slower depending on file system. + // Just do this for now as long as it works and only invent some + // internal voodoo when we absolutely have no other choice. + require('v8').getHeapSnapshot().pause().read(); + console.log(`Generated heap snapshot at ${count}`); + } + if (count % logEvery === 0) { + console.log(`Created ${count} objects`); + } + if (anyFinalized) { + console.log(`Found finalized object at ${count}, stop testing`); + } + } + + createObject(); +} + +module.exports = { + checkIfCollectable, +}; diff --git a/test/es-module/test-dynamic-import-script-lifetime.js b/test/es-module/test-dynamic-import-script-lifetime.js new file mode 100644 index 00000000000000..7ccea887109e05 --- /dev/null +++ b/test/es-module/test-dynamic-import-script-lifetime.js @@ -0,0 +1,32 @@ +// Flags: --expose-gc --experimental-vm-modules + +'use strict'; + +// This tests that vm.Script would not get GC'ed while the script can still +// initiate dynamic import. +// See https://github.com/nodejs/node/issues/43205. + +require('../common'); +const vm = require('vm'); + +const code = ` +new Promise(resolve => { + setTimeout(() => { + gc(); // vm.Script should not be GC'ed while the script is alive. + resolve(); + }, 1); +}).then(() => import('foo'));`; + +// vm.runInThisContext creates a vm.Script underneath, which should not be GC'ed +// while import() can still be initiated. +vm.runInThisContext(code, { + async importModuleDynamically() { + const m = new vm.SyntheticModule(['bar'], () => { + m.setExport('bar', 1); + }); + + await m.link(() => {}); + await m.evaluate(); + return m; + } +}); diff --git a/test/es-module/test-vm-compile-function-leak.js b/test/es-module/test-vm-compile-function-leak.js new file mode 100644 index 00000000000000..f9f04588fdc7c3 --- /dev/null +++ b/test/es-module/test-vm-compile-function-leak.js @@ -0,0 +1,16 @@ +// Flags: --max-old-space-size=16 --trace-gc +'use strict'; + +// This tests that vm.compileFunction with dynamic import callback does not leak. +// See https://github.com/nodejs/node/issues/44211 +require('../common'); +const { checkIfCollectable } = require('../common/gc'); +const vm = require('vm'); + +async function createCompiledFunction() { + return vm.compileFunction(`"${Math.random().toString().repeat(512)}"`, [], { + async importModuleDynamically() {}, + }); +} + +checkIfCollectable(createCompiledFunction, 2048); diff --git a/test/es-module/test-vm-contextified-script-leak.js b/test/es-module/test-vm-contextified-script-leak.js new file mode 100644 index 00000000000000..a8625fc94d246e --- /dev/null +++ b/test/es-module/test-vm-contextified-script-leak.js @@ -0,0 +1,16 @@ +// Flags: --max-old-space-size=16 --trace-gc +'use strict'; + +// This tests that vm.Script with dynamic import callback does not leak. +// See: https://github.com/nodejs/node/issues/33439 +require('../common'); +const { checkIfCollectable } = require('../common/gc'); +const vm = require('vm'); + +async function createContextifyScript() { + // Try to reach the maximum old space size. + return new vm.Script(`"${Math.random().toString().repeat(512)}";`, { + async importModuleDynamically() {}, + }); +} +checkIfCollectable(createContextifyScript, 2048, 512); diff --git a/test/es-module/test-vm-source-text-module-leak.js b/test/es-module/test-vm-source-text-module-leak.js new file mode 100644 index 00000000000000..d05e812ac32c95 --- /dev/null +++ b/test/es-module/test-vm-source-text-module-leak.js @@ -0,0 +1,21 @@ +// Flags: --experimental-vm-modules --max-old-space-size=16 --trace-gc +'use strict'; + +// This tests that vm.SourceTextModule() does not leak. +// See: https://github.com/nodejs/node/issues/33439 +require('../common'); +const { checkIfCollectable } = require('../common/gc'); +const vm = require('vm'); + +async function createSourceTextModule() { + // Try to reach the maximum old space size. + const m = new vm.SourceTextModule(` + const bar = new Array(512).fill("----"); + export { bar }; + `); + await m.link(() => {}); + await m.evaluate(); + return m; +} + +checkIfCollectable(createSourceTextModule, 4096, 1024); diff --git a/test/es-module/test-vm-synthetic-module-leak.js b/test/es-module/test-vm-synthetic-module-leak.js new file mode 100644 index 00000000000000..bc0e4689535327 --- /dev/null +++ b/test/es-module/test-vm-synthetic-module-leak.js @@ -0,0 +1,18 @@ +// Flags: --experimental-vm-modules --max-old-space-size=16 +'use strict'; + +// This tests that vm.SyntheticModule does not leak. +// See https://github.com/nodejs/node/issues/44211 +require('../common'); +const { checkIfCollectable } = require('../common/gc'); +const vm = require('vm'); + +async function createSyntheticModule() { + const m = new vm.SyntheticModule(['bar'], () => { + m.setExport('bar', new Array(512).fill('----')); + }); + await m.link(() => {}); + await m.evaluate(); + return m; +} +checkIfCollectable(createSyntheticModule, 4096); diff --git a/test/fixtures/test-runner/output/abort.snapshot b/test/fixtures/test-runner/output/abort.snapshot index ceca09da14bfb1..1b758a2314c486 100644 --- a/test/fixtures/test-runner/output/abort.snapshot +++ b/test/fixtures/test-runner/output/abort.snapshot @@ -32,7 +32,7 @@ TAP version 13 # Subtest: not ok 2 not ok 6 - not ok 2 --- - duration_ms: ZERO + duration_ms: * location: '/test/fixtures/test-runner/output/abort.js:(LINE):7' failureType: 'cancelledByParent' error: 'test did not finish before its parent and was cancelled' @@ -41,7 +41,7 @@ TAP version 13 # Subtest: not ok 3 not ok 7 - not ok 3 --- - duration_ms: ZERO + duration_ms: * location: '/test/fixtures/test-runner/output/abort.js:(LINE):7' failureType: 'testAborted' error: 'This operation was aborted' @@ -62,7 +62,7 @@ TAP version 13 # Subtest: not ok 4 not ok 8 - not ok 4 --- - duration_ms: ZERO + duration_ms: * location: '/test/fixtures/test-runner/output/abort.js:(LINE):7' failureType: 'testAborted' error: 'This operation was aborted' @@ -83,7 +83,7 @@ TAP version 13 # Subtest: not ok 5 not ok 9 - not ok 5 --- - duration_ms: ZERO + duration_ms: * location: '/test/fixtures/test-runner/output/abort.js:(LINE):7' failureType: 'testAborted' error: 'This operation was aborted' @@ -169,7 +169,7 @@ not ok 2 - promise abort signal # Subtest: not ok 2 not ok 6 - not ok 2 --- - duration_ms: ZERO + duration_ms: * location: '/test/fixtures/test-runner/output/abort.js:(LINE):5' failureType: 'cancelledByParent' error: 'test did not finish before its parent and was cancelled' @@ -178,7 +178,7 @@ not ok 2 - promise abort signal # Subtest: not ok 3 not ok 7 - not ok 3 --- - duration_ms: ZERO + duration_ms: * location: '/test/fixtures/test-runner/output/abort.js:(LINE):5' failureType: 'testAborted' error: 'This operation was aborted' @@ -199,7 +199,7 @@ not ok 2 - promise abort signal # Subtest: not ok 4 not ok 8 - not ok 4 --- - duration_ms: ZERO + duration_ms: * location: '/test/fixtures/test-runner/output/abort.js:(LINE):5' failureType: 'testAborted' error: 'This operation was aborted' @@ -220,7 +220,7 @@ not ok 2 - promise abort signal # Subtest: not ok 5 not ok 9 - not ok 5 --- - duration_ms: ZERO + duration_ms: * location: '/test/fixtures/test-runner/output/abort.js:(LINE):5' failureType: 'testAborted' error: 'This operation was aborted' diff --git a/test/fixtures/test-runner/output/abort_hooks.snapshot b/test/fixtures/test-runner/output/abort_hooks.snapshot index d0b567bb6a22cd..278b5e5fd36ca5 100644 --- a/test/fixtures/test-runner/output/abort_hooks.snapshot +++ b/test/fixtures/test-runner/output/abort_hooks.snapshot @@ -11,7 +11,7 @@ TAP version 13 # Subtest: test 1 not ok 1 - test 1 --- - duration_ms: ZERO + duration_ms: * location: '/test/fixtures/test-runner/output/abort_hooks.js:(LINE):3' failureType: 'cancelledByParent' error: 'test did not finish before its parent and was cancelled' @@ -20,7 +20,7 @@ TAP version 13 # Subtest: test 2 not ok 2 - test 2 --- - duration_ms: ZERO + duration_ms: * location: '/test/fixtures/test-runner/output/abort_hooks.js:(LINE):3' failureType: 'cancelledByParent' error: 'test did not finish before its parent and was cancelled' diff --git a/test/fixtures/test-runner/output/abort_suite.snapshot b/test/fixtures/test-runner/output/abort_suite.snapshot index e7e8c4f4e2360f..30d48d236ff4a5 100644 --- a/test/fixtures/test-runner/output/abort_suite.snapshot +++ b/test/fixtures/test-runner/output/abort_suite.snapshot @@ -32,7 +32,7 @@ TAP version 13 # Subtest: not ok 2 not ok 6 - not ok 2 --- - duration_ms: ZERO + duration_ms: * location: '/test/fixtures/test-runner/output/abort_suite.js:(LINE):3' failureType: 'cancelledByParent' error: 'test did not finish before its parent and was cancelled' @@ -41,7 +41,7 @@ TAP version 13 # Subtest: not ok 3 not ok 7 - not ok 3 --- - duration_ms: ZERO + duration_ms: * location: '/test/fixtures/test-runner/output/abort_suite.js:(LINE):3' failureType: 'testAborted' error: 'This operation was aborted' @@ -62,7 +62,7 @@ TAP version 13 # Subtest: not ok 4 not ok 8 - not ok 4 --- - duration_ms: ZERO + duration_ms: * location: '/test/fixtures/test-runner/output/abort_suite.js:(LINE):3' failureType: 'testAborted' error: 'This operation was aborted' @@ -83,7 +83,7 @@ TAP version 13 # Subtest: not ok 5 not ok 9 - not ok 5 --- - duration_ms: ZERO + duration_ms: * location: '/test/fixtures/test-runner/output/abort_suite.js:(LINE):3' failureType: 'testAborted' error: 'This operation was aborted' diff --git a/test/fixtures/test-runner/output/arbitrary-output.snapshot b/test/fixtures/test-runner/output/arbitrary-output.snapshot index 2389096398cd09..601aaa42f3c74a 100644 --- a/test/fixtures/test-runner/output/arbitrary-output.snapshot +++ b/test/fixtures/test-runner/output/arbitrary-output.snapshot @@ -1,17 +1,17 @@ TAP version 13 ok 1 - test --- - duration_ms: ZERO + duration_ms: * ... # arbitrary - pre ok 2 - test --- - duration_ms: ZERO + duration_ms: * ... # arbitrary - mid ok 3 - test --- - duration_ms: ZERO + duration_ms: * ... # arbitrary - post 1..3 diff --git a/test/fixtures/test-runner/output/describe_it.snapshot b/test/fixtures/test-runner/output/describe_it.snapshot index be345f11575c8d..1d4f7853ead0d1 100644 --- a/test/fixtures/test-runner/output/describe_it.snapshot +++ b/test/fixtures/test-runner/output/describe_it.snapshot @@ -513,7 +513,7 @@ not ok 51 - subtest sync throw fails # Subtest: should not run not ok 1 - should not run --- - duration_ms: ZERO + duration_ms: * location: '/test/fixtures/test-runner/output/describe_it.js:(LINE):3' failureType: 'cancelledByParent' error: 'test did not finish before its parent and was cancelled' @@ -544,7 +544,7 @@ not ok 52 - describe sync throw fails # Subtest: should not run not ok 1 - should not run --- - duration_ms: ZERO + duration_ms: * location: '/test/fixtures/test-runner/output/describe_it.js:(LINE):3' failureType: 'cancelledByParent' error: 'test did not finish before its parent and was cancelled' diff --git a/test/fixtures/test-runner/output/hooks.snapshot b/test/fixtures/test-runner/output/hooks.snapshot index 5afe398ed3d0ea..6cf29612c535cb 100644 --- a/test/fixtures/test-runner/output/hooks.snapshot +++ b/test/fixtures/test-runner/output/hooks.snapshot @@ -37,7 +37,7 @@ ok 1 - describe hooks # Subtest: 1 not ok 1 - 1 --- - duration_ms: ZERO + duration_ms: * location: '/test/fixtures/test-runner/output/hooks.js:(LINE):3' failureType: 'cancelledByParent' error: 'test did not finish before its parent and was cancelled' @@ -46,7 +46,7 @@ ok 1 - describe hooks # Subtest: 2 not ok 2 - 2 --- - duration_ms: ZERO + duration_ms: * location: '/test/fixtures/test-runner/output/hooks.js:(LINE):3' failureType: 'cancelledByParent' error: 'test did not finish before its parent and was cancelled' diff --git a/test/fixtures/test-runner/output/unresolved_promise.snapshot b/test/fixtures/test-runner/output/unresolved_promise.snapshot index 839ec311a65e04..0090885468c338 100644 --- a/test/fixtures/test-runner/output/unresolved_promise.snapshot +++ b/test/fixtures/test-runner/output/unresolved_promise.snapshot @@ -18,7 +18,7 @@ not ok 2 - never resolving promise # Subtest: fail not ok 3 - fail --- - duration_ms: ZERO + duration_ms: * location: '/test/fixtures/test-runner/output/unresolved_promise.js:(LINE):1' failureType: 'cancelledByParent' error: 'Promise resolution is still pending but the event loop has already resolved' diff --git a/test/js-native-api/test_reference/binding.gyp b/test/js-native-api/test_reference/binding.gyp index d8940028915f15..a9d81ef9d2c05d 100644 --- a/test/js-native-api/test_reference/binding.gyp +++ b/test/js-native-api/test_reference/binding.gyp @@ -5,6 +5,12 @@ "sources": [ "test_reference.c" ] + }, + { + "target_name": "test_finalizer", + "sources": [ + "test_finalizer.c" + ] } ] } diff --git a/test/js-native-api/test_reference/test_finalizer.c b/test/js-native-api/test_reference/test_finalizer.c new file mode 100644 index 00000000000000..51492d9623f69c --- /dev/null +++ b/test/js-native-api/test_reference/test_finalizer.c @@ -0,0 +1,71 @@ +#include +#include +#include +#include "../common.h" +#include "../entry_point.h" + +static int test_value = 1; +static int finalize_count = 0; + +static void FinalizeExternalCallJs(napi_env env, void* data, void* hint) { + int* actual_value = data; + NODE_API_ASSERT_RETURN_VOID( + env, + actual_value == &test_value, + "The correct pointer was passed to the finalizer"); + + napi_ref finalizer_ref = (napi_ref)hint; + napi_value js_finalizer; + napi_value recv; + NODE_API_CALL_RETURN_VOID( + env, napi_get_reference_value(env, finalizer_ref, &js_finalizer)); + NODE_API_CALL_RETURN_VOID(env, napi_get_global(env, &recv)); + NODE_API_CALL_RETURN_VOID( + env, napi_call_function(env, recv, js_finalizer, 0, NULL, NULL)); + NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, finalizer_ref)); +} + +static napi_value CreateExternalWithJsFinalize(napi_env env, + napi_callback_info info) { + size_t argc = 1; + napi_value args[1]; + NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); + NODE_API_ASSERT(env, argc == 1, "Wrong number of arguments"); + napi_value finalizer = args[0]; + napi_valuetype finalizer_valuetype; + NODE_API_CALL(env, napi_typeof(env, finalizer, &finalizer_valuetype)); + NODE_API_ASSERT(env, + finalizer_valuetype == napi_function, + "Wrong type of first argument"); + napi_ref finalizer_ref; + NODE_API_CALL(env, napi_create_reference(env, finalizer, 1, &finalizer_ref)); + + napi_value result; + NODE_API_CALL(env, + napi_create_external(env, + &test_value, + FinalizeExternalCallJs, + finalizer_ref, /* finalize_hint */ + &result)); + + finalize_count = 0; + return result; +} + +EXTERN_C_START +napi_value Init(napi_env env, napi_value exports) { + napi_property_descriptor descriptors[] = { + DECLARE_NODE_API_PROPERTY("createExternalWithJsFinalize", + CreateExternalWithJsFinalize), + }; + + NODE_API_CALL( + env, + napi_define_properties(env, + exports, + sizeof(descriptors) / sizeof(*descriptors), + descriptors)); + + return exports; +} +EXTERN_C_END diff --git a/test/js-native-api/test_reference/test_finalizer.js b/test/js-native-api/test_reference/test_finalizer.js index b70582fd0342fe..a5270512dc87c1 100644 --- a/test/js-native-api/test_reference/test_finalizer.js +++ b/test/js-native-api/test_reference/test_finalizer.js @@ -2,7 +2,7 @@ // Flags: --expose-gc --force-node-api-uncaught-exceptions-policy const common = require('../../common'); -const test_reference = require(`./build/${common.buildType}/test_reference`); +const binding = require(`./build/${common.buildType}/test_finalizer`); const assert = require('assert'); process.on('uncaughtException', common.mustCall((err) => { @@ -11,7 +11,7 @@ process.on('uncaughtException', common.mustCall((err) => { (async function() { { - test_reference.createExternalWithJsFinalize( + binding.createExternalWithJsFinalize( common.mustCall(() => { throw new Error('finalizer error'); })); diff --git a/test/js-native-api/test_reference/test_reference.c b/test/js-native-api/test_reference/test_reference.c index 82c1f17d9dce0e..058be07363588b 100644 --- a/test/js-native-api/test_reference/test_reference.c +++ b/test/js-native-api/test_reference/test_reference.c @@ -22,20 +22,6 @@ static void FinalizeExternal(napi_env env, void* data, void* hint) { finalize_count++; } -static void FinalizeExternalCallJs(napi_env env, void* data, void* hint) { - int *actual_value = data; - NODE_API_ASSERT_RETURN_VOID(env, actual_value == &test_value, - "The correct pointer was passed to the finalizer"); - - napi_ref finalizer_ref = (napi_ref)hint; - napi_value js_finalizer; - napi_value recv; - NODE_API_CALL_RETURN_VOID(env, napi_get_reference_value(env, finalizer_ref, &js_finalizer)); - NODE_API_CALL_RETURN_VOID(env, napi_get_global(env, &recv)); - NODE_API_CALL_RETURN_VOID(env, napi_call_function(env, recv, js_finalizer, 0, NULL, NULL)); - NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, finalizer_ref)); -} - static napi_value CreateExternal(napi_env env, napi_callback_info info) { int* data = &test_value; @@ -118,31 +104,6 @@ CreateExternalWithFinalize(napi_env env, napi_callback_info info) { return result; } -static napi_value -CreateExternalWithJsFinalize(napi_env env, napi_callback_info info) { - size_t argc = 1; - napi_value args[1]; - NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); - NODE_API_ASSERT(env, argc == 1, "Wrong number of arguments"); - napi_value finalizer = args[0]; - napi_valuetype finalizer_valuetype; - NODE_API_CALL(env, napi_typeof(env, finalizer, &finalizer_valuetype)); - NODE_API_ASSERT(env, finalizer_valuetype == napi_function, "Wrong type of first argument"); - napi_ref finalizer_ref; - NODE_API_CALL(env, napi_create_reference(env, finalizer, 1, &finalizer_ref)); - - napi_value result; - NODE_API_CALL(env, - napi_create_external(env, - &test_value, - FinalizeExternalCallJs, - finalizer_ref, /* finalize_hint */ - &result)); - - finalize_count = 0; - return result; -} - static napi_value CheckExternal(napi_env env, napi_callback_info info) { size_t argc = 1; napi_value arg; @@ -263,24 +224,24 @@ static napi_value ValidateDeleteBeforeFinalize(napi_env env, napi_callback_info EXTERN_C_START napi_value Init(napi_env env, napi_value exports) { napi_property_descriptor descriptors[] = { - DECLARE_NODE_API_GETTER("finalizeCount", GetFinalizeCount), - DECLARE_NODE_API_PROPERTY("createExternal", CreateExternal), - DECLARE_NODE_API_PROPERTY("createExternalWithFinalize", - CreateExternalWithFinalize), - DECLARE_NODE_API_PROPERTY("createExternalWithJsFinalize", - CreateExternalWithJsFinalize), - DECLARE_NODE_API_PROPERTY("checkExternal", CheckExternal), - DECLARE_NODE_API_PROPERTY("createReference", CreateReference), - DECLARE_NODE_API_PROPERTY("createSymbol", CreateSymbol), - DECLARE_NODE_API_PROPERTY("createSymbolFor", CreateSymbolFor), - DECLARE_NODE_API_PROPERTY("createSymbolForEmptyString", CreateSymbolForEmptyString), - DECLARE_NODE_API_PROPERTY("createSymbolForIncorrectLength", CreateSymbolForIncorrectLength), - DECLARE_NODE_API_PROPERTY("deleteReference", DeleteReference), - DECLARE_NODE_API_PROPERTY("incrementRefcount", IncrementRefcount), - DECLARE_NODE_API_PROPERTY("decrementRefcount", DecrementRefcount), - DECLARE_NODE_API_GETTER("referenceValue", GetReferenceValue), - DECLARE_NODE_API_PROPERTY("validateDeleteBeforeFinalize", - ValidateDeleteBeforeFinalize), + DECLARE_NODE_API_GETTER("finalizeCount", GetFinalizeCount), + DECLARE_NODE_API_PROPERTY("createExternal", CreateExternal), + DECLARE_NODE_API_PROPERTY("createExternalWithFinalize", + CreateExternalWithFinalize), + DECLARE_NODE_API_PROPERTY("checkExternal", CheckExternal), + DECLARE_NODE_API_PROPERTY("createReference", CreateReference), + DECLARE_NODE_API_PROPERTY("createSymbol", CreateSymbol), + DECLARE_NODE_API_PROPERTY("createSymbolFor", CreateSymbolFor), + DECLARE_NODE_API_PROPERTY("createSymbolForEmptyString", + CreateSymbolForEmptyString), + DECLARE_NODE_API_PROPERTY("createSymbolForIncorrectLength", + CreateSymbolForIncorrectLength), + DECLARE_NODE_API_PROPERTY("deleteReference", DeleteReference), + DECLARE_NODE_API_PROPERTY("incrementRefcount", IncrementRefcount), + DECLARE_NODE_API_PROPERTY("decrementRefcount", DecrementRefcount), + DECLARE_NODE_API_GETTER("referenceValue", GetReferenceValue), + DECLARE_NODE_API_PROPERTY("validateDeleteBeforeFinalize", + ValidateDeleteBeforeFinalize), }; NODE_API_CALL(env, napi_define_properties( diff --git a/test/node-api/test_buffer/binding.gyp b/test/node-api/test_buffer/binding.gyp index e41a3993cd7c9d..e5d0955ae6308e 100644 --- a/test/node-api/test_buffer/binding.gyp +++ b/test/node-api/test_buffer/binding.gyp @@ -3,6 +3,10 @@ { "target_name": "test_buffer", "sources": [ "test_buffer.c" ] + }, + { + "target_name": "test_finalizer", + "sources": [ "test_finalizer.c" ] } ] } diff --git a/test/node-api/test_buffer/test_buffer.c b/test/node-api/test_buffer/test_buffer.c index bc61cd7a2e9062..013a7e2d417fbe 100644 --- a/test/node-api/test_buffer/test_buffer.c +++ b/test/node-api/test_buffer/test_buffer.c @@ -22,17 +22,6 @@ static void noopDeleter(napi_env env, void* data, void* finalize_hint) { deleterCallCount++; } -static void malignDeleter(napi_env env, void* data, void* finalize_hint) { - NODE_API_ASSERT_RETURN_VOID(env, data != NULL && strcmp(data, theText) == 0, "invalid data"); - napi_ref finalizer_ref = (napi_ref)finalize_hint; - napi_value js_finalizer; - napi_value recv; - NODE_API_CALL_RETURN_VOID(env, napi_get_reference_value(env, finalizer_ref, &js_finalizer)); - NODE_API_CALL_RETURN_VOID(env, napi_get_global(env, &recv)); - NODE_API_CALL_RETURN_VOID(env, napi_call_function(env, recv, js_finalizer, 0, NULL, NULL)); - NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, finalizer_ref)); -} - static napi_value newBuffer(napi_env env, napi_callback_info info) { napi_value theBuffer; char* theCopy; @@ -118,30 +107,6 @@ static napi_value staticBuffer(napi_env env, napi_callback_info info) { return theBuffer; } -static napi_value malignFinalizerBuffer(napi_env env, napi_callback_info info) { - size_t argc = 1; - napi_value args[1]; - NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); - NODE_API_ASSERT(env, argc == 1, "Wrong number of arguments"); - napi_value finalizer = args[0]; - napi_valuetype finalizer_valuetype; - NODE_API_CALL(env, napi_typeof(env, finalizer, &finalizer_valuetype)); - NODE_API_ASSERT(env, finalizer_valuetype == napi_function, "Wrong type of first argument"); - napi_ref finalizer_ref; - NODE_API_CALL(env, napi_create_reference(env, finalizer, 1, &finalizer_ref)); - - napi_value theBuffer; - NODE_API_CALL( - env, - napi_create_external_buffer(env, - sizeof(theText), - (void*)theText, - malignDeleter, - finalizer_ref, // finalize_hint - &theBuffer)); - return theBuffer; -} - static napi_value Init(napi_env env, napi_value exports) { napi_value theValue; @@ -151,14 +116,13 @@ static napi_value Init(napi_env env, napi_value exports) { napi_set_named_property(env, exports, "theText", theValue)); napi_property_descriptor methods[] = { - DECLARE_NODE_API_PROPERTY("newBuffer", newBuffer), - DECLARE_NODE_API_PROPERTY("newExternalBuffer", newExternalBuffer), - DECLARE_NODE_API_PROPERTY("getDeleterCallCount", getDeleterCallCount), - DECLARE_NODE_API_PROPERTY("copyBuffer", copyBuffer), - DECLARE_NODE_API_PROPERTY("bufferHasInstance", bufferHasInstance), - DECLARE_NODE_API_PROPERTY("bufferInfo", bufferInfo), - DECLARE_NODE_API_PROPERTY("staticBuffer", staticBuffer), - DECLARE_NODE_API_PROPERTY("malignFinalizerBuffer", malignFinalizerBuffer), + DECLARE_NODE_API_PROPERTY("newBuffer", newBuffer), + DECLARE_NODE_API_PROPERTY("newExternalBuffer", newExternalBuffer), + DECLARE_NODE_API_PROPERTY("getDeleterCallCount", getDeleterCallCount), + DECLARE_NODE_API_PROPERTY("copyBuffer", copyBuffer), + DECLARE_NODE_API_PROPERTY("bufferHasInstance", bufferHasInstance), + DECLARE_NODE_API_PROPERTY("bufferInfo", bufferInfo), + DECLARE_NODE_API_PROPERTY("staticBuffer", staticBuffer), }; NODE_API_CALL(env, napi_define_properties( diff --git a/test/node-api/test_buffer/test_finalizer.c b/test/node-api/test_buffer/test_finalizer.c new file mode 100644 index 00000000000000..eb5426d8f29cdf --- /dev/null +++ b/test/node-api/test_buffer/test_finalizer.c @@ -0,0 +1,61 @@ +#include +#include +#include +#include "../../js-native-api/common.h" + +static const char theText[] = + "Lorem ipsum dolor sit amet, consectetur adipiscing elit."; + +static void malignDeleter(napi_env env, void* data, void* finalize_hint) { + NODE_API_ASSERT_RETURN_VOID( + env, data != NULL && strcmp(data, theText) == 0, "invalid data"); + napi_ref finalizer_ref = (napi_ref)finalize_hint; + napi_value js_finalizer; + napi_value recv; + NODE_API_CALL_RETURN_VOID( + env, napi_get_reference_value(env, finalizer_ref, &js_finalizer)); + NODE_API_CALL_RETURN_VOID(env, napi_get_global(env, &recv)); + NODE_API_CALL_RETURN_VOID( + env, napi_call_function(env, recv, js_finalizer, 0, NULL, NULL)); + NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, finalizer_ref)); +} + +static napi_value malignFinalizerBuffer(napi_env env, napi_callback_info info) { + size_t argc = 1; + napi_value args[1]; + NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); + NODE_API_ASSERT(env, argc == 1, "Wrong number of arguments"); + napi_value finalizer = args[0]; + napi_valuetype finalizer_valuetype; + NODE_API_CALL(env, napi_typeof(env, finalizer, &finalizer_valuetype)); + NODE_API_ASSERT(env, + finalizer_valuetype == napi_function, + "Wrong type of first argument"); + napi_ref finalizer_ref; + NODE_API_CALL(env, napi_create_reference(env, finalizer, 1, &finalizer_ref)); + + napi_value theBuffer; + NODE_API_CALL(env, + napi_create_external_buffer(env, + sizeof(theText), + (void*)theText, + malignDeleter, + finalizer_ref, // finalize_hint + &theBuffer)); + return theBuffer; +} + +static napi_value Init(napi_env env, napi_value exports) { + napi_property_descriptor methods[] = { + DECLARE_NODE_API_PROPERTY("malignFinalizerBuffer", malignFinalizerBuffer), + }; + + NODE_API_CALL( + env, + napi_define_properties( + env, exports, sizeof(methods) / sizeof(methods[0]), methods)); + + return exports; +} + +NAPI_MODULE(NODE_GYP_MODULE_NAME, Init) diff --git a/test/node-api/test_buffer/test_finalizer.js b/test/node-api/test_buffer/test_finalizer.js index b706c68c7c3e02..35511fcb016c95 100644 --- a/test/node-api/test_buffer/test_finalizer.js +++ b/test/node-api/test_buffer/test_finalizer.js @@ -2,7 +2,7 @@ // Flags: --expose-gc --force-node-api-uncaught-exceptions-policy const common = require('../../common'); -const binding = require(`./build/${common.buildType}/test_buffer`); +const binding = require(`./build/${common.buildType}/test_finalizer`); const assert = require('assert'); const tick = require('util').promisify(require('../../common/tick')); diff --git a/test/node-api/test_threadsafe_function/binding.gyp b/test/node-api/test_threadsafe_function/binding.gyp index b60352e05af103..58a9d04d4a5619 100644 --- a/test/node-api/test_threadsafe_function/binding.gyp +++ b/test/node-api/test_threadsafe_function/binding.gyp @@ -3,6 +3,20 @@ { 'target_name': 'binding', 'sources': ['binding.c'] + }, + { + 'target_name': 'test_uncaught_exception_v9', + 'defines': [ + 'NAPI_VERSION=9' + ], + 'sources': ['test_uncaught_exception.c'] + }, + { + 'target_name': 'test_uncaught_exception', + 'defines': [ + 'NAPI_EXPERIMENTAL' + ], + 'sources': ['test_uncaught_exception.c'] } ] } diff --git a/test/node-api/test_threadsafe_function/test_force_uncaught_exception.js b/test/node-api/test_threadsafe_function/test_legacy_uncaught_exception.js similarity index 85% rename from test/node-api/test_threadsafe_function/test_force_uncaught_exception.js rename to test/node-api/test_threadsafe_function/test_legacy_uncaught_exception.js index b1f95715eadf60..a8743e00b5b8c5 100644 --- a/test/node-api/test_threadsafe_function/test_force_uncaught_exception.js +++ b/test/node-api/test_threadsafe_function/test_legacy_uncaught_exception.js @@ -2,7 +2,7 @@ // Flags: --no-force-node-api-uncaught-exceptions-policy const common = require('../../common'); -const binding = require(`./build/${common.buildType}/binding`); +const binding = require(`./build/${common.buildType}/test_uncaught_exception_v9`); process.on( 'uncaughtException', diff --git a/test/node-api/test_threadsafe_function/test_uncaught_exception.c b/test/node-api/test_threadsafe_function/test_uncaught_exception.c new file mode 100644 index 00000000000000..f8499d4fe4d680 --- /dev/null +++ b/test/node-api/test_threadsafe_function/test_uncaught_exception.c @@ -0,0 +1,62 @@ +#include +#include "../../js-native-api/common.h" + +// Testing calling into JavaScript +static void ThreadSafeFunctionFinalize(napi_env env, + void* finalize_data, + void* finalize_hint) { + napi_ref js_func_ref = (napi_ref)finalize_data; + napi_value js_func; + napi_value recv; + NODE_API_CALL_RETURN_VOID( + env, napi_get_reference_value(env, js_func_ref, &js_func)); + NODE_API_CALL_RETURN_VOID(env, napi_get_global(env, &recv)); + NODE_API_CALL_RETURN_VOID( + env, napi_call_function(env, recv, js_func, 0, NULL, NULL)); + NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, js_func_ref)); +} + +// Testing calling into JavaScript +static napi_value CallIntoModule(napi_env env, napi_callback_info info) { + size_t argc = 4; + napi_value argv[4]; + NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, argv, NULL, NULL)); + + napi_ref finalize_func; + NODE_API_CALL(env, napi_create_reference(env, argv[3], 1, &finalize_func)); + + napi_threadsafe_function tsfn; + NODE_API_CALL(env, + napi_create_threadsafe_function(env, + argv[0], + argv[1], + argv[2], + 0, + 1, + finalize_func, + ThreadSafeFunctionFinalize, + NULL, + NULL, + &tsfn)); + NODE_API_CALL(env, + napi_call_threadsafe_function(tsfn, NULL, napi_tsfn_blocking)); + NODE_API_CALL(env, napi_release_threadsafe_function(tsfn, napi_tsfn_release)); + return NULL; +} + +// Module init +static napi_value Init(napi_env env, napi_value exports) { + napi_property_descriptor properties[] = { + DECLARE_NODE_API_PROPERTY("CallIntoModule", CallIntoModule), + }; + + NODE_API_CALL( + env, + napi_define_properties(env, + exports, + sizeof(properties) / sizeof(properties[0]), + properties)); + + return exports; +} +NAPI_MODULE(NODE_GYP_MODULE_NAME, Init) diff --git a/test/node-api/test_threadsafe_function/test_uncaught_exception.js b/test/node-api/test_threadsafe_function/test_uncaught_exception.js index 2529757908999b..81b2623d702790 100644 --- a/test/node-api/test_threadsafe_function/test_uncaught_exception.js +++ b/test/node-api/test_threadsafe_function/test_uncaught_exception.js @@ -1,27 +1,7 @@ 'use strict'; -// Flags: --force-node-api-uncaught-exceptions-policy const common = require('../../common'); -const assert = require('assert'); -const binding = require(`./build/${common.buildType}/binding`); +const binding = require(`./build/${common.buildType}/test_uncaught_exception`); +const { testUncaughtException } = require('./uncaught_exception'); -const callbackCheck = common.mustCall((err) => { - assert.throws(() => { throw err; }, /callback error/); - process.removeListener('uncaughtException', callbackCheck); - process.on('uncaughtException', finalizerCheck); -}); -const finalizerCheck = common.mustCall((err) => { - assert.throws(() => { throw err; }, /finalizer error/); -}); -process.on('uncaughtException', callbackCheck); - -binding.CallIntoModule( - common.mustCall(() => { - throw new Error('callback error'); - }), - {}, - 'resource_name', - common.mustCall(function finalizer() { - throw new Error('finalizer error'); - }), -); +testUncaughtException(binding); diff --git a/test/node-api/test_threadsafe_function/test_uncaught_exception_v9.js b/test/node-api/test_threadsafe_function/test_uncaught_exception_v9.js new file mode 100644 index 00000000000000..28e628918fdff2 --- /dev/null +++ b/test/node-api/test_threadsafe_function/test_uncaught_exception_v9.js @@ -0,0 +1,8 @@ +'use strict'; +// Flags: --force-node-api-uncaught-exceptions-policy + +const common = require('../../common'); +const binding = require(`./build/${common.buildType}/test_uncaught_exception_v9`); +const { testUncaughtException } = require('./uncaught_exception'); + +testUncaughtException(binding); diff --git a/test/node-api/test_threadsafe_function/uncaught_exception.js b/test/node-api/test_threadsafe_function/uncaught_exception.js new file mode 100644 index 00000000000000..da2aa2f4efef8a --- /dev/null +++ b/test/node-api/test_threadsafe_function/uncaught_exception.js @@ -0,0 +1,31 @@ +'use strict'; + +const common = require('../../common'); +const assert = require('assert'); + +function testUncaughtException(binding) { + const callbackCheck = common.mustCall((err) => { + assert.throws(() => { throw err; }, /callback error/); + process.removeListener('uncaughtException', callbackCheck); + process.on('uncaughtException', finalizerCheck); + }); + const finalizerCheck = common.mustCall((err) => { + assert.throws(() => { throw err; }, /finalizer error/); + }); + process.on('uncaughtException', callbackCheck); + + binding.CallIntoModule( + common.mustCall(() => { + throw new Error('callback error'); + }), + {}, + 'resource_name', + common.mustCall(function finalizer() { + throw new Error('finalizer error'); + }), + ); +} + +module.exports = { + testUncaughtException, +}; diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status index 6f4b9de73778b2..fe8ddee7cbf05e 100644 --- a/test/parallel/parallel.status +++ b/test/parallel/parallel.status @@ -5,6 +5,8 @@ prefix parallel # sample-test : PASS,FLAKY [true] # This section applies to all platforms +# https://github.com/nodejs/node/issues/49853 +test-runner-output: PASS,FLAKY [$system==win32] # https://github.com/nodejs/node/issues/41206 diff --git a/test/parallel/test-os.js b/test/parallel/test-os.js index f0f55fbe261946..d82f2ece3159a6 100644 --- a/test/parallel/test-os.js +++ b/test/parallel/test-os.js @@ -81,6 +81,12 @@ const hostname = os.hostname(); is.string(hostname); assert.ok(hostname.length > 0); +const DUMMY_PRIORITY = 10; +os.setPriority(DUMMY_PRIORITY); +const priority = os.getPriority(); +is.number(priority); +assert.strictEqual(priority, DUMMY_PRIORITY); + // On IBMi, os.uptime() returns 'undefined' if (!common.isIBMi) { const uptime = os.uptime(); diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs index fff6fed92655e9..372ca8f3bae0ff 100644 --- a/test/parallel/test-runner-output.mjs +++ b/test/parallel/test-runner-output.mjs @@ -10,7 +10,6 @@ const skipForceColors = function replaceTestDuration(str) { return str - .replaceAll(/duration_ms: 0(\r?\n)/g, 'duration_ms: ZERO$1') .replaceAll(/duration_ms: [0-9.]+/g, 'duration_ms: *') .replaceAll(/duration_ms [0-9.]+/g, 'duration_ms *'); } @@ -20,7 +19,6 @@ const stackTraceBasePath = new RegExp(`${color}\\(${process.cwd()}/?${color}(.*) function replaceSpecDuration(str) { return str - .replaceAll(/\(0(\r?\n)ms\)/g, '(ZEROms)') .replaceAll(/[0-9.]+ms/g, '*ms') .replaceAll(/duration_ms [0-9.]+/g, 'duration_ms *') .replace(stackTraceBasePath, '$3'); @@ -28,7 +26,6 @@ function replaceSpecDuration(str) { function replaceJunitDuration(str) { return str - .replaceAll(/time="0"/g, 'time="ZERO"') .replaceAll(/time="[0-9.]+"/g, 'time="*"') .replaceAll(/duration_ms [0-9.]+/g, 'duration_ms *') .replaceAll(hostname(), 'HOSTNAME') diff --git a/test/parallel/test-trace-events-bootstrap.js b/test/parallel/test-trace-events-bootstrap.js index 9497bd896e7bd6..0ad9c33800d799 100644 --- a/test/parallel/test-trace-events-bootstrap.js +++ b/test/parallel/test-trace-events-bootstrap.js @@ -32,7 +32,7 @@ if (process.argv[2] === 'child') { const file = tmpdir.resolve('node_trace.1.log'); assert(fs.existsSync(file)); - fs.readFile(file, common.mustCall((err, data) => { + fs.readFile(file, common.mustSucceed((data) => { const traces = JSON.parse(data.toString()).traceEvents .filter((trace) => trace.cat !== '__metadata'); traces.forEach((trace) => { diff --git a/test/parallel/test-url-null-char.js b/test/parallel/test-url-null-char.js index 468080844d534b..d81cbcfb6648d8 100644 --- a/test/parallel/test-url-null-char.js +++ b/test/parallel/test-url-null-char.js @@ -4,5 +4,5 @@ const assert = require('assert'); assert.throws( () => { new URL('a\0b'); }, - { input: 'a\0b' } + { code: 'ERR_INVALID_URL', input: 'a\0b' } ); diff --git a/test/parallel/test-whatwg-url-custom-parsing.js b/test/parallel/test-whatwg-url-custom-parsing.js index 905028fee3812c..cdeda59eec0c98 100644 --- a/test/parallel/test-whatwg-url-custom-parsing.js +++ b/test/parallel/test-whatwg-url-custom-parsing.js @@ -55,7 +55,7 @@ for (const test of failureTests) { () => new URL(test.input, test.base), (error) => { assert.throws(() => { throw error; }, expectedError); - assert.strictEqual(`${error}`, 'TypeError [ERR_INVALID_URL]: Invalid URL'); + assert.strictEqual(`${error}`, 'TypeError: Invalid URL'); assert.strictEqual(error.message, 'Invalid URL'); return true; });