Safe worker termination#2
Closed
aduh95 wants to merge 846 commits into
Closed
Conversation
Refs: nodejs#26725 Fixes: nodejs#29813 Refs: nodejs#29814 PR-URL: nodejs#35755 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Christian Clauss <cclauss@me.com>
The test is not nearly as unreliable as it used to be but we're still seeing failures around the timing checks that will definitely be affected by other tests running in other processes. So move it to sequential. Refs: nodejs#35961 (comment) PR-URL: nodejs#35996 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
The test uses a file name twice, causing unreliability in CI. In particular, it's failing a lot on the Raspberry Pi devices. Fixes: nodejs#36090 PR-URL: nodejs#36102 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>
Add libuv's cumulative idle time in the diagnostic report. Add the data under the libuv's loop section Refs: nodejs#34938 PR-URL: nodejs#35940 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>
Use apostrophe for possessive. PR-URL: nodejs#36066 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Replace `var` with `const` in vm context for test-util-inspect.js. PR-URL: nodejs#36069 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
For scenario where target env is explicitly specified as vs2019, do not clear VCINSTALLDIR which was being cleared to handle fallback to vs2017 block when attempting to find a matching available VS. Fixes: nodejs#35856 PR-URL: nodejs#36009 Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
We were fetching the buffer from the float array to send out the response in js land, however that logic is being duplicated in node_process.h. Now they will be using an inline to fetch the array buffers and making it more generic. PR-URL: nodejs#34553 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: nodejs#35998 Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: nodejs#36003 Reviewed-By: Rich Trott <rtrott@gmail.com>
Refs: https://www.w3.org/TR/WebCryptoAPI/#subtlecrypto-interface Fixes: nodejs#36083 PR-URL: nodejs#36087 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
PR-URL: nodejs#36011 Reviewed-By: Rich Trott <rtrott@gmail.com>
Add test case to cover currently-uncovered code. Refs: https://coverage.nodejs.org/coverage-39a7f7663e8f70fc/lib/internal/util/inspect.js.html#L333 PR-URL: nodejs#36086 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
PR-URL: nodejs#36025 Reviewed-By: Shingo Inoue <leko.noor@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: nodejs#36019 Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: nodejs#36023 Reviewed-By: Rich Trott <rtrott@gmail.com>
Fixes: nodejs#36064 PR-URL: nodejs#36094 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
- Remove unneeded listener for the `'error'` event. - Use `common.mustCall()`. - Verify that the `src` stream gets paused. PR-URL: nodejs#36056 Refs: nodejs#35941 Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
PR-URL: nodejs#33099 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
If the option cwd does not exist, the error ENOENT is the same as the error emitted when the command does not exist, it's confusing. PR-URL: nodejs#34505 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Render all properties of nodeTiming enumerable so JSON.stringify and Object.keys can access them Fixes: nodejs#35936 PR-URL: nodejs#35977 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
In email, Shigeki Ohtsu indicated that a move to emeritus would be appropriate at this time. I'm especially grateful for the crypto and security work over the years, and I'm sure I'm not the only one. PR-URL: nodejs#36093 Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Inadvertent double hash in link makes it invalid and no longer pointing to the documentation section it intends to point to. PR-URL: nodejs#36109 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
PR-URL: nodejs#36101 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
PR-URL: nodejs#35700 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Shelley Vohr <codebytere@gmail.com>
PR-URL: nodejs#35700 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Major V8 updates are usually API/ABI incompatible with previous versions. This commit adapts NODE_MODULE_VERSION for V8 8.7. Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md PR-URL: nodejs#35700 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Original commit message:
[testrunner] delete ancient junit compatible format support
Testrunner has ancient support for JUnit compatible XML output.
This CL removes this old feature.
R=mstarzinger@chromium.org,jgruber@chromium.org,jkummerow@chromium.org
CC=machenbach@chromium.org
Bug: v8:8728
Change-Id: I7e1beb011dbaec3aa1a27398a5c52abdd778eaf0
Reviewed-on: https://chromium-review.googlesource.com/c/1430065
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Tamer Tas <tmrts@chromium.org>
Cr-Commit-Position: refs/heads/master@{nodejs#59045}
Refs: v8/v8@bd019bd
PR-URL: nodejs#32116
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Patch V8 (compiler/js-heap-broker.cc) to remove the use of an optional property, which is a fairly new C++ feature, since that requires a newer XCode version than the minimum requirement in BUILDING.md and thus breaks CI. PR-URL: nodejs#32116 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Fixes a compilation issue on some platforms PR-URL: nodejs#32116 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs#36436 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Verify that if something different than Abortcontroller.signal is passed to child_process.execFile(), ERR_INVALID_ARG_TYPE is thrown. PR-URL: nodejs#36429 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: nodejs#36356 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
PR-URL: nodejs#36431 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Signed-off-by: Michael Dawson <mdawson@devrus.com> PR-URL: nodejs#36475 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs#36406 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
This commit adds a typedef for the callback parameter used in CleanupHookCallback's constructor, AddCleanupHook, and RemoveCleanupHook. PR-URL: nodejs#36442 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
There are situations where one wants to invoke a JS callback's ->Call()
from C++ and in particular retain any existing async_context state, but
where it's not obvious that a plain ->Call() would be safe at the point
in question.
Such callsites usually resort to
node::MakeCallback(..., async_context{0, 0}), which unconditionally
pushes the async_context{0, 0} and takes the required provisions for the
->Call() itself such as triggering the tick after its return, if needed.
An example would be the PerformanceObserver invocation from
PerformanceEntry::Notify(): this can get called when coming from JS
through e.g. perf_hooks.performance.mark() and alike, but perhaps also
from nghttp2 (c.f. EmitStatistics() in node_http2.cc).
In the former case, a plain ->Call() would be safe and it would be
desirable to retain the current async_context so that
PerformanceObservers can access it resp. the associated
AsyncLocalStorage. However, in the second case the additional provisions
taken by node::MakeCallback() might potentially be strictly required.
So PerformanceEntry::Notify() bites the bullet and invokes the
PerformanceObservers through node::MakeCallback() unconditionally,
thereby always rendering any possibly preexisting async_context
inaccessible.
Introduce the convenience node::MakeSyncCallback() for such usecases,
which would basically forward to ->Call() if safe and to
node::MakeCallback(..., async_context{0, 0}) otherwise.
Co-Authored-By: ZauberNerd <zaubernerd@zaubernerd.de>
PR-URL: nodejs#36343
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
It's desirable to retain async_contexts active at callsites of
perf_hooks.performance.mark() and alike in the subsequent
PerformanceObserver invocations such that the latter can access e.g.
associated AsyncLocalStorage instances.
In working towards this goal replace the node::MakeCallback(...,
async_context{0, 0}) in PerformanceEntry::doNotify() by the new
node::MakeSyncCallback() introduced specifically for this purpose.
This change will retain the original async_context, if any, in
perf_hook's observersCallback() and thus, for the subsequent doNotify()
on unbuffered PerformanceObservers.
Co-Authored-By: ZauberNerd <zaubernerd@zaubernerd.de>
PR-URL: nodejs#36343
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
This reverts commit 009e418. AFAIU the discussion at [1], PerformanceObserver had been made to inherit from AsyncResource more or less as a band-aid in lack of a better async_context candidate to invoke it in. In order to enable access to AsyncLocalStores from PerformanceObservers invoked synchronously through e.g. measure() or mark(), the current async_context, if any, should be retained. Note that this is a breaking change, but - as has been commented at [1], PerformanceObserver being derived from AsyncResource is a "minor divergence from the spec" anyway, - to my knowledge this is an internal implementation detail which has never been documented and - I can't think of a good reason why existing PerformanceObserver implementations would possibly rely on it. OTOH, it's probably worthwhile to not potentially invoke before() and after() async_hooks for each and every PerformanceObserver notification. [1] nodejs#18789 Co-Authored-By: ZauberNerd <zaubernerd@zaubernerd.de> PR-URL: nodejs#36343 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>
This test proves that the PerformanceObserver callback gets called with the async context of the callsite of performance.mark()/measure() and therefore AsyncLocalStorage can be used inside a PerformanceObserver. PR: nodejs#36343 PR-URL: nodejs#36343 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>
Refs: nodejs#36473 Refs: GHSA-qqgx-2p2h-9c37 PR-URL: nodejs#36474 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs#36459 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
Node.js sets a stack trace handler specific to the v8::Context corresponding to the current Environment. When Electron is running in a non-Node.js v8::Context (e.g in the renderer process with contextIsolation enabled), there will be no correspondent Environment - we therefore need to prevent this handler being set so that Blink falls back to its default handling and displays the correct stacktrace. PR-URL: nodejs#36447 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>
aduh95
pushed a commit
that referenced
this pull request
Jun 4, 2026
This function call can fail with `Z_VERSION_ERROR` if the compiled
library vs loaded library mismatched in version number or in
stream structure size.
In those cases, zlib doesn't initialize the `strm_.msg` field to
null. Therefore, when a `CompressionError` object is created via
`ErrorForMessage()`, it can read a stale or uninitialized `strm_.msg`
pointer that will cause a crash.
Example ASAN report:
```
AddressSanitizer: SEGV on unknown address
#0 __strlen_avx2
string/../sysdeps/x86_64/multiarch/strlen-avx2.S:76
#1 strlen (/work/node/out/Debug/node+0x1a42ab7)
#2 v8::(anonymous namespace)::StringLength(char const*)
/work/node/out/../deps/v8/src/api/api.cc:7581:16
#3 v8::(anonymous namespace)::StringLength(unsigned char const*)
/work/node/out/../deps/v8/src/api/api.cc:7587:10
#4 v8::String::NewFromOneByte(v8::Isolate*,
unsigned char const*, v8::NewStringType, int)
/work/node/out/../deps/v8/src/api/api.cc:7677:3
#5 node::OneByteString(v8::Isolate*,
char const*, int, v8::NewStringType)
/work/node/out/../src/util-inl.h:166:10
#6 node::(anonymous namespace)::CompressionStream<
node::(anonymous namespace)::ZlibContext>
::EmitError(node::(anonymous namespace)
::CompressionError const&)
/work/node/out/../src/node_zlib.cc:565:7
#7 node::(anonymous namespace)::CompressionStream<
node::(anonymous namespace)::ZlibContext>
::CheckError()
/work/node/out/../src/node_zlib.cc:519:5
#8 node::(anonymous namespace)::CompressionStream<
node::(anonymous namespace)::ZlibContext>
::AfterThreadPoolWork(int)
/work/node/out/../src/node_zlib.cc:543:10
#9 node::ThreadPoolWork::ScheduleWork()
::'lambda'(uv_work_s*, int)
::operator()(uv_work_s*, int) const
/work/node/out/../src/threadpoolwork-inl.h:57:15
#10 node::ThreadPoolWork::ScheduleWork()
::'lambda'(uv_work_s*, int)
::__invoke(uv_work_s*, int)
/work/node/out/../src/threadpoolwork-inl.h:48:7
#11 uv__work_done /work/libuv-1.51.0/src/threadpool.c:330:5
#12 uv__async_io.part.0
/work/libuv-1.51.0/src/unix/async.c:208:5
```
Signed-off-by: ndossche <nora.dossche@ugent.be>
PR-URL: nodejs#63476
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
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.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes