Reintroduce v8::DEFAULT to simplify migration #165
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Subsystems: blob, child_process, common, crypto, http, http2, readline, repl, snapshot, trace_events PR-URL: nodejs#49127 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Debadree Chatterjee <[email protected]>
PR-URL: nodejs#49128 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#49143 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Deokjin Kim <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
We previously only return startup data for the first slot for BaseObjects because we can already serialize all the necessary information in one go, but slots that do not get special startup data would be serialized verbatim which means that the pointer addresses are going to be part of the snapshot blob, resulting in indeterminism. This patch updates the serialization routines and capture information for both of the two slots - the first slot with type information and memory management type (which we can use in the future for cppgc-managed objects) and the second slot with data about the object itself. This way the embeedder slots can be serialized in a reproducible manner in the snapshot. PR-URL: nodejs#48996 Refs: nodejs/build#3043 Reviewed-By: Rafael Gonzaga <[email protected]>
Signed-off-by: Michal Biesek <[email protected]> PR-URL: nodejs#49106 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Deokjin Kim <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Fix warning about dereferencing null env Signed-off-by: Michael Dawson <[email protected]> PR-URL: nodejs#48954 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
getDefaultEncoding() always returns 'buffer' in Node.js 20. It requires some careful justification but the default encoding can be eliminated from sig.js entirely. In Sign.prototype.update, we can safely remove the conditional assignment of getDefaultEncoding() to encoding. This is because SignUpdate() in crypto_sig.cc internally calls node::crypto::Decode, which returns UTF8 for falsy encoding values. In other words, with the conditional assignment, StringBytes::Write() ultimately receives the encoding BUFFER, and without the conditional assignment, it receives the encoding UTF8. However, StringBytes::Write() treats both encodings identically, so there is no need to deviate from the internal default encoding UTF8. In Sign.prototype.sign, we can also safely remove the conditional assignment of getDefaultEncoding() to encoding. Whether encoding is falsy or 'buffer' makes no difference. In Verify.prototype.verify, we can also safely remove the conditional assignment of getDefaultEncoding() to sigEncoding. This is because the function passes the sigEncoding to getArrayBufferOrView(), which passes it to Buffer.from(). If sigEncoding is 'buffer', getArrayBufferOrView() instead passes 'utf8' to Buffer.from(). Because the default encoding of Buffer.from() is 'utf8', passing a falsy encoding to getArrayBufferOrView() instead of 'buffer' results in the same behavior. Refs: nodejs#47182 PR-URL: nodejs#49145 Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
This version avoids the additional access to the embedder slot when we already have a reference to the realm. PR-URL: nodejs#49007 Refs: nodejs#48836 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
This reduce the number of embedder slot accesses and also removes the assumption in a few binding methods that the current realm is the principal realm of the current environment (which is not true for shadow realms). PR-URL: nodejs#49007 Refs: nodejs#48836 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
These can be used to check the state and the output of a child process launched with `spawnSync()`. They log additional information about the child process when the check fails to facilitate debugging test failures. PR-URL: nodejs#49020 Reviewed-By: Luigi Pinca <[email protected]>
..and replace the similar code added for logging. PR-URL: nodejs#49020 Reviewed-By: Luigi Pinca <[email protected]>
This (not particularly elegant) native addon tests the effect of UV_THREADPOOL_SIZE on node-api. The test fails if Node.js allows more than UV_THREADPOOL_SIZE async tasks to run concurrently, or if it limits the number of concurrent async tasks to anything less than UV_THREADPOOL_SIZE. PR-URL: nodejs#49165 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
getDefaultEncoding() always returns 'buffer' in Node.js 20. It requires some careful justification but the default encoding can be eliminated from hash.js entirely. The reasoning is almost identical with that in nodejs#49145 so I won't repeat it here. Refs: nodejs#47182 PR-URL: nodejs#49167 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
getDefaultEncoding() always returns 'buffer' in Node.js 20. In diffiehellman.js, this value is always used as input to either toBuf(), encode(), or getArrayBufferOrView(). All of these functions treat any falsy encoding just like 'buffer', so we can safely remove the calls to getDefaultEncoding(). Refs: nodejs#47182 PR-URL: nodejs#49169 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
Previously we assume that the objects are GC'ed after one global.gc() returns, which is not necessarily always the case. Use gcUntil() to run GC multiple times if they are not GC'ed in the first time around. PR-URL: nodejs#49053 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
The C++ implementation can now be done entirely in JS using WeakRef. Re-implement it in JS instead to simplify the code. PR-URL: nodejs#49053 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#49053 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#49199 Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#49019 Fixes: nodejs#48995 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Previously the ContextifyContext holds a MicrotaskQueueWrap which in turns holds a MicrotaskQueue in a shared pointer. The indirection is actually unnecessary, we can directly hold the MicrotaskQueue via a unique pointer in ContextifyContext, the lifetime would still remain the same but the graph would be simpler, and this removes the additional JS -> C++ to create the wrapper object. PR-URL: nodejs#48982 Reviewed-By: Stephen Belanger <[email protected]>
Add `process.sourceMapsEnabled` to detect whether source-maps are enabled. Fixes: nodejs#46304 PR-URL: nodejs#46391 Reviewed-By: Chemi Atlow <[email protected]>
PR-URL: nodejs#48890 Refs: https://github.com/orgs/nodejs/discussions/44975 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Paolo Insogna <[email protected]>
PR-URL: nodejs#49215 Fixes: nodejs#49049 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
V8 now no longer supports serializing code cache in an isolate with unfinalized read-only space. So guard the code cache regeneration with the `is_building_snapshot()` flag. When the isolate is created for snapshot generation, the code cache is going to be serialized separately anyway, so there is no need to do it in the builtin loader. PR-URL: nodejs#49108 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
Fixes: nodejs#48112 Ref: nodejs#48208 PR-URL: nodejs#49184 Refs: nodejs#48208 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
PR-URL: nodejs#49186 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
Support for a single comma separates list for allow-fs-* flags is removed. Instead now multiple flags can be passed to allow multiple paths. Fixes: nodejs/security-wg#1039 PR-URL: nodejs#49047 Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Marco Ippolito <[email protected]>
Refs: nodejs#32930 PR-URL: nodejs#49180 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#49175 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Ruy Adorno <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#49112 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Merging based on marja@ request.
# Conflicts: # test/parallel/parallel.status
# Conflicts: # test/parallel/parallel.status
The CL https://crrev.com/c/3799431 adds a new function to the console object so we need to temporarily disable a repl test that checks for the concrete shape of the console object. # Conflicts: # test/parallel/test-repl.js
These tests fail on the node-ci builder: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20-%20node.js%20integration%20ng/21832/overview
Skip test-debugger-break and test-debugger-run-after-quit-restart to unblock https://crrev.com/c/4290476
…#155) The failing test is a regression test for nodejs#39717, which is about a crash that occurs when the flag is disabled. Given that that will no longer be possible once the flag is gone, this seems safe to disable forever.
Re-enable tests temporarily disabled in a8952fc
Co-authored-by: Rezvan Mahdavi Hezaveh <[email protected]>
V8 will increase the maximum TypedArray size in https://crrev.com/c/4872536; this CL adapts the tests to allow for that. Tests that hard-code smaller sizes or are already tested in V8 are removed; one test is adapted to not rely on a specific limit.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Temporarily reintroduce v8::DEFAULT to simplify migration.