PR-URL: https://github.com/nodejs/node/pull/14971
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The 'test-graph.tcp' and 'test-tcpwrap' tests are using '::' as server
address. This is mapped to localhost on Linux, but fails on Windows.
PR-URL: https://github.com/nodejs/node/pull/14865
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
CRLF variable was defined but only used on line 22 so the variable
was deleted and placed inside line 22 as a string literal. This
was in file test-httpparser.request.js
On line 46 there's a function declared that takes 3 arguments but
none of them are ever used so removed. This is in file
test-httpparser.response.js
PR-URL: https://github.com/nodejs/node/pull/14818
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann <refack@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>
Raising SIGABRT is handled in the CRT in windows, calling _exit()
with ambiguous code "3" by default.
This adjustment to the abort behavior gives a more sane exit code
on abort, by calling _exit directly with code 134.
PR-URL: https://github.com/nodejs/node/pull/13947
Fixes: https://github.com/nodejs/node/issues/12271
Refs: https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/abort
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
ESLint 4.x has stricter linting than previous versions. We are currently
using the legacy indentation rules in the test directory. This commit
changes the indentation of files to comply with the stricter 4.x linting
and enable stricter linting in the test directory.
PR-URL: https://github.com/nodejs/node/pull/14431
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@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>
If TTY isn't available then the test will always fail. Also use the
already available process.stdin instead of opening another ReadStream.
PR-URL: https://github.com/nodejs/node/pull/13991
Fixes: https://github.com/nodejs/node/issues/13984
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Timers and nextTick have special emitBefore and emitAfter functions for
historic reasons. These function are not needed any more, thus the
public emitBefore and emitAfter function can be used.
PR-URL: https://github.com/nodejs/node/pull/14050
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
In the case where triggerAsyncId is null it should default to the
current executionAsyncId. This worked but as a side-effect the resource
object was changed too.
This fix also makes the null check more strict. EmitInitS is not a
documented API, thus there is no reason to be flexible in its input.
Ref: https://github.com/nodejs/node/issues/13548#issuecomment-310985270
PR-URL: https://github.com/nodejs/node/pull/14026
Reviewed-By: Refael Ackermann <refack@gmail.com>
If the .listen() hasn't been called on the server, there is no handle
object. In this case use null as the triggerAsyncId.
Fixes: https://github.com/nodejs/node/issues/13548
PR-URL: https://github.com/nodejs/node/pull/13938
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Since V8 5.9 V8 installs a default signal handler for some signals
when creating a default platform instance that prints a stack trace.
However, Node already does the same thing, so it would seem like the
two different stack traces would be printed; also, the V8 handler
would lead to a `SIGSEGV` under some circumstances, rather than
letting the abort continue normally.
Resolve this by disabling V8’s signal handler by default.
PR-URL: https://github.com/nodejs/node/pull/13985
Fixes: https://github.com/nodejs/node/issues/13865
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Watching directories has limited support on AIX. This is documented.
Watch a file in test/async-hooks/test-fseventwrap.js to accommodate AIX.
PR-URL: https://github.com/nodejs/node/pull/13766
Ref: https://github.com/nodejs/node/issues/13577#issuecomment-308038674
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
currentId is renamed to executionAsyncId
triggerId is renamed to triggerAsyncId
AsyncResource.triggerId is renamed to AsyncResource.triggerAsyncId
AsyncHooksGetCurrentId is renamed to AsyncHooksGetExecutionAsyncId
AsyncHooksGetTriggerId is renamed to AsyncHooksGetTriggerAsyncId
PR-URL: https://github.com/nodejs/node/pull/13490
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Two child processes have their logic in a switch statement and a third
uses an `if` statement to detect it. Move all three child process tasks
into switch statement.
PR-URL: https://github.com/nodejs/node/pull/13554
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
PR-URL: https://github.com/nodejs/node/pull/13336
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Improve error messages in the async hooks tests, mostly by removing
unhelpful `message` parameters for assertions.
PR-URL: https://github.com/nodejs/node/pull/13243
Reviewed-By: Kunal Pathak <kunal.pathak@microsoft.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: Trevor Norris <trev.norris@gmail.com>
async_hooks init callback will be triggered when promise newly created,
in previous version, the parent promise which pass from chrome V8
PromiseHook is ignored, so we can't tell the promise is a pure
new promise or a chained promise.
In this commit, we use the parent promise's id as triggerId to
trigger the init callback.
Fixes: https://github.com/nodejs/node/issues/13302
PR-URL: https://github.com/nodejs/node/pull/13367
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Calling the destroy callbacks in a uv_idle_t causes a timing issue where
if a handle or request is closed then the class isn't deleted until
uv_close() callbacks are called (which happens after the poll phase).
This results in some destroy callbacks not being called just before the
application exits. So instead switch the destroy callbacks to be called
in a uv_timer_t with the timeout set to zero.
When uv_run() is called with UV_RUN_ONCE the final operation of the
event loop is to process all remaining timers. By setting the timeout to
zero it results in the destroy callbacks being processed after
uv_close() but before uv_run() returned. Processing the destroyed ids
that were previously missed.
Also, process the destroy_ids_list() in a do {} while() loop that makes
sure the vector is empty before returning. Which also makes running
clear() unnecessary.
Fixes: https://github.com/nodejs/node/issues/13262
PR-URL: https://github.com/nodejs/node/pull/13369
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
`AsyncEvent` is not a good name given its semantics.
PR-URL: https://github.com/nodejs/node/pull/13192
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This change provides unified tracking of asynchronous promise lifecycles
for both domains and async hooks.
PR-URL: https://github.com/nodejs/node/pull/13000
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This reverts commit 571882c5a4.
Removing the process.nextTick() call can prevent the consumer
from being able to catch error events.
PR-URL: https://github.com/nodejs/node/pull/12854
Fixes: https://github.com/nodejs/node/issues/12841
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Currently the async provider type CONNECTION is used in node_crypto.h
and it might be clearer if it was named SSLCONNECTION as suggested by
addaleax.
This commit renames only the provider type as I was not sure if it was
alright to change the class Connection as well.
Refs: https://github.com/nodejs/node/pull/12967#discussion_r115978735
PR-URL: https://github.com/nodejs/node/pull/12989
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Currently when configured --without-ssl these test will fail. In
test-crypto-pbkdf2.js and test-crypto-randomBytes.js the check exists
but just need to be moved before the require of crypto.
There was no check in test-async-wrap-uncaughtexception.js so one was
added.
PR-URL: https://github.com/nodejs/node/pull/12968
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Luca Maraschi <luca.maraschi@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Async wrap providers tested:
- crypto.randomBytes
- crypto.pbkdf2
- fs event wrap
- fsreqwrap access
- fsreqwrap readFile
- getaddrinforeq wrap
- getnameinforeq wrap
- pipe connect wrap
- query wrap
- pipewrap
- processwrap
- shutdown wrap
- tcpwrap
- udpwrap
- send wrap
- detailed signal wrap
- statwatcher
- timerwrap via setTimeout
- timerwrap via setInterval
- for Immediate
- http parser request
- http parser response
- connection via ssl server
- tls wrap
- write wrap
- ttywrap via readstream
- ttywrap via wriream
- zctx via zlib binding deflate
Embedder API:
- async-event tests
- one test looks at the happy paths
- another ensures that in cases of events emitted in an order that
doesn't make sense, the order is enforced by async hooks throwing a
meaningful error
- embedder enforcement tests are split up since async hook stack
corruption now the process
- therefore we launch a child and check for error output of the offending code
Additional tests:
- tests that show that we can enable/disable hooks inside their lifetime
events
- tests that verify the graph of resources triggering the creation of
other resources
Test Helpers:
- init-hooks:
- returns one collector instance
- when created an async hook is created and the lifetime events are
registered to call the appropriate collector functions
- the collector also exposes `enable` and `disable` functions which call
through to the async hook
- hook checks:
- checks invocations of life time hooks against the actual invocations
that were collected
- in some cases like `destroy` a min/max range of invocations can be
supplied since in these cases the exact number is non-deterministic
- verify graph:
- verifies the triggerIds of specific async resources are as expected,
i.e. the creation of resources was triggered by the resource we expect
- includes a printGraph function to generate easily readable test
input for verify graph
- both functions prune TickObjects to create less brittle and easier
to understand tests
PR-URL: https://github.com/nodejs/node/pull/12892
Ref: https://github.com/nodejs/node/pull/11883
Ref: https://github.com/nodejs/node/pull/8531
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Simon Brewster
Miguel Angel Asencio Hurtado
Mohd Maqbool Alam
Bartosz Sosnowski
Runite618
Trevor Norris
Jared Kantrowitz
erdun
Rich Trott
Song, Bintao Garfield
Sebastiaan Deckers
Vincent Xue
Matteo Collina
jkzing
vercent deng
weiyuanyue
blade254353074
Andreas Madsen
Matt Sergeant
Vse Mozhet Byt
Anna Henningsen
Refael Ackermann
Roman Reiss
Brian White
David Cai
JiaLi.Passion
Matt Loring
Evan Lucas
Daniel Bevenius
Thorsten Lorenz