Age | Commit message (Collapse) | Author |
|
Promises are never destroyed manually therefore it's not needed to
attach an object to track if destroy hook was called already.
PR-URL: https://github.com/nodejs/node/pull/42402
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
|
|
This is no longer necessary (and actually deprecated) since C++17.
PR-URL: https://github.com/nodejs/node/pull/41755
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
|
|
This reverts commit 7ca2f1303996e0c79c354e979a1527da444ca886.
PR-URL: https://github.com/nodejs/node/pull/40741
Fixes: https://github.com/nodejs/node/issues/40693
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/40782
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
|
|
Signed-off-by: Darshan Sen <darshan.sen@postman.com>
PR-URL: https://github.com/nodejs/node/pull/38468
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/39135
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
|
|
The new JS PromiseHooks introduced in the referenced PR are per
v8::Context. This meant that code depending on them, such as
AsyncLocalStorage, wouldn't behave correctly across vm.Context
instances.
PromiseHooks are now synchronized across the main Context and any
Context created via vm.Context.
Refs: https://github.com/nodejs/node/pull/36394
Fixes: https://github.com/nodejs/node/issues/38781
Signed-off-by: Bryan English <bryan@bryanenglish.com>
PR-URL: https://github.com/nodejs/node/pull/38821
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/36394
Reviewed-By: Bryan English <bryan@bryanenglish.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
Fixes: https://github.com/nodejs/node-v8/issues/193
PR-URL: https://github.com/nodejs/node/pull/37587
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
|
|
Fall back to using the outer context’s microtask queue, rather than
the Isolate’s default one. This would otherwise result in surprising
behavior if an embedder specified a custom microtask queue for the
main Node.js context.
PR-URL: https://github.com/nodejs/node/pull/36482
Refs: https://github.com/v8/v8/commit/4bf051d536a172e7932845d82f918ad7280c7873
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
|
|
Following the crypto subsystem refactor, this is now unused.
Begone!!
Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/35511
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
|
|
When a process exits cleanly, i.e. because the event loop ends up
without things to wait for, the Node.js objects that are left on
the heap should be:
1. weak, i.e. ready for garbage collection once no longer
referenced, or
2. detached, i.e. scheduled for destruction once no longer
referenced, or
3. an unrefed libuv handle, i.e. does not keep the event loop
alive, or
4. an inactive libuv handle (essentially the same here)
There are a few exceptions to this rule, but generally,
if there are C++-backed Node.js objects on the heap
that do not fall into the above categories, we may be looking
at a potential memory leak. Most likely, the cause is a missing
`MakeWeak()` call on the corresponding object.
In order to avoid this kind of problem, we check the list
of BaseObjects for these criteria. In this commit, we only do so
when explicitly instructed to or when in debug mode
(where --verify-base-objects is always-on).
In particular, this avoids the kinds of memory leak issues
that were fixed in the PRs referenced below.
Refs: https://github.com/nodejs/node/pull/35488
Refs: https://github.com/nodejs/node/pull/35487
Refs: https://github.com/nodejs/node/pull/35481
PR-URL: https://github.com/nodejs/node/pull/35490
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
|
|
Native side of fast-path promise hook was not calling JS
fastPromiseHook function when there were no async ids
previously assigned to the promise. Because of that already
created promises could not get id assigned in situations
when an async hook without a before listener function is
enabled after their creation. As the result executionAsyncId
could return wrong id when called within promise's .then().
Refs: https://github.com/nodejs/node/pull/34512
PR-URL: https://github.com/nodejs/node/pull/34548
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
|
|
Use a microtask to call destroy hooks in case there are a lot queued
as immediate may be scheduled late in case of long running
promise chains.
Queuing a mircrotasks in GC context is not allowed therefore an
interrupt is triggered to do this in JS context as fast as possible.
fixes: https://github.com/nodejs/node/issues/34328
refs: https://github.com/nodejs/node/issues/33896
PR-URL: https://github.com/nodejs/node/pull/34342
Fixes: https://github.com/nodejs/node/issues/34328
Refs: https://github.com/nodejs/node/issues/33896
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
Remove unnecessary native-to-JS code switches in fast-path for
PromiseHooks. Those switches happen even if a certain type of
hook (say, before) is not installed, which may lead to sub-optimal
performance in the AsyncLocalStorage scenario, i.e. when there is
only an init hook.
PR-URL: https://github.com/nodejs/node/pull/34512
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
|
|
This runs `lib/internal/bootstrap/node.js` before creating
the builtin snapshot and deserialize the loaders from the
snapshot in deserialization mode.
PR-URL: https://github.com/nodejs/node/pull/32984
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
|
|
Removes some of the performance overhead that came with
`executionAsyncResource()` by using the JS resource array
only as a cache for the values provided by C++. The fact that we now
use an entry trampoline is used to pass the resource without
requiring extra C++/JS boundary crossings, and the direct accesses
to the JS resource array from C++ are removed in all fast paths.
This particularly improves performance when async hooks are not
being used.
This is a continuation of https://github.com/nodejs/node/pull/33575
and shares some of its code with it.
./node benchmark/compare.js --new ./node --old ./node-master --runs 30 --filter messageport worker | Rscript benchmark/compare.R
[00:06:14|% 100| 1/1 files | 60/60 runs | 2/2 configs]: Done
confidence improvement accuracy (*) (**) (***)
worker/messageport.js n=1000000 payload='object' ** 12.64 % ±7.30% ±9.72% ±12.65%
worker/messageport.js n=1000000 payload='string' * 11.08 % ±9.00% ±11.98% ±15.59%
./node benchmark/compare.js --new ./node --old ./node-master --runs 20 --filter async-resource-vs-destroy async_hooks | Rscript benchmark/compare.R
[00:22:35|% 100| 1/1 files | 40/40 runs | 6/6 configs]: Done
confidence improvement accuracy (*)
async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='async' type='async-local-storage' benchmarker='autocannon' 1.60 % ±7.35%
async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='async' type='async-resource' benchmarker='autocannon' 6.05 % ±6.57%
async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='async' type='destroy' benchmarker='autocannon' * 8.27 % ±7.50%
async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='callbacks' type='async-local-storage' benchmarker='autocannon' 7.42 % ±8.22%
async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='callbacks' type='async-resource' benchmarker='autocannon' 4.33 % ±7.84%
async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='callbacks' type='destroy' benchmarker='autocannon' 5.96 % ±7.15%
(**) (***)
async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='async' type='async-local-storage' benchmarker='autocannon' ±9.84% ±12.94%
async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='async' type='async-resource' benchmarker='autocannon' ±8.81% ±11.60%
async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='async' type='destroy' benchmarker='autocannon' ±10.07% ±13.28%
async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='callbacks' type='async-local-storage' benchmarker='autocannon' ±11.01% ±14.48%
async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='callbacks' type='async-resource' benchmarker='autocannon' ±10.50% ±13.81%
async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='callbacks' type='destroy' benchmarker='autocannon' ±9.58% ±12.62%
Refs: https://github.com/nodejs/node/pull/33575
PR-URL: https://github.com/nodejs/node/pull/34319
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/33801
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
|
|
PR-URL: https://github.com/nodejs/node/pull/33772
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
|
|
This commit removes the unused using declarations reported by lint-cpp.
PR-URL: https://github.com/nodejs/node/pull/33268
Refs: https://github.com/nodejs/node/issues/29226
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
|
|
Refs: https://github.com/nodejs/node/pull/33320#discussion_r423141443
PR-URL: https://github.com/nodejs/node/pull/33444
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
|
|
Use a symbol on the bindings object to store the public resource object,
rather than a `v8::Global` Persistent. This has several advantages:
- It’s harder to inadvertently create memory leaks this way.
The garbage collector sees the `AsyncWrap` → resource link like
a regular JS property, and can collect the objects as a group,
even if the resource object should happen to point back to the
`AsyncWrap` object.
- This will make it easier in the future to use `owner_symbol` for
this purpose, which is generally the direction we should be moving
the `async_hooks` API into (i.e. using more public objects instead
of letting internal wires stick out).
PR-URL: https://github.com/nodejs/node/pull/31745
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
|
|
This commit adds promise_resolve to the list of callbacks mentioned in
the comment. It also fixes a minor typo, every -> ever.
PR-URL: https://github.com/nodejs/node/pull/33365
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/33350
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
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: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
|
|
../src/async_wrap.cc: In function ‘uint16_t node::ToAsyncHooksType(v8::PromiseHookType)’:
../src/async_wrap.cc:313:1: error: control reaches end of non-void function [-Werror=return-type]
}
Refs: https://github.com/nodejs/node/pull/32891
PR-URL: https://github.com/nodejs/node/pull/33322
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
|
|
This avoids the need to wrap every promise in an AsyncWrap and also
makes it easier to skip the machinery to track destroy events when
there's no destroy listener.
Co-authored-by: Andrey Pechkurov <apechkurov@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/32891
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/33051
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/32729
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
|
|
Otherwise, this leaks memory if the weak callback is never called.
PR-URL: https://github.com/nodejs/node/pull/32421
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
Wrap reused read_wrap in a unique async resource to ensure that
executionAsyncResource() is not ambiguous.
PR-URL: https://github.com/nodejs/node/pull/31972
Refs: https://github.com/nodejs/node/pull/30959
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
|
|
This was an oversight in 9fdb6e6aaf45b2364bac89a.
Fixing this is necessary to make `executionAsyncResource()` work
as expected.
Refs: https://github.com/nodejs/node/pull/30959
Fixes: https://github.com/nodejs/node/issues/32060
PR-URL: https://github.com/nodejs/node/pull/32063
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
Change suggested by bnoordhuis.
Improve handing of internal field counting by using enums.
Helps protect against future possible breakage if field
indexes are ever changed or added to.
Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/31960
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
|
|
Remove the need for the destroy hook in the basic APM case.
Co-authored-by: Stephen Belanger <admin@stephenbelanger.com>
PR-URL: https://github.com/nodejs/node/pull/30959
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
Node codebase has evolved a lot in the more than 10 years of its
existence. As more features (and code) have been added, changed,
removed, it's sometimes hard to keep track of what gets used and what
not.
This commits attempts to clean some of those potentially left-over
headers using suggestions from include-what-you-use
Refs: https://github.com/nodejs/node/issues/27531
PR-URL: https://github.com/nodejs/node/pull/30328
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
Modify the native `SetImmediate()` functions to take generic C++
callables as arguments. This makes passing arguments to the callback
easier, and in particular, it allows passing `std::unique_ptr`s
directly, which in turn makes sure that the data they point to is
deleted if the `Environment` is torn down before the callback can run.
PR-URL: https://github.com/nodejs/node/pull/28704
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
Heap dumps can be taken either through the inspector or the public API
for it during an async_hooks init() hook, but at that point the
AsyncWrap in question is not done initializing yet and virtual methods
cannot be called on it.
Address this issue (somewhat hackily) by excluding `AsyncWrap`
instances which have not yet executed their `init()` hook fully
from heap dumps.
Fixes: https://github.com/nodejs/node/issues/28786
PR-URL: https://github.com/nodejs/node/pull/28789
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rich Trott <rtrott@gmail.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/28611
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
|
|
As discussed in https://github.com/nodejs/diagnostics/issues/248,
https://github.com/nodejs/node/pull/21313 and
https://docs.google.com/document/d/1g8OrG5lMIUhRn1zbkutgY83MiTSMx-0NHDs8Bf-nXxM/preview
reusing the resource object is a blocker for landing a resource based
async hooks API and get rid of the promise destroy hook.
This PR ensures that HttpAgent uses the a new resource object in case
the socket handle gets reused.
PR-URL: https://github.com/nodejs/node/pull/27581
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
|
|
The promise hook has been disabled asynchronously in order to solve
issues when an async hook is disabled during a microtask.
This means that after scheduling the disable-promise-hook call,
attempts to enable it synchronously will later be unintentionally
overridden.
In order to solve this, make sure that the promise hooks are still
no longer desired at the time at which we would disable them.
Fixes: https://github.com/nodejs/node/issues/27585
PR-URL: https://github.com/nodejs/node/pull/27590
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
|
|
PR-URL: https://github.com/nodejs/node/pull/27579
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
|
|
Fix some issues introduced/not fixed via
https://github.com/nodejs/node/pull/25094:
* Init hook is not emitted for a reused HTTPParser
* HTTPParser was still used as resource in init hook
* type used in init hook was always HTTPINCOMINGMESSAGE even for client
requests
* some tests have not been adapted to new resource names
With this change the async hooks init event is emitted during a call
to Initialize() as the type and resource object is available at this
time. As a result Initialize() must be called now which could be seen
as breaking change even HTTPParser is not part of documented API.
It was needed to put the ClientRequest instance into a wrapper object
instead passing it directly as async resource otherwise
test-domain-multi fails. I think this is because adding an EventEmitter
to a Domain adds a property 'domain' and the presence of this changes
the context propagation in domains.
Besides that tests still refering to resource HTTPParser have been
updated/improved.
Fixes: https://github.com/nodejs/node/issues/27467
Fixes: https://github.com/nodejs/node/issues/26961
Refs: https://github.com/nodejs/node/pull/25094
PR-URL: https://github.com/nodejs/node/pull/27477
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
|
|
`v8::Global` is essentially a nicer variant of `node::Persistent` that,
in addition to reset-on-destroy, also implements move semantics.
This commit makes the necessary replacements, removes
`node::Persistent` and (now-)unnecessary inclusions of the
`node_persistent.h` header, and makes some of the functions that
take Persistents as arguments more generic so that they work with all
`v8::PersistentBase` flavours.
PR-URL: https://github.com/nodejs/node/pull/27287
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
|
Change resource being used, previously HTTParser was being reused.
We are now using IncomingMessage and ClientRequest objects. The goal
here is to make the async resource unique for each async operatio
Refs: https://github.com/nodejs/node/pull/24330
Refs: https://github.com/nodejs/diagnostics/issues/248
Refs: https://github.com/nodejs/node/pull/21313
Co-authored-by: Matheus Marchini <mat@mmarchini.me>
PR-URL: https://github.com/nodejs/node/pull/25094
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benedikt Meurer <benedikt.meurer@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
|
|
FromJust() is often used not for its return value, but for its
side-effects. In these cases, Check() exists, and is more clear as to
the intent. From its comment:
To be used, where the actual value of the Maybe is not needed, like
Object::Set.
See: https://github.com/nodejs/node/pull/26929/files#r269256335
PR-URL: https://github.com/nodejs/node/pull/27162
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/26824
Refs: https://github.com/nodejs/node/issues/26776
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
Remove this, as the underlying `Isolate::SetPromiseHook()` may be
removed as well in its current form in the future, and `async_hooks`
also serves this use case.
Refs: https://docs.google.com/document/d/1g8OrG5lMIUhRn1zbkutgY83MiTSMx-0NHDs8Bf-nXxM/
Refs: https://github.com/nodejs/node/pull/26529
PR-URL: https://github.com/nodejs/node/pull/26574
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/26306
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/25916
Refs: https://github.com/nodejs/node/pull/20914
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
|
|
This patch moves most of the public C++ APIs into src/api/*.cc
so that it's easier to tell that we need to be careful about
the compatibility of these code.
Some APIs, like `node::LoadEnvironmet()`, `node::Start()` and
`node::Init()` still stay in `node.cc` because they are still
very specific to our use cases and do not work quite well yet
for embedders anyway - we could not even manage to write cctest for
them at the moment.
PR-URL: https://github.com/nodejs/node/pull/25541
Reviewed-By: Gus Caplan <me@gus.host>
|
|
PR-URL: https://github.com/nodejs/node/pull/25734
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
|