v8.16.0 proposal #26933
Merged
v8.16.0 proposal #26933
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
Original commit message:
[cpu-profiler] Fix a leak caused by re-logging existing functions.
Don't re-log all existing functions during StartProcessorIfNotStarted().
They will already be in the CodeMap attached to the ProfileGenerator and
re-logging them causes leaks. See the linked bug for more details.
Bug: v8:8253
Change-Id: Ibb1a1ab2431c588e8c3a3a9ff714767cdf61a88e
Reviewed-on: https://chromium-review.googlesource.com/1256763
Commit-Queue: Peter Marshall <[email protected]>
Reviewed-by: Yang Guo <[email protected]>
Cr-Commit-Position: refs/heads/master@{#56336}
Refs: v8/v8@525b396
PR-URL: #25041
Reviewed-By: Yang Guo <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Refs: #8895 PR-URL: #25123 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
In 180f865, the test was changed so that the `env` argument of `createInternalRepl()` also contained external environment variables, because keeping them can be necessary for spawning processes on some systems. However, this test does not spawn new processes, and relies on the fact that the environment variables it tests are not already set (and fails otherwise); therefore, reverting to the original state should fix this. Fixes: #21451 Fixes: nodejs/build#1377 Refs: #25219 PR-URL: #25226 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Bundle a `uv_async_t`, a `uv_idle_t`, a `uv_mutex_t`, a `uv_cond_t`, and a `v8::Persistent<v8::Function>` to make it possible to call into JS from another thread. The API accepts a void data pointer and a callback which will be invoked on the loop thread and which will receive the `napi_value` representing the JavaScript function to call so as to perform the call into JS. The callback is run inside a `node::CallbackScope`. A `std::queue<void*>` is used to store calls from the secondary threads, and an idle loop is started by the `uv_async_t` callback on the loop thread to drain the queue, calling into JS with each item. Items can be added to the queue blockingly or non-blockingly. The thread-safe function can be referenced or unreferenced, with the same semantics as libuv handles. Re: nodejs/help#1035 Re: #20964 Fixes: #13512 Backport-PR-URL: #25002 PR-URL: #17887 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
private field 'async_context' is not used [-Wunused-private-field] PR-URL: #21597 Refs: #17887 Backport-PR-URL: #25002 Reviewed-By: Gabriel Schulhof <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
A condition variable is only created by the thread-safe function if the queue size is set to something larger than zero. This adds null-checks around the condition variable and tests for the case where the queue size is zero. Fixes: nodejs/help#1387 PR-URL: #21871 Backport-PR-URL: #25002 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Backport-PR-URL: #25002 PR-URL: #21898 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
The idle_running member variable in TsFn is always false and can therefore be removed. Backport-PR-URL: #25002 PR-URL: #22520 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Gabriel Schulhof <[email protected]> Reviewed-By: Gus Caplan <[email protected]>
* Move class `TsFn` to name space `v8impl` and rename it to `ThreadSafeFunction` * Remove `NAPI_EXTERN` from API declarations, because it's only needed in the header file. Backport-PR-URL: #25002 PR-URL: #22259 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Kyle Farnung <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
The thread_finalize_data and thread_finalize_cb parameters in napi_create_threadsafe_function are optional. Backport-PR-URL: #25002 PR-URL: #22998 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Currently when building with --debug
test/addons-napi/test_threadsafe_function will error:
$ out/Debug/node test/addons-napi/test_threadsafe_function/test.js
FATAL ERROR: v8::HandleScope::CreateHandle()
Cannot create a handle without a HandleScope
1: 0x10004e287 node::DumpBacktrace(__sFILE*) [node/out/Debug/node]
2: 0x1000cd37b node::Abort() [/node/out/Debug/node]
3: 0x1000cd69f node::OnFatalError(char const*, char const*)
[/node/out/Debug/node]
4: 0x1004df0b1 v8::Utils::ReportApiFailure(char const*, char const*)
[/nodejs/node/out/Debug/node]
5: 0x100a8c0a9 v8::internal::HandleScope::Extend(
v8::internal::Isolate*)
[/node/out/Debug/node]
6: 0x1004e4229 v8::EmbedderDataFor(v8::Context*,
int, bool,
char const*)
[/node/out/Debug/node]
7: 0x1004e43fa v8::Context::SlowGetAlignedPointerFromEmbedderData(int)
[/node/out/Debug/node]
8: 0x10001c26b v8::Context::GetAlignedPointerFromEmbedderData(int)
[/node/out/Debug/node]
9: 0x1000144ea node::Environment::GetCurrent(v8::Local<v8::Context>)
[/node/out/Debug/node]
10: 0x1000f49e2 napi_env__::node_env() const
[/node/out/Debug/node]
11: 0x1000f9885
(anonymous namespace)::v8impl::ThreadSafeFunction::
CloseHandlesAndMaybeDelete(bool)
[/node/out/Debug/node]
12: 0x1000fb34f (anonymous namespace)::v8impl::ThreadSafeFunction::
DispatchOne()
[/node/out/Debug/node]
13: 0x1000fb129
(anonymous namespace)::v8impl::ThreadSafeFunction::
IdleCb(uv_idle_s*)
[/node/out/Debug/node]
14: 0x1011a1b69 uv__run_idle
[/node/out/Debug/node]
15: 0x101198179 uv_run
[/node/out/Debug/node]
16: 0x1000dfca1
node::Start(...)
[/node/out/Debug/node]
17: 0x1000dae50 node::Start(...)
[/node/out/Debug/node]
18: 0x1000da56f node::Start(int, char**)
[/node/out/Debug/node]
19: 0x10141112e main
[/node/out/Debug/node]
20: 0x100001034 start
[/node/out/Debug/node]
Abort trap: 6
This commit adds two HandleScope's, one to CloseHandlesAndMaybeDelete
and one to the lambda.
SlowGetAlignedPointerFromEmbedderData will only be called for debug
builds:
https://github.com/v8/v8/blob/2ef0aa662fe907a1b36ac1abe7d77ad2bcd27733
/include/v8.h#L10440-L10447
Backport-PR-URL: #25002
PR-URL: #24011
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
The test fails consistently on windows-fanned with vs2017. mark it as flaky while the issue is being progressed, and to keep CI green / amber. Ref: #23621 Backport-PR-URL: #25002 PR-URL: #24714 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Refs: #25028 PR-URL: #25032 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]>
AIX 7.1 appears to return a different error message compared to AIX 6.1. PR-URL: #25418 Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: George Adams <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
#17604 refactored the gyp files so that `-blibpath:` on AIX was only set if `node_shared=="true"`. Restore the setting for non-shared builds. Fixes: #25444 Backport-PR-URL: #25521 PR-URL: #25447 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Since faking TTY input is not otherwise fake-able, we need support in the test runner for it. Backport-PR-URL: #25351 PR-URL: #23053 Reviewed-By: James M Snell <[email protected]>
Allow reading from stdio streams that are conventionally associated with process output, since this is only convention. This involves disabling the oddness around closing stdio streams. Its purpose is to prevent the file descriptors 0 through 2 from being closed, since doing so can lead to information leaks when new file descriptors are being opened; instead, not doing anything seems like a more reasonable choice. Fixes: #21203 Backport-PR-URL: #25351 PR-URL: #23053 Reviewed-By: James M Snell <[email protected]>
Make sure that `process.stdin.write()`, and in particular ending the stream, works. Backport-PR-URL: #25351 PR-URL: #23053 Reviewed-By: James M Snell <[email protected]>
Backport-PR-URL: #25351 PR-URL: #23051 Fixes: #22814 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: George Adams <[email protected]> Reviewed-By: James M Snell <[email protected]>
* Move `napi_get_uv_event_loop` into the `NAPI_VERSION >= 2` section * Move `napi_open_callback_scope`, `napi_close_callback_scope`, `napi_fatal_exception`, `napi_add_env_cleanup_hook`, and `napi_remove_env_cleanup_hook` into the `NAPI_VERSION >= 3` section * Added a missing `added` property to `napi_get_uv_event_loop` in the docs * Added a `napiVersion` property to the docs and updated the parser and generator to use it. * Added usage documentation PR-URL: #19962 Backport-PR-URL: #25648 Reviewed-By: Gabriel Schulhof <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Fixes: #24249 PR-URL: #25556 Backport-PR-URL: #25648 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Reused parsers can be attached to the domain that corresponds to the active domain when the underlying socket was created, which is not necessarily correct. Instead, we attach parsers to the active domain if there is one when they're reused from the pool. Refs: #25456 PR-URL: #25459 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
`http_parser_execute(..., nullptr, 0)` returns either `0` or `1`. The expectation is that no error must be returned if it is `0`, and if it is `1` - a `Error` object must be returned back to user. The introduction of `llhttp` and the refactor that happened during it accidentally removed the error-returning code. This commit reverts it back to its original state. Backport-PR-URL: #25938 Fix: #24585 PR-URL: #24738 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
Calling into the engine from a weak callback is unsafe, however, the engine offers a way to attach a second-pass weak callback which gets called when it is safe to call into JavaScript. This moves the point at which the N-API finalize callback gets called to this latter point. Fixes: #25927 PR-URL: #25992 Backport-PR-URL: #26060 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Building on Mac OS/X as follows: ``` ./configure --shared make -j4 test ``` Results in: ``` dyld: Library not loaded: @rpath/libnode.67.dylib Referenced from: /Users/rubys/git/node-shared/out/Release/cctest Reason: image not found make: *** [cctest] Abort trap: 6 ``` This change adds the loader path to the runtime path for the `cctest` executable. Backport-PR-URL: #25681 PR-URL: #23168 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]>
cctest depends on some internal APIs which don't declare `__declspec(dllexport)` and causes build failure when building node as shared lib on Windows. Since we already have good test coverage in static lib, we decide to skip the cctest in shared lib build on Windows. Signed-off-by: Yihong Wang <[email protected]> Backport-PR-URL: #25758 PR-URL: #21228 Reviewed-By: Refael Ackermann <[email protected]>
This seems to have been broken ever since its introduction 5 years ago in commit 75ea11f ("tls: introduce asynchronous `newSession`") and no one complained but that's not going to stop me from fixing it anyway because otherwise I can't write a regression test for issue #26428. Refs: #26428 PR-URL: #26452 Fixes: #26428 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Beth Griggs <[email protected]>
There is a time window between the first and the last step of processing the clienthello event and the SecurePair may have been destroyed during that interval. Fixes: #26428 PR-URL: #26452 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Beth Griggs <[email protected]>
Original commit message:
PPC: fix Regex addi overflow
using add insetad of addi when Operand is more than 16 bits long
Change-Id: I7f9452381ed8b321ec71e68d0d90485508b69885
Reviewed-on: https://chromium-review.googlesource.com/c/1430619
Commit-Queue: Junliang Yan <[email protected]>
Reviewed-by: Junliang Yan <[email protected]>
Cr-Commit-Position: refs/heads/master@{#59049}
Refs: v8/v8@3cc6919
PR-URL: #25874
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: George Adams <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
On AIX the underlying fsync system call returns EBADF on a file descriptor for an open directory. So avoid running fsync on it. PR-URL: #21298 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: James M Snell <[email protected]> Signed-off-by: Beth Griggs <[email protected]>
|
@MylesBorins it would be nice to include #27202 in this release. Is that still possible? |
|
@BridgeAR does this break anything? Assuming it is only a fix. |
2 tasks
|
@MylesBorins it's only a regression fix for a backported commit and won't break anything. Without the fix we throw a weird error in the error case. |
This fixes a regression for an error case with `assert.rejects` and `assert.doesNotReject`. Fixes: #27185 PR-URL: #27202 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Beth Griggs <[email protected]>
Improve performance creating strings using N-API by ensuring that the strings are not internalized. Added test cases for latin-1 and utf-16 strings. PR-URL: #26439 Fixes: #26437 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: James M Snell <[email protected]>
MylesBorins
force-pushed
the
v8.16.0-proposal
branch
from
3a7f44e
to
4a6662c
Compare
|
https://ci.nodejs.org/job/node-test-commit-linux/26998/nodes=centos6-64-gcc48/console is weird: Sounds like a compiler bug? I’ll start a resume CI but this shouldn’t happen… |
|
@nodejs/build any idea what is going on with centos6? This compiler bug is new and only happening in CI. It was not happening a week ago and there have been no changes to this release that should cause that bug |
|
Seems like |
This comment has been minimized.
This comment has been minimized.
|
FWIW |
|
Unable to replicate got or fastify failures on a fresh VM, promoting the release |
MylesBorins
force-pushed
the
v8.16.0-proposal
branch
from
4a6662c
to
5f93cf7
Compare
|
I'm trying to compile node v.12.1.0, and am also getting bit by the centos 6 / devtoolset-2-binutils, except for devtoolset-7, with g++ 6.5.0 on CentOS 6.10. I've tried to check out the "properly setup" link in @refack 's post, but I'm new here and don't have "overall/read" permission. Sorry for all the newbieisms, but is there a fix available? Thanks! |
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.
2019-04-16, Version 8.16.0 'Carbon' (LTS), @MylesBorins
Notable Changes
Commits
c07ba9681f] - build: skip cctest on Windows shared lib build (Yihong Wang) #2122863522886ea] - build: add loader path to rpath for cctest (Sam Ruby) #23168e9369073d9] - build: set-blibpath:for AIX (Richard Lau) #2544797cc0fc51d] - deps: V8: cherry-pick 3cc6919 (Farazmand) #25874a1aff28fba] - deps: cherry-pick 525b396 from V8 upstream (Peter Marshall) #250416b7cccc88a] - doc: fix optional parameters in n-api.md (Lars-Magnus Skog) #22998b17819db3d] - doc: update the http.request.setTimeout docs to be accurate (James Bunton) #25123ac9b8f7645] - http: fix error check inExecute()(Brian White) #247381d862610f8] - http: attach reused parser to correct domain (Julien Gilli) #254591e76323061] - n-api: improve performance creating strings (Anthony Tuininga) #264392b2ad96ef2] - n-api: finalize during second-pass callback (Gabriel Schulhof) #25992d6ffabc37f] - (SEMVER-MINOR) n-api: mark thread-safe function as stable (Gabriel Schulhof) #2555644609d1274] - n-api: restrict exports by version (Kyle Farnung) #19962fe4328252a] - n-api: add missing handle scopes (Daniel Bevenius) #24011902b07959f] - n-api: clean up thread-safe function (Gabriel Schulhof) #2225909b88aabb3] - n-api: remove idle_running from TsFn (Lars-Magnus Skog) #22520367505940a] - n-api: guard against cond null dereference (Gabriel Schulhof) #21871c5a11dc58e] - n-api: fix compiler warning (cjihrig) #21597759a0180b5] - (SEMVER-MINOR) n-api: add API for asynchronous functions (Gabriel Schulhof) #17887ea5628e77a] - process: allow reading from stdout/stderr sockets (Anna Henningsen) #2305367b6e0d19c] - src: fix may be uninitialized warning in n-api (Michael Dawson) #21898eaf474cc5d] - test: shared lib build doesn't handle SIGPIPE (Yihong Wang) #192113128cb7da6] - test: avoid running fsync on directory on AIX (John Barboza) #21298b4c5435a46] - test: add process.stdin.end() TTY regression test (Matteo Collina) #23051c56f3edb10] - test: add stdin writable regression test (Anna Henningsen) #23053f6ff8c51bc] - test: fix module loading error for AIX 7.1 (Richard Lau) #25418d4b6643ac3] - test: mark test-cli-node-options flaky on arm (Rich Trott) #2503260db455961] - test: mark test_threadsafe_function/test as flaky (Gireesh Punathil) #24714fbafe8d311] - test: fix test-repl-envvars (Anna Henningsen) #252267573b55a15] - tls: fix legacy SecurePair clienthello race window (Ben Noordhuis) #2645291620b8bd6] - tls: fix legacy SecurePair session resumption (Ben Noordhuis) #264521a9582b7a6] - tools: allow input for TTY tests (Anna Henningsen) #23053