Manually destroy the AsyncResource created by AsyncLocalStore.run() to
avoid unneeded GC tracking in case a destroy hooks is present.
PR-URL: https://github.com/nodejs/node/pull/34653
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Only call executionAsyncResource() in getStore() if the ALS instance is
enabled because the resource is not needed otherwise.
PR-URL: https://github.com/nodejs/node/pull/34617
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Creates an internal AsyncResource and binds a function to it,
ensuring that the function is invoked within execution context
in which bind was called.
PR-URL: https://github.com/nodejs/node/pull/34574
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
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>
This allows transitioning the entire following sync and async execution
sub-tree to the given async storage context. With this one can be sure
the context binding will remain for any following sync activity and all
descending async execution whereas the `run*(...)` methods must wrap
everything that is intended to exist within the context. This is helpful
for scenarios such as prepending a `'connection'` event to an http
server which binds everything that occurs within each request to
the given context. This is helpful for APMs to minimize the need
for patching and especially adding closures.
PR-URL: https://github.com/nodejs/node/pull/31945
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
PR-URL: https://github.com/nodejs/node/pull/32085
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
This commit introduces store as the first argument in
AsyncLocalStorage's run methods. The change is motivated by the
following expectation: most users are going to use a custom object
as the store and an extra Map created by the previous implementation
is an overhead for their use case.
Important note. This is a backwards incompatible change.
It was discussed and agreed an incompatible change is ok
since the API is still experimental and the modified
methods were only added within the last week so usage
will be minimal to none.
PR-URL: https://github.com/nodejs/node/pull/31930
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Adding AsyncLocalStorage class to async_hooks
module.
This API provide a simple CLS-like set
of features.
Co-authored-by: Andrey Pechkurov <apechkurov@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/26540
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
The exception handler user-defined will not automatically emit after
for the async resource.
Also removes a duplicated case
`test-emit-after-uncaught-exception-runInAsyncScope.js`
which is identical to test-emit-after-uncaught-exception.js.
Refs: https://github.com/nodejs/node/pull/30965
PR-URL: https://github.com/nodejs/node/pull/31784
Fixes: https://github.com/nodejs/node/issues/31783
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
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>
This error checking is mostly unnecessary and is just a Node core
developer nicety, rather than something that is needed for the
user-land. It can be safely removed without any practical
impact while making nextTick, timers, immediates and AsyncResource
substantially faster.
PR-URL: https://github.com/nodejs/node/pull/30967
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
We should never try to manually run emitAfter in case of an error,
the exception handler will do it for us, if we're going to recover.
PR-URL: https://github.com/nodejs/node/pull/30965
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Store all primordials as properties of the primordials object.
Static functions are prefixed by the constructor's name and prototype
methods are prefixed by the constructor's name followed by "Prototype".
For example: primordials.Object.keys becomes primordials.ObjectKeys.
PR-URL: https://github.com/nodejs/node/pull/30610
Refs: https://github.com/nodejs/node/issues/29766
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
We clear the async id stack inside the uncaught exception handler and
emit `after` events in the process, so we should not emit `after`
a second time from the `runInAsyncScope()` code.
This should match the behaviour we have in C++.
Fixes: https://github.com/nodejs/node/issues/30080
PR-URL: https://github.com/nodejs/node/pull/30087
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Optimize the hot code paths of queueMicrotask by
not creating unnecessary objects, not looking up
properties on frozen primordials, etc.
PR-URL: https://github.com/nodejs/node/pull/28093
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Accessing symbols is generally quite expensive and so is emitInit,
only do both when actually required.
PR-URL: https://github.com/nodejs/node/pull/27032
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Use the "no-restricted-globals" ESLint rule to lint for it.
PR-URL: https://github.com/nodejs/node/pull/27027
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
AsyncResource.emitBefore and AsyncResource.emitAfter have been
deprecated in https://github.com/nodejs/node/pull/18632. This PR removes
it all.
This commit also updates some embedder tests to use internal APIs.
The conditions are still possible for Node.js core developers but not
for end users.
PR-URL: https://github.com/nodejs/node/pull/26530
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Benedikt Meurer <benedikt.meurer@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Pulls out a common argument validator to `internal/validators`
PR-URL: https://github.com/nodejs/node/pull/22101
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This makes a effort to make sure all of these errors will actually
also show the received input.
On top of that it refactors a few tests for better maintainability.
It will also change the returned type to always be a simple typeof
instead of special handling null.
PR-URL: https://github.com/nodejs/node/pull/19445
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This commit removes the builtin async_wrap module from
lib/async_hooks.js.
The motivation for this is that lib/async_hooks.js requires
lib/internal/async_hooks which also binds async_wrap. Instead of
lib/async_hooks.js also binding async_wrap it now only has to require
the internal async_hooks and access it's exports.
There might be a very good reason for doing it the current way but the
reason is not obvious to me. Hopefully someone can shed some light on
this.
PR-URL: https://github.com/nodejs/node/pull/19368
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This commit removes the setting of hook_field[kTotals] to szero in
AsyncHook's enable function.
As far as I can tell this would not be required if the setting of
this field is done with the assignment operator instead of using the
addition assignment operator.
PR-URL: https://github.com/nodejs/node/pull/19219
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
This is a first batch of updates that touches non-underscored modules in
lib.
PR-URL: https://github.com/nodejs/node/pull/19034
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
There are two minor issues in the AsyncHook constructor, if the object
passed in has an after and/or destroy property that are not functions
the errors thrown will still be:
TypeError [ERR_ASYNC_CALLBACK]: before must be a function
This commit updates the code and adds a unit test.
PR-URL: https://github.com/nodejs/node/pull/19000
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matheus Marchini <matheus@sthima.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Instead of exposing internals of async_hooks & async_wrap throughout
the code base, create necessary helper methods within the internal
async_hooks that allows easy usage by Node.js internals. This stops
every single internal user of async_hooks from importing a ton of
functions, constants and internal Aliased Buffers from C++ async_wrap.
Adds functions initHooksExist, afterHooksExist, and destroyHooksExist
to determine whether the related emit methods need to be triggered.
Adds clearDefaultTriggerAsyncId and clearAsyncIdStack on the JS side
as an alternative to always calling C++.
Moves async_id_symbol and trigger_async_id_symbol to internal
async_hooks as they are never used in C++.
Renames newUid to newAsyncId for added clarity of its purpose.
Adjusts usage throughout the codebase, as well as in a couple of tests.
PR-URL: https://github.com/nodejs/node/pull/18720
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
The emit{Before,After} APIs in AsyncResource are problematic.
* emit{Before,After} are named to suggest that the only thing they do
is emit the before and after hooks. However, they in fact, mutate
the current execution context.
* They must be properly nested. Failure to do so by user code leads
to catastrophic (unrecoverable) exceptions. It is very easy for the
users to forget that they must be using a try/finally block around
the code that must be surrounded by these operations. Even the
example provided in the official docs makes this mistake. Failing
to use a finally can lead to a catastrophic crash if the callback
ends up throwing.
This change provides a safer `runInAsyncScope` API as an alternative
and deprecates emit{Before,After}.
PR-URL: https://github.com/nodejs/node/pull/18513
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
rename initTriggerId to defaultTriggerAsyncId such it matches the rest
of our naming.
PR-URL: https://github.com/nodejs/node/pull/17273
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
In cases where libraries create AsyncResources which may be emitting
more events depending on usage, the only way to ensure that destroy is
called properly is by calling it when the resource gets garbage
collected.
Fixes: https://github.com/nodejs/node/issues/16153
PR-URL: https://github.com/nodejs/node/pull/16998
Fixes: https://github.com/nodejs/node/issues/16153
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/15748
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Add a `promiseResolve()` hook.
PR-URL: https://github.com/nodejs/node/pull/15296
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* id values of -1 are allowed. They indicate that the id was never
correctly assigned to the async resource. These will appear in any
call graph, and will only be apparent to those using the async_hooks
module, then reported in an issue.
* ids < -1 are still not allowed and will cause the application to
exit the process; because there is no scenario where this should ever
happen.
* Add asyncId range checks to emitAfterScript().
* Fix emitBeforeScript() range checks which should have been || not &&.
* Replace errors with entries in internal/errors.
* Fix async_hooks tests that check for exceptions to match new
internal/errors entries.
NOTE: emit{Before,After,Destroy}() must continue to exit the process
because in the case of an exception during hook execution the state of
the application is unknowable. For example, an exception could cause a
memory leak:
const id_map = new Map();
before(id) {
id_map.set(id, /* data object or similar */);
},
after(id) {
throw new Error('id never dies!');
id_map.delete(id);
}
Allowing a recoverable exception may also cause an abort because of a
stack check in Environment::AsyncHooks::pop_ids() that verifies the
async id and pop'd ids match. This case would be more difficult to debug
than if fatalError() (lib/async_hooks.js) was called immediately.
try {
async_hooks.emitBefore(null, NaN);
} catch (e) { }
// do something
async_hooks.emitAfter(5);
It also allows an edge case where emitBefore() could be called twice and
not have the pop_ids() CHECK fail:
try {
async_hooks.emitBefore(5, NaN);
} catch (e) { }
async_hooks.emitBefore(5);
// do something
async_hooks.emitAfter(5);
There is the option of allowing mismatches in the stack and ignoring the
check if no async hooks are enabled, but I don't believe going this far
is necessary.
PR-URL: https://github.com/nodejs/node/pull/14722
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
* Reword several of the comments to be more descriptive.
* Rename functions to better indicate what they are doing.
* Change AsyncHooks::uid_fields_ to be a fixed array instead of a
pointer.
* Define regex early so line ends before 80 columns.
* Remove obsolete comments.
* Rename AsyncHooks::InitScope::uid_fields_ to uid_fields_ptr_ because
using the same name as AsyncHooks::uid_fields_ was confusing.
* Place variables that are used to store the new set of hooks if another
hook is enabled/disabled during hook execution into an object to act
as a namespace.
PR-URL: https://github.com/nodejs/node/pull/14722
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
These APIs were introduced during the lifetime of Node 8 in an
experimental API and should safely be removable in Node 9+.
PR-URL: https://github.com/nodejs/node/pull/14414
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
ESLint 4.3.0 flags an extra indentation issue that affects two of our
destructuring assignments in `lib`. This is a whitespace-only update to
comply prior to updating.
PR-URL: https://github.com/nodejs/node/pull/14417
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Refael Ackermann <refack@gmail.com>
AsyncResource previously called emitInitNative. Since AsyncResource is
just an abstraction on top of the emitEventScript functions, it should
call emitInitScript instead.
PR-URL: https://github.com/nodejs/node/pull/14152
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
There are two categories of emit functions in async_hooks, those used by
c++ (native) and those used by JavaScript (script). Previously these
were named N for native and S for script. Finally, there was an odd case
where emitInitN was called just init. This makes it more explicit by
using the names emitInitNative and emitInitScript. The other emit
functions are also renamed.
PR-URL: https://github.com/nodejs/node/pull/14152
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
