From 7a5e3279d0fbd453ca2075934c50656d52fae391 Mon Sep 17 00:00:00 2001 From: Yihong Wang Date: Sat, 21 Oct 2017 23:16:50 -0700 Subject: [PATCH 01/22] src: explicitly register built-in modules Previously, built-in modules are registered before main() via __attribute__((constructor)) mechanism in GCC and similiar mechanism in MSVC. This causes some issues when node is built as static library. Calling module registration function for built-in modules in node::Init() helps to avoid the issues. Signed-off-by: Yihong Wang PR-URL: https://github.com/nodejs/node/pull/16565 Refs: https://github.com/nodejs/node/pull/14986#issuecomment-332758206 Reviewed-By: Gireesh Punathil Reviewed-By: Anna Henningsen Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell Reviewed-By: Colin Ihrig --- node.gyp | 1 + src/async-wrap.cc | 3 +- src/cares_wrap.cc | 2 +- src/fs_event_wrap.cc | 2 +- src/inspector_js_api.cc | 2 +- src/js_stream.cc | 2 +- src/node.cc | 24 +++++++++- src/node_buffer.cc | 2 +- src/node_config.cc | 2 +- src/node_contextify.cc | 2 +- src/node_crypto.cc | 2 +- src/node_file.cc | 2 +- src/node_http2.cc | 2 +- src/node_http_parser.cc | 2 +- src/node_i18n.cc | 2 +- src/node_internals.h | 86 ++++++++++++++++++++++++++++++++-- src/node_os.cc | 2 +- src/node_perf.cc | 2 +- src/node_serdes.cc | 2 +- src/node_url.cc | 2 +- src/node_util.cc | 2 +- src/node_v8.cc | 3 +- src/node_zlib.cc | 2 +- src/pipe_wrap.cc | 2 +- src/process_wrap.cc | 2 +- src/signal_wrap.cc | 2 +- src/spawn_sync.cc | 2 +- src/stream_wrap.cc | 2 +- src/tcp_wrap.cc | 2 +- src/timer_wrap.cc | 2 +- src/tls_wrap.cc | 2 +- src/tty_wrap.cc | 2 +- src/udp_wrap.cc | 2 +- src/uv.cc | 3 +- test/cctest/node_module_reg.cc | 28 +++++++++++ 35 files changed, 167 insertions(+), 37 deletions(-) create mode 100644 test/cctest/node_module_reg.cc diff --git a/node.gyp b/node.gyp index 4ae9d36a552413..3c598d8770172c 100644 --- a/node.gyp +++ b/node.gyp @@ -798,6 +798,7 @@ 'defines': [ 'NODE_WANT_INTERNALS=1' ], 'sources': [ + 'test/cctest/node_module_reg.cc', 'test/cctest/node_test_fixture.cc', 'test/cctest/test_aliased_buffer.cc', 'test/cctest/test_base64.cc', diff --git a/src/async-wrap.cc b/src/async-wrap.cc index 1b1452b69379c7..ee07ebb0368cda 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -729,8 +729,7 @@ void EmitAsyncDestroy(Isolate* isolate, async_context asyncContext) { } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(async_wrap, node::AsyncWrap::Initialize) - +NODE_BUILTIN_MODULE_CONTEXT_AWARE(async_wrap, node::AsyncWrap::Initialize) // Only legacy public API below this line. diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index 605709c61945fd..1459dfafab030c 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -2241,4 +2241,4 @@ void Initialize(Local target, } // namespace cares_wrap } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(cares_wrap, node::cares_wrap::Initialize) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(cares_wrap, node::cares_wrap::Initialize) diff --git a/src/fs_event_wrap.cc b/src/fs_event_wrap.cc index 5a8693c380b822..a7af5293bbfc72 100644 --- a/src/fs_event_wrap.cc +++ b/src/fs_event_wrap.cc @@ -222,4 +222,4 @@ void FSEventWrap::Close(const FunctionCallbackInfo& args) { } // anonymous namespace } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(fs_event_wrap, node::FSEventWrap::Initialize) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(fs_event_wrap, node::FSEventWrap::Initialize) diff --git a/src/inspector_js_api.cc b/src/inspector_js_api.cc index f72517bac18c70..df700bf6a1b671 100644 --- a/src/inspector_js_api.cc +++ b/src/inspector_js_api.cc @@ -348,5 +348,5 @@ void InitInspectorBindings(Local target, Local unused, } // namespace inspector } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(inspector, +NODE_BUILTIN_MODULE_CONTEXT_AWARE(inspector, node::inspector::InitInspectorBindings); diff --git a/src/js_stream.cc b/src/js_stream.cc index 9d28b90585048a..842fc3b711bd51 100644 --- a/src/js_stream.cc +++ b/src/js_stream.cc @@ -251,4 +251,4 @@ void JSStream::Initialize(Local target, } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(js_stream, node::JSStream::Initialize) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(js_stream, node::JSStream::Initialize) diff --git a/src/node.cc b/src/node.cc index fa8b44e64d0cfd..f70d2bd7448a8c 100644 --- a/src/node.cc +++ b/src/node.cc @@ -120,6 +120,16 @@ typedef int mode_t; extern char **environ; #endif +// This is used to load built-in modules. Instead of using +// __attribute__((constructor)), we call the _register_ +// function for each built-in modules explicitly in +// node::RegisterBuiltinModules(). This is only forward declaration. +// The definitions are in each module's implementation when calling +// the NODE_BUILTIN_MODULE_CONTEXT_AWARE. +#define V(modname) void _register_##modname(); + NODE_BUILTIN_MODULES(V) +#undef V + namespace node { using v8::Array; @@ -4572,6 +4582,9 @@ void Init(int* argc, // Initialize prog_start_time to get relative uptime. prog_start_time = static_cast(uv_now(uv_default_loop())); + // Register built-in modules + node::RegisterBuiltinModules(); + // Make inherited handles noninheritable. uv_disable_stdio_inheritance(); @@ -4947,11 +4960,18 @@ int Start(int argc, char** argv) { return exit_code; } +// Call built-in modules' _register_ function to +// do module registration explicitly. +void RegisterBuiltinModules() { +#define V(modname) _register_##modname(); + NODE_BUILTIN_MODULES(V) +#undef V +} } // namespace node #if !HAVE_INSPECTOR -static void InitEmptyBindings() {} +void InitEmptyBindings() {} -NODE_MODULE_CONTEXT_AWARE_BUILTIN(inspector, InitEmptyBindings) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(inspector, InitEmptyBindings) #endif // !HAVE_INSPECTOR diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 0828091a1dfb6e..66ae9feb697a32 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -1304,4 +1304,4 @@ void Initialize(Local target, } // namespace Buffer } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(buffer, node::Buffer::Initialize) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(buffer, node::Buffer::Initialize) diff --git a/src/node_config.cc b/src/node_config.cc index 7fa275b8df05cc..07f891cf0c012a 100644 --- a/src/node_config.cc +++ b/src/node_config.cc @@ -129,4 +129,4 @@ static void InitConfig(Local target, } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(config, node::InitConfig) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(config, node::InitConfig) diff --git a/src/node_contextify.cc b/src/node_contextify.cc index c297a7fadb7c51..47857e8cab0a10 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -1085,4 +1085,4 @@ void InitContextify(Local target, } // anonymous namespace } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(contextify, node::InitContextify) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(contextify, node::InitContextify) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 5504ea3f7ee6cb..7255fa32dbcdde 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -6195,4 +6195,4 @@ void InitCrypto(Local target, } // namespace crypto } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(crypto, node::crypto::InitCrypto) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(crypto, node::crypto::InitCrypto) diff --git a/src/node_file.cc b/src/node_file.cc index 2b67978556316c..9de7fe0e378642 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -1518,4 +1518,4 @@ void InitFs(Local target, } // end namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(fs, node::InitFs) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(fs, node::InitFs) diff --git a/src/node_http2.cc b/src/node_http2.cc index 4235ba0f17f47f..89d68de88f8cfe 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -2250,4 +2250,4 @@ HTTP_STATUS_CODES(V) } // namespace http2 } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(http2, node::http2::Initialize) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(http2, node::http2::Initialize) diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index 329da616f7cf1a..aa325277f6c6dd 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -810,4 +810,4 @@ void InitHttpParser(Local target, } // anonymous namespace } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(http_parser, node::InitHttpParser) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(http_parser, node::InitHttpParser) diff --git a/src/node_i18n.cc b/src/node_i18n.cc index 1b3f0293e2f3f0..cc02092dd8630a 100644 --- a/src/node_i18n.cc +++ b/src/node_i18n.cc @@ -874,6 +874,6 @@ void Init(Local target, } // namespace i18n } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(icu, node::i18n::Init) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(icu, node::i18n::Init) #endif // NODE_HAVE_I18N_SUPPORT diff --git a/src/node_internals.h b/src/node_internals.h index 4baeed9e56247a..55798d2e7a87f6 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -56,6 +56,81 @@ struct sockaddr; constant_attributes).FromJust(); \ } while (0) + +#if HAVE_OPENSSL +#define NODE_BUILTIN_OPENSSL_MODULES(V) V(crypto) V(tls_wrap) +#else +#define NODE_BUILTIN_OPENSSL_MODULES(V) +#endif + +#if NODE_HAVE_I18N_SUPPORT +#define NODE_BUILTIN_ICU_MODULES(V) V(icu) +#else +#define NODE_BUILTIN_ICU_MODULES(V) +#endif + +// A list of built-in modules. In order to do module registration +// in node::Init(), need to add built-in modules in the following list. +// Then in node::RegisterBuiltinModules(), it calls modules' registration +// function. This helps the built-in modules are loaded properly when +// node is built as static library. No need to depends on the +// __attribute__((constructor)) like mechanism in GCC. +#define NODE_BUILTIN_STANDARD_MODULES(V) \ + V(async_wrap) \ + V(buffer) \ + V(cares_wrap) \ + V(config) \ + V(contextify) \ + V(fs) \ + V(fs_event_wrap) \ + V(http2) \ + V(http_parser) \ + V(inspector) \ + V(js_stream) \ + V(module_wrap) \ + V(os) \ + V(performance) \ + V(pipe_wrap) \ + V(process_wrap) \ + V(serdes) \ + V(signal_wrap) \ + V(spawn_sync) \ + V(stream_wrap) \ + V(tcp_wrap) \ + V(timer_wrap) \ + V(tty_wrap) \ + V(udp_wrap) \ + V(url) \ + V(util) \ + V(uv) \ + V(v8) \ + V(zlib) + +#define NODE_BUILTIN_MODULES(V) \ + NODE_BUILTIN_STANDARD_MODULES(V) \ + NODE_BUILTIN_OPENSSL_MODULES(V) \ + NODE_BUILTIN_ICU_MODULES(V) + +#define NODE_MODULE_CONTEXT_AWARE_CPP(modname, regfunc, priv, flags) \ + static node::node_module _module = { \ + NODE_MODULE_VERSION, \ + flags, \ + nullptr, \ + __FILE__, \ + nullptr, \ + (node::addon_context_register_func) (regfunc), \ + NODE_STRINGIFY(modname), \ + priv, \ + nullptr \ + }; \ + void _register_ ## modname() { \ + node_module_register(&_module); \ + } + + +#define NODE_BUILTIN_MODULE_CONTEXT_AWARE(modname, regfunc) \ + NODE_MODULE_CONTEXT_AWARE_CPP(modname, regfunc, nullptr, NM_F_BUILTIN) + namespace node { // Set in node.cc by ParseArgs with the value of --openssl-config. @@ -182,6 +257,12 @@ void SetupProcessObject(Environment* env, int exec_argc, const char* const* exec_argv); +// Call _register functions for all of +// the built-in modules. Because built-in modules don't +// use the __attribute__((constructor)). Need to +// explicitly call the _register* functions. +void RegisterBuiltinModules(); + enum Endianness { kLittleEndian, // _Not_ LITTLE_ENDIAN, clashes with endian.h. kBigEndian @@ -305,9 +386,8 @@ class InternalCallbackScope { bool closed_ = false; }; -#define NODE_MODULE_CONTEXT_AWARE_INTERNAL(modname, regfunc) \ - /* NOLINTNEXTLINE (readability/null_usage) */ \ - NODE_MODULE_CONTEXT_AWARE_X(modname, regfunc, NULL, NM_F_INTERNAL) \ +#define NODE_MODULE_CONTEXT_AWARE_INTERNAL(modname, regfunc) \ + NODE_MODULE_CONTEXT_AWARE_CPP(modname, regfunc, nullptr, NM_F_INTERNAL) } // namespace node diff --git a/src/node_os.cc b/src/node_os.cc index 98fa6087f3b5b3..4ebca206a57c17 100644 --- a/src/node_os.cc +++ b/src/node_os.cc @@ -430,4 +430,4 @@ void Initialize(Local target, } // namespace os } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(os, node::os::Initialize) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(os, node::os::Initialize) diff --git a/src/node_perf.cc b/src/node_perf.cc index a3b428bac94fc4..94c3a0f8e0c047 100644 --- a/src/node_perf.cc +++ b/src/node_perf.cc @@ -390,4 +390,4 @@ void Init(Local target, } // namespace performance } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(performance, node::performance::Init) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(performance, node::performance::Init) diff --git a/src/node_serdes.cc b/src/node_serdes.cc index 4e99513a5fd31c..19b6f163e6ef7d 100644 --- a/src/node_serdes.cc +++ b/src/node_serdes.cc @@ -483,4 +483,4 @@ void InitializeSerdesBindings(Local target, } // anonymous namespace } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(serdes, node::InitializeSerdesBindings) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(serdes, node::InitializeSerdesBindings) diff --git a/src/node_url.cc b/src/node_url.cc index b0cfc626598227..ad9996a4ce5765 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -2220,4 +2220,4 @@ static void Init(Local target, } // namespace url } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(url, node::url::Init) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(url, node::url::Init) diff --git a/src/node_util.cc b/src/node_util.cc index ab1f3c9f91257d..a40401b71926b3 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -238,4 +238,4 @@ void Initialize(Local target, } // namespace util } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(util, node::util::Initialize) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(util, node::util::Initialize) diff --git a/src/node_v8.cc b/src/node_v8.cc index 8b4d31a880dc0c..0fbae2d8a297a8 100644 --- a/src/node_v8.cc +++ b/src/node_v8.cc @@ -20,6 +20,7 @@ // USE OR OTHER DEALINGS IN THE SOFTWARE. #include "node.h" +#include "node_internals.h" #include "env-inl.h" #include "util-inl.h" #include "v8.h" @@ -206,4 +207,4 @@ void InitializeV8Bindings(Local target, } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(v8, node::InitializeV8Bindings) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(v8, node::InitializeV8Bindings) diff --git a/src/node_zlib.cc b/src/node_zlib.cc index 1a623075a5a421..0accd6441a5889 100644 --- a/src/node_zlib.cc +++ b/src/node_zlib.cc @@ -701,4 +701,4 @@ void InitZlib(Local target, } // anonymous namespace } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(zlib, node::InitZlib) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(zlib, node::InitZlib) diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc index b76b31e81df45b..bb5a6d27dd2316 100644 --- a/src/pipe_wrap.cc +++ b/src/pipe_wrap.cc @@ -203,4 +203,4 @@ void PipeWrap::Connect(const FunctionCallbackInfo& args) { } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(pipe_wrap, node::PipeWrap::Initialize) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(pipe_wrap, node::PipeWrap::Initialize) diff --git a/src/process_wrap.cc b/src/process_wrap.cc index a73e4d9779ed46..3667b0449e4e2c 100644 --- a/src/process_wrap.cc +++ b/src/process_wrap.cc @@ -311,4 +311,4 @@ class ProcessWrap : public HandleWrap { } // anonymous namespace } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(process_wrap, node::ProcessWrap::Initialize) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(process_wrap, node::ProcessWrap::Initialize) diff --git a/src/signal_wrap.cc b/src/signal_wrap.cc index d5bbddc8a025e7..66ec8d69faa2e1 100644 --- a/src/signal_wrap.cc +++ b/src/signal_wrap.cc @@ -127,4 +127,4 @@ class SignalWrap : public HandleWrap { } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(signal_wrap, node::SignalWrap::Initialize) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(signal_wrap, node::SignalWrap::Initialize) diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc index 626ecbb04ff5b7..4e51cc5d8f1dca 100644 --- a/src/spawn_sync.cc +++ b/src/spawn_sync.cc @@ -1081,5 +1081,5 @@ void SyncProcessRunner::KillTimerCloseCallback(uv_handle_t* handle) { } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(spawn_sync, +NODE_BUILTIN_MODULE_CONTEXT_AWARE(spawn_sync, node::SyncProcessRunner::Initialize) diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index c2915ac453ea33..b089f1506746dc 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -399,5 +399,5 @@ void LibuvStreamWrap::OnAfterWriteImpl(WriteWrap* w, void* ctx) { } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(stream_wrap, +NODE_BUILTIN_MODULE_CONTEXT_AWARE(stream_wrap, node::LibuvStreamWrap::Initialize) diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index ce86108cb0acbb..af64f89b546079 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -360,4 +360,4 @@ Local AddressToJS(Environment* env, } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(tcp_wrap, node::TCPWrap::Initialize) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(tcp_wrap, node::TCPWrap::Initialize) diff --git a/src/timer_wrap.cc b/src/timer_wrap.cc index 08701b4ff08d55..dc4c2ea5c3def3 100644 --- a/src/timer_wrap.cc +++ b/src/timer_wrap.cc @@ -137,4 +137,4 @@ class TimerWrap : public HandleWrap { } // anonymous namespace } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(timer_wrap, node::TimerWrap::Initialize) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(timer_wrap, node::TimerWrap::Initialize) diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index 3315267330afa2..d5b56ca9ba3dd7 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -982,4 +982,4 @@ void TLSWrap::Initialize(Local target, } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(tls_wrap, node::TLSWrap::Initialize) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(tls_wrap, node::TLSWrap::Initialize) diff --git a/src/tty_wrap.cc b/src/tty_wrap.cc index 872a126c6d4ee4..ae8ff64f70c29a 100644 --- a/src/tty_wrap.cc +++ b/src/tty_wrap.cc @@ -175,4 +175,4 @@ TTYWrap::TTYWrap(Environment* env, } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(tty_wrap, node::TTYWrap::Initialize) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(tty_wrap, node::TTYWrap::Initialize) diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index 54f1b610238ff9..73a976057719fa 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -511,4 +511,4 @@ uv_udp_t* UDPWrap::UVHandle() { } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(udp_wrap, node::UDPWrap::Initialize) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(udp_wrap, node::UDPWrap::Initialize) diff --git a/src/uv.cc b/src/uv.cc index fbe61c9b0164d7..6cbba07e59039c 100644 --- a/src/uv.cc +++ b/src/uv.cc @@ -21,6 +21,7 @@ #include "uv.h" #include "node.h" +#include "node_internals.h" #include "env-inl.h" namespace node { @@ -61,4 +62,4 @@ void InitializeUV(Local target, } // anonymous namespace } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(uv, node::InitializeUV) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(uv, node::InitializeUV) diff --git a/test/cctest/node_module_reg.cc b/test/cctest/node_module_reg.cc new file mode 100644 index 00000000000000..f8d9d03c1cdb99 --- /dev/null +++ b/test/cctest/node_module_reg.cc @@ -0,0 +1,28 @@ +// Need to create empty definition for these modules' +// registration function for cctest. Because when +// building cctest, the definitions for the following +// registration functions are not included. +void _register_cares_wrap() {} +void _register_config() {} +void _register_contextify() {} +void _register_fs() {} +void _register_fs_event_wrap() {} +void _register_http2() {} +void _register_http_parser() {} +void _register_js_stream() {} +void _register_module_wrap() {} +void _register_os() {} +void _register_pipe_wrap() {} +void _register_process_wrap() {} +void _register_serdes() {} +void _register_signal_wrap() {} +void _register_spawn_sync() {} +void _register_stream_wrap() {} +void _register_tcp_wrap() {} +void _register_timer_wrap() {} +void _register_tty_wrap() {} +void _register_udp_wrap() {} +void _register_util() {} +void _register_uv() {} +void _register_v8() {} +void _register_zlib() {} From 6e98aa9297b68cea953815ed228034298dc13ae3 Mon Sep 17 00:00:00 2001 From: Franziska Hinkelmann Date: Sun, 12 Nov 2017 22:16:10 +0100 Subject: [PATCH 02/22] src: use unique pointer for tracing_agent Use std::unique_ptr instead of raw pointers for the tracing_agent_ in node.cc. This makes ownership clearer and we don't risk a memory leak. PR-URL: https://github.com/nodejs/node/pull/17012 Reviewed-By: Daniel Bevenius Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Timothy Gu --- src/node.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/node.cc b/src/node.cc index f70d2bd7448a8c..946260bf665bde 100644 --- a/src/node.cc +++ b/src/node.cc @@ -272,8 +272,8 @@ node::DebugOptions debug_options; static struct { #if NODE_USE_V8_PLATFORM void Initialize(int thread_pool_size, uv_loop_t* loop) { - tracing_agent_ = - trace_enabled ? new tracing::Agent() : nullptr; + tracing_agent_.reset( + trace_enabled ? new tracing::Agent() : nullptr); platform_ = new NodePlatform(thread_pool_size, loop, trace_enabled ? tracing_agent_->GetTracingController() : nullptr); V8::InitializePlatform(platform_); @@ -285,8 +285,7 @@ static struct { platform_->Shutdown(); delete platform_; platform_ = nullptr; - delete tracing_agent_; - tracing_agent_ = nullptr; + tracing_agent_.reset(nullptr); } void DrainVMTasks() { @@ -315,7 +314,7 @@ static struct { tracing_agent_->Stop(); } - tracing::Agent* tracing_agent_; + std::unique_ptr tracing_agent_; NodePlatform* platform_; #else // !NODE_USE_V8_PLATFORM void Initialize(int thread_pool_size, uv_loop_t* loop) {} From 2bffd70ebfe3fc00b35c1d1cb0c762643eb81106 Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Mon, 17 Jul 2017 16:47:12 -0700 Subject: [PATCH 03/22] async_hooks: add trace events to async_hooks This will allow trace event to record timing information for all asynchronous operations that are observed by async_hooks. PR-URL: https://github.com/nodejs/node/pull/15538 Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Anna Henningsen --- doc/api/tracing.md | 4 +- lib/_http_common.js | 3 +- lib/internal/bootstrap_node.js | 1 + lib/internal/trace_events_async_hooks.js | 67 +++++++++ node.gyp | 2 + src/async-wrap.cc | 102 ++++++++++--- src/async-wrap.h | 14 +- src/node.cc | 21 ++- src/node_http_parser.cc | 13 ++ src/node_trace_events.cc | 136 ++++++++++++++++++ src/tracing/agent.cc | 1 + test/parallel/test-trace-event.js | 41 ------ test/parallel/test-trace-events-all.js | 56 ++++++++ .../parallel/test-trace-events-async-hooks.js | 58 ++++++++ test/parallel/test-trace-events-binding.js | 48 +++++++ .../test-trace-events-category-used.js | 35 +++++ test/parallel/test-trace-events-none.js | 20 +++ test/parallel/test-trace-events-v8.js | 58 ++++++++ 18 files changed, 607 insertions(+), 73 deletions(-) create mode 100644 lib/internal/trace_events_async_hooks.js create mode 100644 src/node_trace_events.cc delete mode 100644 test/parallel/test-trace-event.js create mode 100644 test/parallel/test-trace-events-all.js create mode 100644 test/parallel/test-trace-events-async-hooks.js create mode 100644 test/parallel/test-trace-events-binding.js create mode 100644 test/parallel/test-trace-events-category-used.js create mode 100644 test/parallel/test-trace-events-none.js create mode 100644 test/parallel/test-trace-events-v8.js diff --git a/doc/api/tracing.md b/doc/api/tracing.md index 99687cdc3c17fa..e03477b1adf20c 100644 --- a/doc/api/tracing.md +++ b/doc/api/tracing.md @@ -10,10 +10,10 @@ Node.js application. The set of categories for which traces are recorded can be specified using the `--trace-event-categories` flag followed by a list of comma separated category names. -By default the `node` and `v8` categories are enabled. +By default the `node`, `node.async_hooks`, and `v8` categories are enabled. ```txt -node --trace-events-enabled --trace-event-categories v8,node server.js +node --trace-events-enabled --trace-event-categories v8,node,node.async_hooks server.js ``` Running Node.js with tracing enabled will produce log files that can be opened diff --git a/lib/_http_common.js b/lib/_http_common.js index a3e5f072b42ff6..ad0dec520d1210 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -26,7 +26,6 @@ const { methods, HTTPParser } = process.binding('http_parser'); const FreeList = require('internal/freelist'); const { ondrain } = require('internal/http'); const incoming = require('_http_incoming'); -const { emitDestroy } = require('async_hooks'); const { IncomingMessage, readStart, @@ -217,7 +216,7 @@ function freeParser(parser, req, socket) { } else { // Since the Parser destructor isn't going to run the destroy() callbacks // it needs to be triggered manually. - emitDestroy(parser.getAsyncId()); + parser.free(); } } if (req) { diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index 48a4588543678e..bebf47e5837860 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -55,6 +55,7 @@ if (global.__coverage__) NativeModule.require('internal/process/write-coverage').setup(); + NativeModule.require('internal/trace_events_async_hooks').setup(); NativeModule.require('internal/inspector_async_hook').setup(); // Do not initialize channel in debugger agent, it deletes env variable diff --git a/lib/internal/trace_events_async_hooks.js b/lib/internal/trace_events_async_hooks.js new file mode 100644 index 00000000000000..9f47f2aa5fd658 --- /dev/null +++ b/lib/internal/trace_events_async_hooks.js @@ -0,0 +1,67 @@ +'use strict'; + +const trace_events = process.binding('trace_events'); +const async_wrap = process.binding('async_wrap'); +const async_hooks = require('async_hooks'); + +// Use small letters such that chrome://traceing groups by the name. +// The behaviour is not only useful but the same as the events emitted using +// the specific C++ macros. +const BEFORE_EVENT = 'b'.charCodeAt(0); +const END_EVENT = 'e'.charCodeAt(0); + +// In trace_events it is not only the id but also the name that needs to be +// repeated. Since async_hooks doesn't expose the provider type in the +// non-init events, use a map to manually map the asyncId to the type name. +const typeMemory = new Map(); + +// It is faster to emit trace_events directly from C++. Thus, this happens in +// async_wrap.cc. However, events emitted from the JavaScript API or the +// Embedder C++ API can't be emitted from async_wrap.cc. Thus they are +// emitted using the JavaScript API. To prevent emitting the same event +// twice the async_wrap.Providers list is used to filter the events. +const nativeProviders = new Set(Object.keys(async_wrap.Providers)); + +const hook = async_hooks.createHook({ + init(asyncId, type, triggerAsyncId, resource) { + if (nativeProviders.has(type)) return; + + typeMemory.set(asyncId, type); + trace_events.emit(BEFORE_EVENT, 'node.async_hooks', + type, asyncId, 'triggerAsyncId', triggerAsyncId); + }, + + before(asyncId) { + const type = typeMemory.get(asyncId); + if (type === undefined) return; + + trace_events.emit(BEFORE_EVENT, 'node.async_hooks', + type + '_CALLBACK', asyncId); + }, + + after(asyncId) { + const type = typeMemory.get(asyncId); + if (type === undefined) return; + + trace_events.emit(END_EVENT, 'node.async_hooks', + type + '_CALLBACK', asyncId); + }, + + destroy(asyncId) { + const type = typeMemory.get(asyncId); + if (type === undefined) return; + + trace_events.emit(END_EVENT, 'node.async_hooks', + type, asyncId); + + // cleanup asyncId to type map + typeMemory.delete(asyncId); + } +}); + + +exports.setup = function() { + if (trace_events.categoryGroupEnabled('node.async_hooks')) { + hook.enable(); + } +}; diff --git a/node.gyp b/node.gyp index 3c598d8770172c..f268c611bb2744 100644 --- a/node.gyp +++ b/node.gyp @@ -113,6 +113,7 @@ 'lib/internal/repl.js', 'lib/internal/socket_list.js', 'lib/internal/test/unicode.js', + 'lib/internal/trace_events_async_hooks.js', 'lib/internal/url.js', 'lib/internal/util.js', 'lib/internal/util/types.js', @@ -202,6 +203,7 @@ 'src/node_platform.cc', 'src/node_perf.cc', 'src/node_serdes.cc', + 'src/node_trace_events.cc', 'src/node_url.cc', 'src/node_util.cc', 'src/node_v8.cc', diff --git a/src/async-wrap.cc b/src/async-wrap.cc index ee07ebb0368cda..6281a204accd4a 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -168,18 +168,6 @@ static void DestroyAsyncIdsCallback(uv_timer_t* handle) { } -static void PushBackDestroyAsyncId(Environment* env, double id) { - if (env->async_hooks()->fields()[AsyncHooks::kDestroy] == 0) - return; - - if (env->destroy_async_id_list()->empty()) - uv_timer_start(env->destroy_async_ids_timer_handle(), - DestroyAsyncIdsCallback, 0, 0); - - env->destroy_async_id_list()->push_back(id); -} - - void AsyncWrap::EmitPromiseResolve(Environment* env, double async_id) { AsyncHooks* async_hooks = env->async_hooks(); @@ -199,6 +187,21 @@ void AsyncWrap::EmitPromiseResolve(Environment* env, double async_id) { } +void AsyncWrap::EmitTraceEventBefore() { + switch (provider_type()) { +#define V(PROVIDER) \ + case PROVIDER_ ## PROVIDER: \ + TRACE_EVENT_NESTABLE_ASYNC_BEGIN0("node.async_hooks", \ + #PROVIDER "_CALLBACK", static_cast(get_async_id())); \ + break; + NODE_ASYNC_PROVIDER_TYPES(V) +#undef V + default: + UNREACHABLE(); + } +} + + void AsyncWrap::EmitBefore(Environment* env, double async_id) { AsyncHooks* async_hooks = env->async_hooks(); @@ -218,6 +221,21 @@ void AsyncWrap::EmitBefore(Environment* env, double async_id) { } +void AsyncWrap::EmitTraceEventAfter() { + switch (provider_type()) { +#define V(PROVIDER) \ + case PROVIDER_ ## PROVIDER: \ + TRACE_EVENT_NESTABLE_ASYNC_END0("node.async_hooks", \ + #PROVIDER "_CALLBACK", static_cast(get_async_id())); \ + break; + NODE_ASYNC_PROVIDER_TYPES(V) +#undef V + default: + UNREACHABLE(); + } +} + + void AsyncWrap::EmitAfter(Environment* env, double async_id) { AsyncHooks* async_hooks = env->async_hooks(); @@ -328,8 +346,10 @@ static void PromiseHook(PromiseHookType type, Local promise, if (type == PromiseHookType::kBefore) { env->async_hooks()->push_async_ids( wrap->get_async_id(), wrap->get_trigger_async_id()); + wrap->EmitTraceEventBefore(); AsyncWrap::EmitBefore(wrap->env(), wrap->get_async_id()); } else if (type == PromiseHookType::kAfter) { + wrap->EmitTraceEventAfter(); AsyncWrap::EmitAfter(wrap->env(), wrap->get_async_id()); if (env->execution_async_id() == wrap->get_async_id()) { // This condition might not be true if async_hooks was enabled during @@ -456,7 +476,8 @@ void AsyncWrap::AsyncReset(const FunctionCallbackInfo& args) { void AsyncWrap::QueueDestroyAsyncId(const FunctionCallbackInfo& args) { CHECK(args[0]->IsNumber()); - PushBackDestroyAsyncId(Environment::GetCurrent(args), args[0]->NumberValue()); + AsyncWrap::EmitDestroy( + Environment::GetCurrent(args), args[0]->NumberValue()); } void AsyncWrap::AddWrapMethods(Environment* env, @@ -605,7 +626,34 @@ AsyncWrap::AsyncWrap(Environment* env, AsyncWrap::~AsyncWrap() { - PushBackDestroyAsyncId(env(), get_async_id()); + EmitTraceEventDestroy(); + EmitDestroy(env(), get_async_id()); +} + +void AsyncWrap::EmitTraceEventDestroy() { + switch (provider_type()) { + #define V(PROVIDER) \ + case PROVIDER_ ## PROVIDER: \ + TRACE_EVENT_NESTABLE_ASYNC_END0("node.async_hooks", \ + #PROVIDER, static_cast(get_async_id())); \ + break; + NODE_ASYNC_PROVIDER_TYPES(V) + #undef V + default: + UNREACHABLE(); + } +} + +void AsyncWrap::EmitDestroy(Environment* env, double async_id) { + if (env->async_hooks()->fields()[AsyncHooks::kDestroy] == 0) + return; + + if (env->destroy_async_id_list()->empty()) { + uv_timer_start(env->destroy_async_ids_timer_handle(), + DestroyAsyncIdsCallback, 0, 0); + } + + env->destroy_async_id_list()->push_back(async_id); } @@ -617,6 +665,19 @@ void AsyncWrap::AsyncReset(double execution_async_id, bool silent) { execution_async_id == -1 ? env()->new_async_id() : execution_async_id; trigger_async_id_ = env()->get_init_trigger_async_id(); + switch (provider_type()) { +#define V(PROVIDER) \ + case PROVIDER_ ## PROVIDER: \ + TRACE_EVENT_NESTABLE_ASYNC_BEGIN1("node.async_hooks", \ + #PROVIDER, static_cast(get_async_id()), \ + "triggerAsyncId", static_cast(get_trigger_async_id())); \ + break; + NODE_ASYNC_PROVIDER_TYPES(V) +#undef V + default: + UNREACHABLE(); + } + if (silent) return; AsyncWrap::EmitAsyncInit( @@ -664,8 +725,15 @@ void AsyncWrap::EmitAsyncInit(Environment* env, MaybeLocal AsyncWrap::MakeCallback(const Local cb, int argc, Local* argv) { + EmitTraceEventBefore(); + async_context context { get_async_id(), get_trigger_async_id() }; - return InternalMakeCallback(env(), object(), cb, argc, argv, context); + MaybeLocal ret = InternalMakeCallback( + env(), object(), cb, argc, argv, context); + + EmitTraceEventAfter(); + + return ret; } @@ -723,8 +791,8 @@ async_context EmitAsyncInit(Isolate* isolate, } void EmitAsyncDestroy(Isolate* isolate, async_context asyncContext) { - PushBackDestroyAsyncId(Environment::GetCurrent(isolate), - asyncContext.async_id); + AsyncWrap::EmitDestroy( + Environment::GetCurrent(isolate), asyncContext.async_id); } } // namespace node diff --git a/src/async-wrap.h b/src/async-wrap.h index 9510e26577357d..480a06e02d45bf 100644 --- a/src/async-wrap.h +++ b/src/async-wrap.h @@ -130,12 +130,18 @@ class AsyncWrap : public BaseObject { static void EmitAsyncInit(Environment* env, v8::Local object, v8::Local type, - double id, + double async_id, double trigger_async_id); - static void EmitBefore(Environment* env, double id); - static void EmitAfter(Environment* env, double id); - static void EmitPromiseResolve(Environment* env, double id); + static void EmitDestroy(Environment* env, double async_id); + static void EmitBefore(Environment* env, double async_id); + static void EmitAfter(Environment* env, double async_id); + static void EmitPromiseResolve(Environment* env, double async_id); + + void EmitTraceEventBefore(); + void EmitTraceEventAfter(); + void EmitTraceEventDestroy(); + inline ProviderType provider_type() const; diff --git a/src/node.cc b/src/node.cc index 946260bf665bde..92648876079f23 100644 --- a/src/node.cc +++ b/src/node.cc @@ -272,13 +272,20 @@ node::DebugOptions debug_options; static struct { #if NODE_USE_V8_PLATFORM void Initialize(int thread_pool_size, uv_loop_t* loop) { - tracing_agent_.reset( - trace_enabled ? new tracing::Agent() : nullptr); - platform_ = new NodePlatform(thread_pool_size, loop, - trace_enabled ? tracing_agent_->GetTracingController() : nullptr); - V8::InitializePlatform(platform_); - tracing::TraceEventHelper::SetTracingController( - trace_enabled ? tracing_agent_->GetTracingController() : nullptr); + if (trace_enabled) { + tracing_agent_.reset(new tracing::Agent()); + platform_ = new NodePlatform(thread_pool_size, loop, + tracing_agent_->GetTracingController()); + V8::InitializePlatform(platform_); + tracing::TraceEventHelper::SetTracingController( + tracing_agent_->GetTracingController()); + } else { + tracing_agent_.reset(nullptr); + platform_ = new NodePlatform(thread_pool_size, loop, nullptr); + V8::InitializePlatform(platform_); + tracing::TraceEventHelper::SetTracingController( + new v8::TracingController()); + } } void Dispose() { diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index aa325277f6c6dd..7bc15e4fa45f60 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -399,6 +399,18 @@ class Parser : public AsyncWrap { } + static void Free(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + Parser* parser; + ASSIGN_OR_RETURN_UNWRAP(&parser, args.Holder()); + + // Since the Parser destructor isn't going to run the destroy() callbacks + // it needs to be triggered manually. + parser->EmitTraceEventDestroy(); + parser->EmitDestroy(env, parser->get_async_id()); + } + + void Save() { url_.Save(); status_message_.Save(); @@ -794,6 +806,7 @@ void InitHttpParser(Local target, AsyncWrap::AddWrapMethods(env, t); env->SetProtoMethod(t, "close", Parser::Close); + env->SetProtoMethod(t, "free", Parser::Free); env->SetProtoMethod(t, "execute", Parser::Execute); env->SetProtoMethod(t, "finish", Parser::Finish); env->SetProtoMethod(t, "reinitialize", Parser::Reinitialize); diff --git a/src/node_trace_events.cc b/src/node_trace_events.cc new file mode 100644 index 00000000000000..20edb66cd66ed4 --- /dev/null +++ b/src/node_trace_events.cc @@ -0,0 +1,136 @@ +#include "node_internals.h" +#include "tracing/agent.h" + +namespace node { + +using v8::Context; +using v8::FunctionCallbackInfo; +using v8::Int32; +using v8::Local; +using v8::Object; +using v8::Value; + +// The tracing APIs require category groups to be pointers to long-lived +// strings. Those strings are stored here. +static std::unordered_set categoryGroups; + +// Gets a pointer to the category-enabled flags for a tracing category group, +// if tracing is enabled for it. +static const uint8_t* GetCategoryGroupEnabled(const char* category_group) { + if (category_group == nullptr) return nullptr; + + return TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(category_group); +} + +static const char* GetCategoryGroup(Environment* env, + const Local categoryValue) { + CHECK(categoryValue->IsString()); + + Utf8Value category(env->isolate(), categoryValue); + // If the value already exists in the set, insertion.first will point + // to the existing value. Thus, this will maintain a long lived pointer + // to the category c-string. + auto insertion = categoryGroups.insert(category.out()); + + // The returned insertion is a pair whose first item is the object that was + // inserted or that blocked the insertion and second item is a boolean + // indicating whether it was inserted. + return insertion.first->c_str(); +} + +static void Emit(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + Local context = env->context(); + + // Args: [type, category, name, id, argName, argValue] + CHECK_GE(args.Length(), 3); + + // Check the category group first, to avoid doing more work if it's not + // enabled. + const char* category_group = GetCategoryGroup(env, args[1]); + const uint8_t* category_group_enabled = + GetCategoryGroupEnabled(category_group); + if (*category_group_enabled == 0) return; + + // get trace_event phase + CHECK(args[0]->IsNumber()); + char phase = static_cast(args[0]->Int32Value(context).ToChecked()); + + // get trace_event name + CHECK(args[2]->IsString()); + Utf8Value nameValue(env->isolate(), args[2]); + const char* name = nameValue.out(); + + // get trace_event id + int64_t id = 0; + bool has_id = false; + if (args.Length() >= 4 && !args[3]->IsUndefined() && !args[3]->IsNull()) { + has_id = true; + CHECK(args[3]->IsNumber()); + id = args[3]->IntegerValue(context).ToChecked(); + } + + // TODO(AndreasMadsen): Two extra arguments are not supported. + // TODO(AndreasMadsen): String values are not supported. + int32_t num_args = 0; + const char* arg_names[1]; + uint8_t arg_types[1]; + uint64_t arg_values[1]; + + // Create Utf8Value in the function scope to prevent deallocation. + // The c-string will be copied by TRACE_EVENT_API_ADD_TRACE_EVENT at the end. + Utf8Value arg1NameValue(env->isolate(), args[4]); + + if (args.Length() < 6 || (args[4]->IsUndefined() && args[5]->IsUndefined())) { + num_args = 0; + } else { + num_args = 1; + arg_types[0] = TRACE_VALUE_TYPE_INT; + + CHECK(args[4]->IsString()); + arg_names[0] = arg1NameValue.out(); + + CHECK(args[5]->IsNumber()); + arg_values[0] = args[5]->IntegerValue(context).ToChecked(); + } + + // The TRACE_EVENT_FLAG_COPY flag indicates that the name and argument + // name should be copied thus they don't need to long-lived pointers. + // The category name still won't be copied and thus need to be a long-lived + // pointer. + uint32_t flags = TRACE_EVENT_FLAG_COPY; + if (has_id) { + flags |= TRACE_EVENT_FLAG_HAS_ID; + } + + const char* scope = node::tracing::kGlobalScope; + uint64_t bind_id = node::tracing::kNoId; + + TRACE_EVENT_API_ADD_TRACE_EVENT( + phase, category_group_enabled, name, scope, id, bind_id, + num_args, arg_names, arg_types, arg_values, + flags); +} + +static void CategoryGroupEnabled(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + + const char* category_group = GetCategoryGroup(env, args[0]); + const uint8_t* category_group_enabled = + GetCategoryGroupEnabled(category_group); + args.GetReturnValue().Set(*category_group_enabled > 0); +} + +void InitializeTraceEvents(Local target, + Local unused, + Local context, + void* priv) { + Environment* env = Environment::GetCurrent(context); + + env->SetMethod(target, "emit", Emit); + env->SetMethod(target, "categoryGroupEnabled", CategoryGroupEnabled); +} + +} // namespace node + +NODE_MODULE_CONTEXT_AWARE_BUILTIN(trace_events, node::InitializeTraceEvents) diff --git a/src/tracing/agent.cc b/src/tracing/agent.cc index bc04b0b5e60d17..4514a0fce1f0a2 100644 --- a/src/tracing/agent.cc +++ b/src/tracing/agent.cc @@ -36,6 +36,7 @@ void Agent::Start(const string& enabled_categories) { } else { trace_config->AddIncludedCategory("v8"); trace_config->AddIncludedCategory("node"); + trace_config->AddIncludedCategory("node.async_hooks"); } // This thread should be created *after* async handles are created diff --git a/test/parallel/test-trace-event.js b/test/parallel/test-trace-event.js deleted file mode 100644 index 434c7db4d263cd..00000000000000 --- a/test/parallel/test-trace-event.js +++ /dev/null @@ -1,41 +0,0 @@ -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const cp = require('child_process'); -const fs = require('fs'); - -const CODE = 'for (var i = 0; i < 100000; i++) { "test" + i }'; -const FILE_NAME = 'node_trace.1.log'; - -common.refreshTmpDir(); -process.chdir(common.tmpDir); - -const proc_no_categories = cp.spawn( - process.execPath, - [ '--trace-events-enabled', '--trace-event-categories', '""', '-e', CODE ] -); - -proc_no_categories.once('exit', common.mustCall(() => { - assert(!common.fileExists(FILE_NAME)); - - const proc = cp.spawn(process.execPath, - [ '--trace-events-enabled', '-e', CODE ]); - - proc.once('exit', common.mustCall(() => { - assert(common.fileExists(FILE_NAME)); - fs.readFile(FILE_NAME, common.mustCall((err, data) => { - const traces = JSON.parse(data.toString()).traceEvents; - assert(traces.length > 0); - // Values that should be present on all runs to approximate correctness. - assert(traces.some((trace) => { - if (trace.pid !== proc.pid) - return false; - if (trace.cat !== 'v8') - return false; - if (trace.name !== 'V8.ScriptCompiler') - return false; - return true; - })); - })); - })); -})); diff --git a/test/parallel/test-trace-events-all.js b/test/parallel/test-trace-events-all.js new file mode 100644 index 00000000000000..329f99f591244d --- /dev/null +++ b/test/parallel/test-trace-events-all.js @@ -0,0 +1,56 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cp = require('child_process'); +const fs = require('fs'); + +const CODE = + 'setTimeout(() => { for (var i = 0; i < 100000; i++) { "test" + i } }, 1)'; +const FILE_NAME = 'node_trace.1.log'; + +common.refreshTmpDir(); +process.chdir(common.tmpDir); + +const proc = cp.spawn(process.execPath, + [ '--trace-events-enabled', '-e', CODE ]); + +proc.once('exit', common.mustCall(() => { + assert(common.fileExists(FILE_NAME)); + fs.readFile(FILE_NAME, common.mustCall((err, data) => { + const traces = JSON.parse(data.toString()).traceEvents; + assert(traces.length > 0); + // V8 trace events should be generated. + assert(traces.some((trace) => { + if (trace.pid !== proc.pid) + return false; + if (trace.cat !== 'v8') + return false; + if (trace.name !== 'V8.ScriptCompiler') + return false; + return true; + })); + + // C++ async_hooks trace events should be generated. + assert(traces.some((trace) => { + if (trace.pid !== proc.pid) + return false; + if (trace.cat !== 'node.async_hooks') + return false; + if (trace.name !== 'TIMERWRAP') + return false; + return true; + })); + + + // JavaScript async_hooks trace events should be generated. + assert(traces.some((trace) => { + if (trace.pid !== proc.pid) + return false; + if (trace.cat !== 'node.async_hooks') + return false; + if (trace.name !== 'Timeout') + return false; + return true; + })); + })); +})); diff --git a/test/parallel/test-trace-events-async-hooks.js b/test/parallel/test-trace-events-async-hooks.js new file mode 100644 index 00000000000000..7f8fb106200b1a --- /dev/null +++ b/test/parallel/test-trace-events-async-hooks.js @@ -0,0 +1,58 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cp = require('child_process'); +const fs = require('fs'); + +const CODE = + 'setTimeout(() => { for (var i = 0; i < 100000; i++) { "test" + i } }, 1)'; +const FILE_NAME = 'node_trace.1.log'; + +common.refreshTmpDir(); +process.chdir(common.tmpDir); + +const proc = cp.spawn(process.execPath, + [ '--trace-events-enabled', + '--trace-event-categories', 'node.async_hooks', + '-e', CODE ]); + +proc.once('exit', common.mustCall(() => { + assert(common.fileExists(FILE_NAME)); + fs.readFile(FILE_NAME, common.mustCall((err, data) => { + const traces = JSON.parse(data.toString()).traceEvents; + assert(traces.length > 0); + // V8 trace events should be generated. + assert(!traces.some((trace) => { + if (trace.pid !== proc.pid) + return false; + if (trace.cat !== 'v8') + return false; + if (trace.name !== 'V8.ScriptCompiler') + return false; + return true; + })); + + // C++ async_hooks trace events should be generated. + assert(traces.some((trace) => { + if (trace.pid !== proc.pid) + return false; + if (trace.cat !== 'node.async_hooks') + return false; + if (trace.name !== 'TIMERWRAP') + return false; + return true; + })); + + + // JavaScript async_hooks trace events should be generated. + assert(traces.some((trace) => { + if (trace.pid !== proc.pid) + return false; + if (trace.cat !== 'node.async_hooks') + return false; + if (trace.name !== 'Timeout') + return false; + return true; + })); + })); +})); diff --git a/test/parallel/test-trace-events-binding.js b/test/parallel/test-trace-events-binding.js new file mode 100644 index 00000000000000..628c9cace71973 --- /dev/null +++ b/test/parallel/test-trace-events-binding.js @@ -0,0 +1,48 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cp = require('child_process'); +const fs = require('fs'); + +const CODE = ` + process.binding("trace_events").emit( + 'b'.charCodeAt(0), 'custom', + 'type-value', 10, 'extra-value', 20); + process.binding("trace_events").emit( + 'b'.charCodeAt(0), 'custom', + 'type-value', 20); + process.binding("trace_events").emit( + 'b'.charCodeAt(0), 'missing', + 'type-value', 10, 'extra-value', 20); +`; +const FILE_NAME = 'node_trace.1.log'; + +common.refreshTmpDir(); +process.chdir(common.tmpDir); + +const proc = cp.spawn(process.execPath, + [ '--trace-events-enabled', + '--trace-event-categories', 'custom', + '-e', CODE ]); + +proc.once('exit', common.mustCall(() => { + assert(common.fileExists(FILE_NAME)); + fs.readFile(FILE_NAME, common.mustCall((err, data) => { + const traces = JSON.parse(data.toString()).traceEvents; + assert.strictEqual(traces.length, 2); + + assert.strictEqual(traces[0].pid, proc.pid); + assert.strictEqual(traces[0].ph, 'b'); + assert.strictEqual(traces[0].cat, 'custom'); + assert.strictEqual(traces[0].name, 'type-value'); + assert.strictEqual(traces[0].id, '0xa'); + assert.deepStrictEqual(traces[0].args, { 'extra-value': 20 }); + + assert.strictEqual(traces[1].pid, proc.pid); + assert.strictEqual(traces[1].ph, 'b'); + assert.strictEqual(traces[1].cat, 'custom'); + assert.strictEqual(traces[1].name, 'type-value'); + assert.strictEqual(traces[1].id, '0x14'); + assert.deepStrictEqual(traces[1].args, { }); + })); +})); diff --git a/test/parallel/test-trace-events-category-used.js b/test/parallel/test-trace-events-category-used.js new file mode 100644 index 00000000000000..39d09ad862d787 --- /dev/null +++ b/test/parallel/test-trace-events-category-used.js @@ -0,0 +1,35 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cp = require('child_process'); + +const CODE = `console.log( + process.binding("trace_events").categoryGroupEnabled("custom") +);`; + +common.refreshTmpDir(); +process.chdir(common.tmpDir); + +const procEnabled = cp.spawn( + process.execPath, + [ '--trace-events-enabled', '--trace-event-categories', 'custom', '-e', CODE ] +); +let procEnabledOutput = ''; + +procEnabled.stdout.on('data', (data) => procEnabledOutput += data); +procEnabled.stderr.pipe(process.stderr); +procEnabled.once('exit', common.mustCall(() => { + assert.strictEqual(procEnabledOutput, 'true\n'); +})); + +const procDisabled = cp.spawn( + process.execPath, + [ '--trace-events-enabled', '--trace-event-categories', 'other', '-e', CODE ] +); +let procDisabledOutput = ''; + +procDisabled.stdout.on('data', (data) => procDisabledOutput += data); +procDisabled.stderr.pipe(process.stderr); +procDisabled.once('exit', common.mustCall(() => { + assert.strictEqual(procDisabledOutput, 'false\n'); +})); diff --git a/test/parallel/test-trace-events-none.js b/test/parallel/test-trace-events-none.js new file mode 100644 index 00000000000000..9a4d587f2db0e1 --- /dev/null +++ b/test/parallel/test-trace-events-none.js @@ -0,0 +1,20 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cp = require('child_process'); + +const CODE = + 'setTimeout(() => { for (var i = 0; i < 100000; i++) { "test" + i } }, 1)'; +const FILE_NAME = 'node_trace.1.log'; + +common.refreshTmpDir(); +process.chdir(common.tmpDir); + +const proc_no_categories = cp.spawn( + process.execPath, + [ '--trace-events-enabled', '--trace-event-categories', '""', '-e', CODE ] +); + +proc_no_categories.once('exit', common.mustCall(() => { + assert(!common.fileExists(FILE_NAME)); +})); diff --git a/test/parallel/test-trace-events-v8.js b/test/parallel/test-trace-events-v8.js new file mode 100644 index 00000000000000..b17b1473ecaf0c --- /dev/null +++ b/test/parallel/test-trace-events-v8.js @@ -0,0 +1,58 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cp = require('child_process'); +const fs = require('fs'); + +const CODE = + 'setTimeout(() => { for (var i = 0; i < 100000; i++) { "test" + i } }, 1)'; +const FILE_NAME = 'node_trace.1.log'; + +common.refreshTmpDir(); +process.chdir(common.tmpDir); + +const proc = cp.spawn(process.execPath, + [ '--trace-events-enabled', + '--trace-event-categories', 'v8', + '-e', CODE ]); + +proc.once('exit', common.mustCall(() => { + assert(common.fileExists(FILE_NAME)); + fs.readFile(FILE_NAME, common.mustCall((err, data) => { + const traces = JSON.parse(data.toString()).traceEvents; + assert(traces.length > 0); + // V8 trace events should be generated. + assert(traces.some((trace) => { + if (trace.pid !== proc.pid) + return false; + if (trace.cat !== 'v8') + return false; + if (trace.name !== 'V8.ScriptCompiler') + return false; + return true; + })); + + // C++ async_hooks trace events should be generated. + assert(!traces.some((trace) => { + if (trace.pid !== proc.pid) + return false; + if (trace.cat !== 'node.async_hooks') + return false; + if (trace.name !== 'TIMERWRAP') + return false; + return true; + })); + + + // JavaScript async_hooks trace events should be generated. + assert(!traces.some((trace) => { + if (trace.pid !== proc.pid) + return false; + if (trace.cat !== 'node.async_hooks') + return false; + if (trace.name !== 'Timeout') + return false; + return true; + })); + })); +})); From 07e7f2ef93734d95ab2cbfb4a57b8e4b4f4132f7 Mon Sep 17 00:00:00 2001 From: Sebastian Mayr Date: Thu, 9 Nov 2017 22:57:04 +0100 Subject: [PATCH 04/22] async_hooks: add destroy event for gced AsyncResources 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 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell --- benchmark/async_hooks/gc-tracking.js | 45 +++++++++++++++++++ doc/api/async_hooks.md | 18 +++++--- lib/async_hooks.js | 21 ++++++++- src/async-wrap.cc | 41 +++++++++++++++++ src/async-wrap.h | 3 ++ src/env.h | 1 + .../test-embedder.api.async-resource.js | 2 +- .../test-async-hooks-destroy-on-gc.js | 27 +++++++++++ .../test-async-hooks-disable-gc-tracking.js | 21 +++++++++ ...test-async-hooks-prevent-double-destroy.js | 24 ++++++++++ 10 files changed, 196 insertions(+), 7 deletions(-) create mode 100644 benchmark/async_hooks/gc-tracking.js create mode 100644 test/parallel/test-async-hooks-destroy-on-gc.js create mode 100644 test/parallel/test-async-hooks-disable-gc-tracking.js create mode 100644 test/parallel/test-async-hooks-prevent-double-destroy.js diff --git a/benchmark/async_hooks/gc-tracking.js b/benchmark/async_hooks/gc-tracking.js new file mode 100644 index 00000000000000..c71c1b07aa5431 --- /dev/null +++ b/benchmark/async_hooks/gc-tracking.js @@ -0,0 +1,45 @@ +'use strict'; +const common = require('../common.js'); +const { AsyncResource } = require('async_hooks'); + +const bench = common.createBenchmark(main, { + n: [1e6], + method: [ + 'trackingEnabled', + 'trackingDisabled', + ] +}, { + flags: ['--expose-gc'] +}); + +function endAfterGC(n) { + setImmediate(() => { + global.gc(); + setImmediate(() => { + bench.end(n); + }); + }); +} + +function main(conf) { + const n = +conf.n; + + switch (conf.method) { + case 'trackingEnabled': + bench.start(); + for (let i = 0; i < n; i++) { + new AsyncResource('foobar'); + } + endAfterGC(n); + break; + case 'trackingDisabled': + bench.start(); + for (let i = 0; i < n; i++) { + new AsyncResource('foobar', { requireManualDestroy: true }); + } + endAfterGC(n); + break; + default: + throw new Error('Unsupported method'); + } +} diff --git a/doc/api/async_hooks.md b/doc/api/async_hooks.md index 000982eb32af8d..41352820816b3f 100644 --- a/doc/api/async_hooks.md +++ b/doc/api/async_hooks.md @@ -545,12 +545,14 @@ will occur and the process will abort. The following is an overview of the `AsyncResource` API. ```js -const { AsyncResource } = require('async_hooks'); +const { AsyncResource, executionAsyncId } = require('async_hooks'); // AsyncResource() is meant to be extended. Instantiating a // new AsyncResource() also triggers init. If triggerAsyncId is omitted then // async_hook.executionAsyncId() is used. -const asyncResource = new AsyncResource(type, triggerAsyncId); +const asyncResource = new AsyncResource( + type, { triggerAsyncId: executionAsyncId(), requireManualDestroy: false } +); // Call AsyncHooks before callbacks. asyncResource.emitBefore(); @@ -568,11 +570,17 @@ asyncResource.asyncId(); asyncResource.triggerAsyncId(); ``` -#### `AsyncResource(type[, triggerAsyncId])` +#### `AsyncResource(type[, options])` * `type` {string} The type of async event. -* `triggerAsyncId` {number} The ID of the execution context that created this - async event. +* `options` {Object} + * `triggerAsyncId` {number} The ID of the execution context that created this + async event. **Default:** `executionAsyncId()` + * `requireManualDestroy` {boolean} Disables automatic `emitDestroy` when the + object is garbage collected. This usually does not need to be set (even if + `emitDestroy` is called manually), unless the resource's asyncId is retrieved + and the sensitive API's `emitDestroy` is called with it. + **Default:** `false` Example usage: diff --git a/lib/async_hooks.js b/lib/async_hooks.js index afd3f5d85fea58..b642ec1a82f094 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -30,6 +30,9 @@ const { async_hook_fields, async_id_fields } = async_wrap; const { pushAsyncIds, popAsyncIds } = async_wrap; // For performance reasons, only track Proimses when a hook is enabled. const { enablePromiseHook, disablePromiseHook } = async_wrap; +// For userland AsyncResources, make sure to emit a destroy event when the +// resource gets gced. +const { registerDestroyHook } = async_wrap; // Properties in active_hooks are used to keep track of the set of hooks being // executed in case another hook is enabled/disabled. The new set of hooks is // then restored once the active set of hooks is finished executing. @@ -260,13 +263,22 @@ function validateAsyncId(asyncId, type) { // Embedder API // +const destroyedSymbol = Symbol('destroyed'); + class AsyncResource { - constructor(type, triggerAsyncId = initTriggerId()) { + constructor(type, opts = {}) { if (typeof type !== 'string') throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'type', 'string'); + if (typeof opts === 'number') { + opts = { triggerAsyncId: opts, requireManualDestroy: false }; + } else if (opts.triggerAsyncId === undefined) { + opts.triggerAsyncId = initTriggerId(); + } + // Unlike emitInitScript, AsyncResource doesn't supports null as the // triggerAsyncId. + const triggerAsyncId = opts.triggerAsyncId; if (!Number.isSafeInteger(triggerAsyncId) || triggerAsyncId < -1) { throw new errors.RangeError('ERR_INVALID_ASYNC_ID', 'triggerAsyncId', @@ -275,10 +287,16 @@ class AsyncResource { this[async_id_symbol] = ++async_id_fields[kAsyncIdCounter]; this[trigger_async_id_symbol] = triggerAsyncId; + // this prop name (destroyed) has to be synchronized with C++ + this[destroyedSymbol] = { destroyed: false }; emitInitScript( this[async_id_symbol], type, this[trigger_async_id_symbol], this ); + + if (!opts.requireManualDestroy) { + registerDestroyHook(this, this[async_id_symbol], this[destroyedSymbol]); + } } emitBefore() { @@ -292,6 +310,7 @@ class AsyncResource { } emitDestroy() { + this[destroyedSymbol].destroyed = true; emitDestroyScript(this[async_id_symbol]); return this; } diff --git a/src/async-wrap.cc b/src/async-wrap.cc index 6281a204accd4a..a3ef0d07e8d896 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -428,6 +428,46 @@ static void DisablePromiseHook(const FunctionCallbackInfo& args) { } +class DestroyParam { + public: + double asyncId; + v8::Persistent target; + v8::Persistent propBag; +}; + + +void AsyncWrap::WeakCallback(const v8::WeakCallbackInfo& info) { + HandleScope scope(info.GetIsolate()); + + Environment* env = Environment::GetCurrent(info.GetIsolate()); + DestroyParam* p = info.GetParameter(); + Local prop_bag = PersistentToLocal(info.GetIsolate(), p->propBag); + + Local val = prop_bag->Get(env->destroyed_string()); + if (val->IsFalse()) { + AsyncWrap::EmitDestroy(env, p->asyncId); + } + p->target.Reset(); + p->propBag.Reset(); + delete p; +} + + +static void RegisterDestroyHook(const FunctionCallbackInfo& args) { + CHECK(args[0]->IsObject()); + CHECK(args[1]->IsNumber()); + CHECK(args[2]->IsObject()); + + Isolate* isolate = args.GetIsolate(); + DestroyParam* p = new DestroyParam(); + p->asyncId = args[1].As()->Value(); + p->target.Reset(isolate, args[0].As()); + p->propBag.Reset(isolate, args[2].As()); + p->target.SetWeak( + p, AsyncWrap::WeakCallback, v8::WeakCallbackType::kParameter); +} + + void AsyncWrap::GetAsyncId(const FunctionCallbackInfo& args) { AsyncWrap* wrap; args.GetReturnValue().Set(-1); @@ -503,6 +543,7 @@ void AsyncWrap::Initialize(Local target, env->SetMethod(target, "queueDestroyAsyncId", QueueDestroyAsyncId); env->SetMethod(target, "enablePromiseHook", EnablePromiseHook); env->SetMethod(target, "disablePromiseHook", DisablePromiseHook); + env->SetMethod(target, "registerDestroyHook", RegisterDestroyHook); v8::PropertyAttribute ReadOnlyDontDelete = static_cast(v8::ReadOnly | v8::DontDelete); diff --git a/src/async-wrap.h b/src/async-wrap.h index 480a06e02d45bf..e6e717c24ed71d 100644 --- a/src/async-wrap.h +++ b/src/async-wrap.h @@ -86,6 +86,7 @@ namespace node { NODE_ASYNC_INSPECTOR_PROVIDER_TYPES(V) class Environment; +class DestroyParam; class AsyncWrap : public BaseObject { public: @@ -165,6 +166,8 @@ class AsyncWrap : public BaseObject { virtual size_t self_size() const = 0; + static void WeakCallback(const v8::WeakCallbackInfo &info); + private: friend class PromiseWrap; diff --git a/src/env.h b/src/env.h index e26efd8f47bd39..93f18e8cbc5a61 100644 --- a/src/env.h +++ b/src/env.h @@ -120,6 +120,7 @@ class ModuleWrap; V(cwd_string, "cwd") \ V(dest_string, "dest") \ V(destroy_string, "destroy") \ + V(destroyed_string, "destroyed") \ V(detached_string, "detached") \ V(disposed_string, "_disposed") \ V(dns_a_string, "A") \ diff --git a/test/async-hooks/test-embedder.api.async-resource.js b/test/async-hooks/test-embedder.api.async-resource.js index f4dfba89557871..eeeaa447c9668c 100644 --- a/test/async-hooks/test-embedder.api.async-resource.js +++ b/test/async-hooks/test-embedder.api.async-resource.js @@ -18,7 +18,7 @@ common.expectsError( type: TypeError, }); assert.throws(() => { - new AsyncResource('invalid_trigger_id', null); + new AsyncResource('invalid_trigger_id', { triggerAsyncId: null }); }, common.expectsError({ code: 'ERR_INVALID_ASYNC_ID', type: RangeError, diff --git a/test/parallel/test-async-hooks-destroy-on-gc.js b/test/parallel/test-async-hooks-destroy-on-gc.js new file mode 100644 index 00000000000000..fe6325e189734b --- /dev/null +++ b/test/parallel/test-async-hooks-destroy-on-gc.js @@ -0,0 +1,27 @@ +'use strict'; +// Flags: --expose_gc + +// This test ensures that userland-only AsyncResources cause a destroy event to +// be emitted when they get gced. + +const common = require('../common'); +const assert = require('assert'); +const async_hooks = require('async_hooks'); + +const destroyedIds = new Set(); +async_hooks.createHook({ + destroy: common.mustCallAtLeast((asyncId) => { + destroyedIds.add(asyncId); + }, 1) +}).enable(); + +let asyncId = null; +{ + const res = new async_hooks.AsyncResource('foobar'); + asyncId = res.asyncId(); +} + +setImmediate(() => { + global.gc(); + setImmediate(() => assert.ok(destroyedIds.has(asyncId))); +}); diff --git a/test/parallel/test-async-hooks-disable-gc-tracking.js b/test/parallel/test-async-hooks-disable-gc-tracking.js new file mode 100644 index 00000000000000..a34739a9bb2b95 --- /dev/null +++ b/test/parallel/test-async-hooks-disable-gc-tracking.js @@ -0,0 +1,21 @@ +'use strict'; +// Flags: --expose_gc + +// This test ensures that userland-only AsyncResources cause a destroy event to +// be emitted when they get gced. + +const common = require('../common'); +const async_hooks = require('async_hooks'); + +const hook = async_hooks.createHook({ + destroy: common.mustCall(1) // only 1 immediate is destroyed +}).enable(); + +new async_hooks.AsyncResource('foobar', { requireManualDestroy: true }); + +setImmediate(() => { + global.gc(); + setImmediate(() => { + hook.disable(); + }); +}); diff --git a/test/parallel/test-async-hooks-prevent-double-destroy.js b/test/parallel/test-async-hooks-prevent-double-destroy.js new file mode 100644 index 00000000000000..5cd9c5e9a017cb --- /dev/null +++ b/test/parallel/test-async-hooks-prevent-double-destroy.js @@ -0,0 +1,24 @@ +'use strict'; +// Flags: --expose_gc + +// This test ensures that userland-only AsyncResources cause a destroy event to +// be emitted when they get gced. + +const common = require('../common'); +const async_hooks = require('async_hooks'); + +const hook = async_hooks.createHook({ + destroy: common.mustCall(2) // 1 immediate + manual destroy +}).enable(); + +{ + const res = new async_hooks.AsyncResource('foobar'); + res.emitDestroy(); +} + +setImmediate(() => { + global.gc(); + setImmediate(() => { + hook.disable(); + }); +}); From 1a66f91fa1adab3eaefaae8ab64e69abd915be57 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 16 Nov 2017 15:15:05 +0100 Subject: [PATCH 05/22] src: use NODE_BUILTIN_MODULE_CONTEXT_AWARE() macro Commit d217b2850e ("async_hooks: add trace events to async_hooks") used `NODE_MODULE_CONTEXT_AWARE_BUILTIN()` instead. After commit 8680bb9f1a ("src: explicitly register built-in modules") it no longer works for static library builds so remove it. PR-URL: https://github.com/nodejs/node/pull/17071 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Timothy Gu --- src/node.h | 8 -------- src/node_internals.h | 7 +++++++ src/node_trace_events.cc | 2 +- test/cctest/node_module_reg.cc | 1 + 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/node.h b/src/node.h index 76cab566eab258..a48c9bc86f6904 100644 --- a/src/node.h +++ b/src/node.h @@ -425,10 +425,6 @@ typedef void (*addon_context_register_func)( v8::Local context, void* priv); -#define NM_F_BUILTIN 0x01 -#define NM_F_LINKED 0x02 -#define NM_F_INTERNAL 0x04 - struct node_module { int nm_version; unsigned int nm_flags; @@ -513,10 +509,6 @@ extern "C" NODE_EXTERN void node_module_register(void* mod); /* NOLINTNEXTLINE (readability/null_usage) */ \ NODE_MODULE_CONTEXT_AWARE_X(modname, regfunc, NULL, 0) -#define NODE_MODULE_CONTEXT_AWARE_BUILTIN(modname, regfunc) \ - /* NOLINTNEXTLINE (readability/null_usage) */ \ - NODE_MODULE_CONTEXT_AWARE_X(modname, regfunc, NULL, NM_F_BUILTIN) \ - /* * For backward compatibility in add-on modules. */ diff --git a/src/node_internals.h b/src/node_internals.h index 55798d2e7a87f6..53c1ea7501d732 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -38,6 +38,12 @@ #include +enum { + NM_F_BUILTIN = 1 << 0, + NM_F_LINKED = 1 << 1, + NM_F_INTERNAL = 1 << 2, +}; + struct sockaddr; // Variation on NODE_DEFINE_CONSTANT that sets a String value. @@ -98,6 +104,7 @@ struct sockaddr; V(stream_wrap) \ V(tcp_wrap) \ V(timer_wrap) \ + V(trace_events) \ V(tty_wrap) \ V(udp_wrap) \ V(url) \ diff --git a/src/node_trace_events.cc b/src/node_trace_events.cc index 20edb66cd66ed4..b0ffe68eae3f47 100644 --- a/src/node_trace_events.cc +++ b/src/node_trace_events.cc @@ -133,4 +133,4 @@ void InitializeTraceEvents(Local target, } // namespace node -NODE_MODULE_CONTEXT_AWARE_BUILTIN(trace_events, node::InitializeTraceEvents) +NODE_BUILTIN_MODULE_CONTEXT_AWARE(trace_events, node::InitializeTraceEvents) diff --git a/test/cctest/node_module_reg.cc b/test/cctest/node_module_reg.cc index f8d9d03c1cdb99..a0736d2cc3e692 100644 --- a/test/cctest/node_module_reg.cc +++ b/test/cctest/node_module_reg.cc @@ -20,6 +20,7 @@ void _register_spawn_sync() {} void _register_stream_wrap() {} void _register_tcp_wrap() {} void _register_timer_wrap() {} +void _register_trace_events() {} void _register_tty_wrap() {} void _register_udp_wrap() {} void _register_util() {} From b7e3109354a4ee2721cb39971f81a26a05063a88 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Tue, 14 Nov 2017 13:34:52 +0100 Subject: [PATCH 06/22] src: rename async-wrap -> async_wrap This commit renames async-wrap to async_wrap for consitency with other c++ source files. PR-URL: https://github.com/nodejs/node/pull/17022 Reviewed-By: James M Snell Reviewed-By: Refael Ackermann Reviewed-By: Anatoli Papirovski --- node.gyp | 8 ++++---- src/{async-wrap-inl.h => async_wrap-inl.h} | 2 +- src/{async-wrap.cc => async_wrap.cc} | 2 +- src/{async-wrap.h => async_wrap.h} | 0 src/cares_wrap.cc | 2 +- src/connect_wrap.h | 2 +- src/env.cc | 2 +- src/fs_event_wrap.cc | 2 +- src/handle_wrap.cc | 2 +- src/handle_wrap.h | 2 +- src/js_stream.cc | 2 +- src/js_stream.h | 2 +- src/node.cc | 2 +- src/node_crypto.cc | 2 +- src/node_crypto.h | 2 +- src/node_http_parser.cc | 2 +- src/node_stat_watcher.cc | 2 +- src/node_stat_watcher.h | 2 +- src/node_zlib.cc | 2 +- src/pipe_wrap.cc | 2 +- src/pipe_wrap.h | 2 +- src/req-wrap-inl.h | 2 +- src/req-wrap.h | 2 +- src/signal_wrap.cc | 2 +- src/stream_base.h | 2 +- src/tcp_wrap.h | 2 +- src/timer_wrap.cc | 2 +- src/tls_wrap.cc | 2 +- src/tls_wrap.h | 2 +- src/udp_wrap.h | 2 +- 30 files changed, 32 insertions(+), 32 deletions(-) rename src/{async-wrap-inl.h => async_wrap-inl.h} (98%) rename src/{async-wrap.cc => async_wrap.cc} (99%) rename src/{async-wrap.h => async_wrap.h} (100%) diff --git a/node.gyp b/node.gyp index f268c611bb2744..4ec7da6c2f1126 100644 --- a/node.gyp +++ b/node.gyp @@ -177,7 +177,7 @@ ], 'sources': [ - 'src/async-wrap.cc', + 'src/async_wrap.cc', 'src/cares_wrap.cc', 'src/connection_wrap.cc', 'src/connect_wrap.cc', @@ -231,8 +231,8 @@ 'src/uv.cc', # headers to make for a more pleasant IDE experience 'src/aliased_buffer.h', - 'src/async-wrap.h', - 'src/async-wrap-inl.h', + 'src/async_wrap.h', + 'src/async_wrap-inl.h', 'src/base-object.h', 'src/base-object-inl.h', 'src/connection_wrap.h', @@ -816,7 +816,7 @@ 'conditions': [ ['node_target_type!="static_library"', { 'libraries': [ - '<(OBJ_PATH)<(OBJ_SEPARATOR)async-wrap.<(OBJ_SUFFIX)', + '<(OBJ_PATH)<(OBJ_SEPARATOR)async_wrap.<(OBJ_SUFFIX)', '<(OBJ_PATH)<(OBJ_SEPARATOR)env.<(OBJ_SUFFIX)', '<(OBJ_PATH)<(OBJ_SEPARATOR)node.<(OBJ_SUFFIX)', '<(OBJ_PATH)<(OBJ_SEPARATOR)node_buffer.<(OBJ_SUFFIX)', diff --git a/src/async-wrap-inl.h b/src/async_wrap-inl.h similarity index 98% rename from src/async-wrap-inl.h rename to src/async_wrap-inl.h index 617d51dc59f037..219dfa71b6cdf0 100644 --- a/src/async-wrap-inl.h +++ b/src/async_wrap-inl.h @@ -24,7 +24,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include "async-wrap.h" +#include "async_wrap.h" #include "base-object-inl.h" #include "node_internals.h" diff --git a/src/async-wrap.cc b/src/async_wrap.cc similarity index 99% rename from src/async-wrap.cc rename to src/async_wrap.cc index a3ef0d07e8d896..dc5f1477e21961 100644 --- a/src/async-wrap.cc +++ b/src/async_wrap.cc @@ -19,7 +19,7 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE // USE OR OTHER DEALINGS IN THE SOFTWARE. -#include "async-wrap-inl.h" +#include "async_wrap-inl.h" #include "env.h" #include "env-inl.h" #include "util-inl.h" diff --git a/src/async-wrap.h b/src/async_wrap.h similarity index 100% rename from src/async-wrap.h rename to src/async_wrap.h diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index 1459dfafab030c..f064380ff3f6b9 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -21,7 +21,7 @@ #define CARES_STATICLIB #include "ares.h" -#include "async-wrap-inl.h" +#include "async_wrap-inl.h" #include "env.h" #include "env-inl.h" #include "node.h" diff --git a/src/connect_wrap.h b/src/connect_wrap.h index 7b16a5448745aa..59918078960016 100644 --- a/src/connect_wrap.h +++ b/src/connect_wrap.h @@ -5,7 +5,7 @@ #include "env.h" #include "req-wrap.h" -#include "async-wrap.h" +#include "async_wrap.h" #include "v8.h" namespace node { diff --git a/src/env.cc b/src/env.cc index 56166042903d06..c46f02dd0ec52f 100644 --- a/src/env.cc +++ b/src/env.cc @@ -1,5 +1,5 @@ #include "node_internals.h" -#include "async-wrap.h" +#include "async_wrap.h" #include "v8-profiler.h" #include diff --git a/src/fs_event_wrap.cc b/src/fs_event_wrap.cc index a7af5293bbfc72..e4165e5ac69bfd 100644 --- a/src/fs_event_wrap.cc +++ b/src/fs_event_wrap.cc @@ -19,7 +19,7 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE // USE OR OTHER DEALINGS IN THE SOFTWARE. -#include "async-wrap-inl.h" +#include "async_wrap-inl.h" #include "env.h" #include "env-inl.h" #include "util-inl.h" diff --git a/src/handle_wrap.cc b/src/handle_wrap.cc index bc4c222deaf5d0..d1f7cb8227939a 100644 --- a/src/handle_wrap.cc +++ b/src/handle_wrap.cc @@ -20,7 +20,7 @@ // USE OR OTHER DEALINGS IN THE SOFTWARE. #include "handle_wrap.h" -#include "async-wrap-inl.h" +#include "async_wrap-inl.h" #include "env.h" #include "env-inl.h" #include "util-inl.h" diff --git a/src/handle_wrap.h b/src/handle_wrap.h index f8be356e1a730c..29e4f364603bda 100644 --- a/src/handle_wrap.h +++ b/src/handle_wrap.h @@ -24,7 +24,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include "async-wrap.h" +#include "async_wrap.h" #include "util.h" #include "uv.h" #include "v8.h" diff --git a/src/js_stream.cc b/src/js_stream.cc index 842fc3b711bd51..7d4ad7a4e978a6 100644 --- a/src/js_stream.cc +++ b/src/js_stream.cc @@ -1,6 +1,6 @@ #include "js_stream.h" -#include "async-wrap.h" +#include "async_wrap.h" #include "env-inl.h" #include "node_buffer.h" #include "stream_base-inl.h" diff --git a/src/js_stream.h b/src/js_stream.h index a4a67ae3372620..44bf7a06df7f8f 100644 --- a/src/js_stream.h +++ b/src/js_stream.h @@ -3,7 +3,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include "async-wrap.h" +#include "async_wrap.h" #include "env.h" #include "stream_base.h" #include "v8.h" diff --git a/src/node.cc b/src/node.cc index 92648876079f23..261303de6f19ca 100644 --- a/src/node.cc +++ b/src/node.cc @@ -54,7 +54,7 @@ #endif #include "ares.h" -#include "async-wrap-inl.h" +#include "async_wrap-inl.h" #include "env.h" #include "env-inl.h" #include "handle_wrap.h" diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 7255fa32dbcdde..773b95d5033662 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -28,7 +28,7 @@ #include "node_mutex.h" #include "tls_wrap.h" // TLSWrap -#include "async-wrap-inl.h" +#include "async_wrap-inl.h" #include "env.h" #include "env-inl.h" #include "string_bytes.h" diff --git a/src/node_crypto.h b/src/node_crypto.h index a155411aa8195c..61d313d7680a47 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -31,7 +31,7 @@ #include "node_buffer.h" #include "env.h" -#include "async-wrap-inl.h" +#include "async_wrap-inl.h" #include "base-object-inl.h" #include "v8.h" diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index 7bc15e4fa45f60..31dd013736e42b 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -22,7 +22,7 @@ #include "node.h" #include "node_buffer.h" -#include "async-wrap-inl.h" +#include "async_wrap-inl.h" #include "env.h" #include "env-inl.h" #include "http_parser.h" diff --git a/src/node_stat_watcher.cc b/src/node_stat_watcher.cc index 46414bcbfd7a2c..96f836a3c8f24e 100644 --- a/src/node_stat_watcher.cc +++ b/src/node_stat_watcher.cc @@ -20,7 +20,7 @@ // USE OR OTHER DEALINGS IN THE SOFTWARE. #include "node_stat_watcher.h" -#include "async-wrap-inl.h" +#include "async_wrap-inl.h" #include "env.h" #include "env-inl.h" #include "util-inl.h" diff --git a/src/node_stat_watcher.h b/src/node_stat_watcher.h index df15f339d128a2..55f4307fdbcd8c 100644 --- a/src/node_stat_watcher.h +++ b/src/node_stat_watcher.h @@ -25,7 +25,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #include "node.h" -#include "async-wrap.h" +#include "async_wrap.h" #include "env.h" #include "uv.h" #include "v8.h" diff --git a/src/node_zlib.cc b/src/node_zlib.cc index 0accd6441a5889..eafdea4379d694 100644 --- a/src/node_zlib.cc +++ b/src/node_zlib.cc @@ -22,7 +22,7 @@ #include "node.h" #include "node_buffer.h" -#include "async-wrap-inl.h" +#include "async_wrap-inl.h" #include "env.h" #include "env-inl.h" #include "util-inl.h" diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc index bb5a6d27dd2316..4cdf7dc6e6b5c6 100644 --- a/src/pipe_wrap.cc +++ b/src/pipe_wrap.cc @@ -21,7 +21,7 @@ #include "pipe_wrap.h" -#include "async-wrap.h" +#include "async_wrap.h" #include "connection_wrap.h" #include "env-inl.h" #include "handle_wrap.h" diff --git a/src/pipe_wrap.h b/src/pipe_wrap.h index 6db7f4561cb522..6f22038b918f76 100644 --- a/src/pipe_wrap.h +++ b/src/pipe_wrap.h @@ -24,7 +24,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include "async-wrap.h" +#include "async_wrap.h" #include "connection_wrap.h" #include "env.h" diff --git a/src/req-wrap-inl.h b/src/req-wrap-inl.h index 045cb298e46344..2793743ac7935a 100644 --- a/src/req-wrap-inl.h +++ b/src/req-wrap-inl.h @@ -4,7 +4,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #include "req-wrap.h" -#include "async-wrap-inl.h" +#include "async_wrap-inl.h" #include "env.h" #include "env-inl.h" #include "util-inl.h" diff --git a/src/req-wrap.h b/src/req-wrap.h index 0fddae67460d6f..83baf9d2a35285 100644 --- a/src/req-wrap.h +++ b/src/req-wrap.h @@ -3,7 +3,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include "async-wrap.h" +#include "async_wrap.h" #include "env.h" #include "util.h" #include "v8.h" diff --git a/src/signal_wrap.cc b/src/signal_wrap.cc index 66ec8d69faa2e1..1044d2d5f038a7 100644 --- a/src/signal_wrap.cc +++ b/src/signal_wrap.cc @@ -19,7 +19,7 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE // USE OR OTHER DEALINGS IN THE SOFTWARE. -#include "async-wrap-inl.h" +#include "async_wrap-inl.h" #include "env.h" #include "env-inl.h" #include "handle_wrap.h" diff --git a/src/stream_base.h b/src/stream_base.h index 20cb0155c9d7b9..94e4bfd73961da 100644 --- a/src/stream_base.h +++ b/src/stream_base.h @@ -4,7 +4,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #include "env.h" -#include "async-wrap.h" +#include "async_wrap.h" #include "req-wrap-inl.h" #include "node.h" #include "util.h" diff --git a/src/tcp_wrap.h b/src/tcp_wrap.h index 95c0b1c1e5b99e..fa6bac01386256 100644 --- a/src/tcp_wrap.h +++ b/src/tcp_wrap.h @@ -24,7 +24,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include "async-wrap.h" +#include "async_wrap.h" #include "env.h" #include "connection_wrap.h" diff --git a/src/timer_wrap.cc b/src/timer_wrap.cc index dc4c2ea5c3def3..423fe1396266d4 100644 --- a/src/timer_wrap.cc +++ b/src/timer_wrap.cc @@ -19,7 +19,7 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE // USE OR OTHER DEALINGS IN THE SOFTWARE. -#include "async-wrap-inl.h" +#include "async_wrap-inl.h" #include "env.h" #include "env-inl.h" #include "handle_wrap.h" diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index d5b56ca9ba3dd7..3b899ea12d501d 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -20,7 +20,7 @@ // USE OR OTHER DEALINGS IN THE SOFTWARE. #include "tls_wrap.h" -#include "async-wrap-inl.h" +#include "async_wrap-inl.h" #include "node_buffer.h" // Buffer #include "node_crypto.h" // SecureContext #include "node_crypto_bio.h" // NodeBIO diff --git a/src/tls_wrap.h b/src/tls_wrap.h index 99d2dc9121f139..b782e7c3c23178 100644 --- a/src/tls_wrap.h +++ b/src/tls_wrap.h @@ -27,7 +27,7 @@ #include "node.h" #include "node_crypto.h" // SSLWrap -#include "async-wrap.h" +#include "async_wrap.h" #include "env.h" #include "stream_wrap.h" #include "util.h" diff --git a/src/udp_wrap.h b/src/udp_wrap.h index fe9256bcc63a05..50a7455beb6a9e 100644 --- a/src/udp_wrap.h +++ b/src/udp_wrap.h @@ -24,7 +24,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include "async-wrap.h" +#include "async_wrap.h" #include "env.h" #include "handle_wrap.h" #include "req-wrap-inl.h" From 8c83406fc1c306c87589f3ab2feb84269f2fb6ca Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 16 Nov 2017 00:43:12 +0100 Subject: [PATCH 07/22] timers: cross JS/C++ border less frequently This removes the `process._needImmediateCallback` property and its semantics of having a 1/0 switch that tells C++ whether immediates are currently scheduled. Instead, a counter keeping track of all immediates is created, that can be increased on `setImmediate()` or decreased when an immediate is run or cleared. This is faster, because rather than reading/writing a C++ getter, this operation can be performed as a direct memory read/write via a typed array. The only C++ call that is left to make is activating the native handles upon creation of the first `Immediate` after the queue is empty. One other (good!) side-effect is that `immediate._destroyed` now reliably tells whether an `immediate` is still scheduled to run or not. Also, as a nice extra, this should make it easier to implement an internal variant of `setImmediate` for C++ that piggybacks off the same mechanism, which should be useful at least for async hooks and HTTP/2. Benchmark results: $ ./node benchmark/compare.js --new ./node --old ./node-master-1b093cb93df0 --runs 10 --filter immediate timers | Rscript benchmark/compare.R [00:08:53|% 100| 4/4 files | 20/20 runs | 1/1 configs]: Done improvement confidence p.value timers/immediate.js type="breadth" thousands=2000 25.61 % ** 1.432301e-03 timers/immediate.js type="breadth1" thousands=2000 7.66 % 1.320233e-01 timers/immediate.js type="breadth4" thousands=2000 4.61 % 5.669053e-01 timers/immediate.js type="clear" thousands=2000 311.40 % *** 3.896291e-07 timers/immediate.js type="depth" thousands=2000 17.54 % ** 9.755389e-03 timers/immediate.js type="depth1" thousands=2000 17.09 % *** 7.176229e-04 timers/set-immediate-breadth-args.js millions=5 10.63 % * 4.250034e-02 timers/set-immediate-breadth.js millions=10 20.62 % *** 9.150439e-07 timers/set-immediate-depth-args.js millions=10 17.97 % *** 6.819135e-10 PR-URL: https://github.com/nodejs/node/pull/17064 Reviewed-By: Refael Ackermann Reviewed-By: Minwoo Jung Reviewed-By: James M Snell Reviewed-By: Ben Noordhuis --- lib/timers.js | 46 ++++++++++++++------------- src/env-inl.h | 6 ++++ src/env.h | 4 +++ src/node.cc | 86 +++++++++++++++++++++------------------------------ 4 files changed, 69 insertions(+), 73 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index 8e45eeb9d81e0d..c7237a25353e5f 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -46,6 +46,13 @@ const { kInit, kDestroy, kAsyncIdCounter } = async_wrap.constants; const async_id_symbol = Symbol('asyncId'); const trigger_async_id_symbol = Symbol('triggerAsyncId'); +/* This is an Uint32Array for easier sharing with C++ land. */ +const scheduledImmediateCount = process._scheduledImmediateCount; +delete process._scheduledImmediateCount; +/* Kick off setImmediate processing */ +const activateImmediateCheck = process._activateImmediateCheck; +delete process._activateImmediateCheck; + // Timeout values > TIMEOUT_MAX are set to 1. const TIMEOUT_MAX = 2147483647; // 2^31-1 @@ -731,15 +738,9 @@ function processImmediate() { else immediate = next; } - - // Only round-trip to C++ land if we have to. Calling clearImmediate() on an - // immediate that's in |queue| is okay. Worst case is we make a superfluous - // call to NeedImmediateCallbackSetter(). - if (!immediateQueue.head) { - process._needImmediateCallback = false; - } } +process._immediateCallback = processImmediate; // An optimization so that the try/finally only de-optimizes (since at least v8 // 4.7) what is in this smaller function. @@ -751,13 +752,17 @@ function tryOnImmediate(immediate, oldTail) { runCallback(immediate); threw = false; } finally { - // clearImmediate checks _callback === null for kDestroy hooks. immediate._callback = null; if (!threw) emitAfter(immediate[async_id_symbol]); - if (async_hook_fields[kDestroy] > 0 && !immediate._destroyed) { - emitDestroy(immediate[async_id_symbol]); + + if (!immediate._destroyed) { immediate._destroyed = true; + scheduledImmediateCount[0]--; + + if (async_hook_fields[kDestroy] > 0) { + emitDestroy(immediate[async_id_symbol]); + } } if (threw && immediate._idleNext) { @@ -860,10 +865,9 @@ function createImmediate(args, callback) { immediate._argv = args; immediate._onImmediate = callback; - if (!process._needImmediateCallback) { - process._needImmediateCallback = true; - process._immediateCallback = processImmediate; - } + if (scheduledImmediateCount[0] === 0) + activateImmediateCheck(); + scheduledImmediateCount[0]++; immediateQueue.append(immediate); @@ -874,18 +878,16 @@ function createImmediate(args, callback) { exports.clearImmediate = function(immediate) { if (!immediate) return; - if (async_hook_fields[kDestroy] > 0 && - immediate._callback !== null && - !immediate._destroyed) { - emitDestroy(immediate[async_id_symbol]); + if (!immediate._destroyed) { + scheduledImmediateCount[0]--; immediate._destroyed = true; + + if (async_hook_fields[kDestroy] > 0) { + emitDestroy(immediate[async_id_symbol]); + } } immediate._onImmediate = null; immediateQueue.remove(immediate); - - if (!immediateQueue.head) { - process._needImmediateCallback = false; - } }; diff --git a/src/env-inl.h b/src/env-inl.h index 6288f45345d0a4..66c677dd6ebccf 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -304,6 +304,7 @@ inline Environment::Environment(IsolateData* isolate_data, abort_on_uncaught_exception_(false), emit_napi_warning_(true), makecallback_cntr_(0), + scheduled_immediate_count_(isolate_, 1), #if HAVE_INSPECTOR inspector_agent_(new inspector::Agent(this)), #endif @@ -533,6 +534,11 @@ inline void Environment::set_fs_stats_field_array(double* fields) { fs_stats_field_array_ = fields; } +inline AliasedBuffer& +Environment::scheduled_immediate_count() { + return scheduled_immediate_count_; +} + inline performance::performance_state* Environment::performance_state() { return performance_state_; } diff --git a/src/env.h b/src/env.h index 93f18e8cbc5a61..ac5ed89a5a7ba8 100644 --- a/src/env.h +++ b/src/env.h @@ -619,6 +619,8 @@ class Environment { inline double* fs_stats_field_array() const; inline void set_fs_stats_field_array(double* fields); + inline AliasedBuffer& scheduled_immediate_count(); + inline performance::performance_state* performance_state(); inline std::map* performance_marks(); @@ -715,6 +717,8 @@ class Environment { size_t makecallback_cntr_; std::vector destroy_async_id_list_; + AliasedBuffer scheduled_immediate_count_; + performance::performance_state* performance_state_ = nullptr; std::map performance_marks_; diff --git a/src/node.cc b/src/node.cc index 261303de6f19ca..7d8d9bdc424ea1 100644 --- a/src/node.cc +++ b/src/node.cc @@ -387,25 +387,6 @@ static void PrintErrorString(const char* format, ...) { } -static void CheckImmediate(uv_check_t* handle) { - Environment* env = Environment::from_immediate_check_handle(handle); - HandleScope scope(env->isolate()); - Context::Scope context_scope(env->context()); - MakeCallback(env->isolate(), - env->process_object(), - env->immediate_callback_string(), - 0, - nullptr, - {0, 0}).ToLocalChecked(); -} - - -static void IdleImmediateDummy(uv_idle_t* handle) { - // Do nothing. Only for maintaining event loop. - // TODO(bnoordhuis) Maybe make libuv accept nullptr idle callbacks. -} - - static inline const char *errno_string(int errorno) { #define ERRNO_CASE(e) case e: return #e; switch (errorno) { @@ -3284,39 +3265,40 @@ static void DebugEnd(const FunctionCallbackInfo& args); namespace { -void NeedImmediateCallbackGetter(Local property, - const PropertyCallbackInfo& info) { - Environment* env = Environment::GetCurrent(info); - const uv_check_t* immediate_check_handle = env->immediate_check_handle(); - bool active = uv_is_active( - reinterpret_cast(immediate_check_handle)); - info.GetReturnValue().Set(active); +bool MaybeStopImmediate(Environment* env) { + if (env->scheduled_immediate_count()[0] == 0) { + uv_check_stop(env->immediate_check_handle()); + uv_idle_stop(env->immediate_idle_handle()); + return true; + } + return false; } +void CheckImmediate(uv_check_t* handle) { + Environment* env = Environment::from_immediate_check_handle(handle); + HandleScope scope(env->isolate()); + Context::Scope context_scope(env->context()); -void NeedImmediateCallbackSetter( - Local property, - Local value, - const PropertyCallbackInfo& info) { - Environment* env = Environment::GetCurrent(info); + if (MaybeStopImmediate(env)) + return; - uv_check_t* immediate_check_handle = env->immediate_check_handle(); - bool active = uv_is_active( - reinterpret_cast(immediate_check_handle)); + MakeCallback(env->isolate(), + env->process_object(), + env->immediate_callback_string(), + 0, + nullptr, + {0, 0}).ToLocalChecked(); - if (active == value->BooleanValue()) - return; + MaybeStopImmediate(env); +} - uv_idle_t* immediate_idle_handle = env->immediate_idle_handle(); - if (active) { - uv_check_stop(immediate_check_handle); - uv_idle_stop(immediate_idle_handle); - } else { - uv_check_start(immediate_check_handle, CheckImmediate); - // Idle handle is needed only to stop the event loop from blocking in poll. - uv_idle_start(immediate_idle_handle, IdleImmediateDummy); - } +void ActivateImmediateCheck(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + uv_check_start(env->immediate_check_handle(), CheckImmediate); + // Idle handle is needed only to stop the event loop from blocking in poll. + uv_idle_start(env->immediate_idle_handle(), + [](uv_idle_t*){ /* do nothing, just keep the loop running */ }); } @@ -3542,12 +3524,11 @@ void SetupProcessObject(Environment* env, process->SetAccessor(FIXED_ONE_BYTE_STRING(env->isolate(), "ppid"), GetParentProcessId); - auto need_immediate_callback_string = - FIXED_ONE_BYTE_STRING(env->isolate(), "_needImmediateCallback"); - CHECK(process->SetAccessor(env->context(), need_immediate_callback_string, - NeedImmediateCallbackGetter, - NeedImmediateCallbackSetter, - env->as_external()).FromJust()); + auto scheduled_immediate_count = + FIXED_ONE_BYTE_STRING(env->isolate(), "_scheduledImmediateCount"); + CHECK(process->Set(env->context(), + scheduled_immediate_count, + env->scheduled_immediate_count().GetJSArray()).FromJust()); // -e, --eval if (eval_string) { @@ -3673,6 +3654,9 @@ void SetupProcessObject(Environment* env, env->as_external()).FromJust()); // define various internal methods + env->SetMethod(process, + "_activateImmediateCheck", + ActivateImmediateCheck); env->SetMethod(process, "_startProfilerIdleNotifier", StartProfilerIdleNotifier); From d66d481cdf229eae4e2667c3be3c2ec1049bf7d9 Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Sun, 12 Nov 2017 18:46:55 +0100 Subject: [PATCH 08/22] async_hooks: deprecate undocumented API PR-URL: https://github.com/nodejs/node/pull/16972 Refs: https://github.com/nodejs/node/issues/14328 Refs: https://github.com/nodejs/node/issues/15572 Reviewed-By: Anna Henningsen --- doc/api/deprecations.md | 19 + lib/async_hooks.js | 430 ++++-------------- lib/dgram.js | 2 +- lib/internal/async_hooks.js | 349 ++++++++++++++ lib/internal/bootstrap_node.js | 2 +- lib/internal/process/next_tick.js | 2 +- lib/net.js | 2 +- lib/timers.js | 2 +- node.gyp | 1 + test/async-hooks/test-callback-error.js | 21 +- test/async-hooks/test-emit-before-after.js | 11 +- test/async-hooks/test-emit-init.js | 13 +- .../test-async-hooks-run-in-async-id-scope.js | 2 +- .../test-http-client-immediate-error.js | 3 +- 14 files changed, 496 insertions(+), 363 deletions(-) create mode 100644 lib/internal/async_hooks.js diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 3652f469c83fef..aaa31dd6781825 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -665,6 +665,25 @@ function for [`util.inspect()`][] is deprecated. Use [`util.inspect.custom`][] instead. For backwards compatibility with Node.js prior to version 6.4.0, both may be specified. + +### DEP0085: AsyncHooks Sensitive API + +Type: Runtime + +The AsyncHooks Sensitive API was never documented and had various of minor +issues, see https://github.com/nodejs/node/issues/15572. Use the `AsyncResource` +API instead. + + + +### DEP0086: Remove runInAsyncIdScope + +Type: Runtime + +`runInAsyncIdScope` doesn't emit the `before` or `after` event and can thus +cause a lot of issues. See https://github.com/nodejs/node/issues/14328 for more +details. + [`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size [`Buffer.from(array)`]: buffer.html#buffer_class_method_buffer_from_array [`Buffer.from(buffer)`]: buffer.html#buffer_class_method_buffer_from_buffer diff --git a/lib/async_hooks.js b/lib/async_hooks.js index b642ec1a82f094..52c887ecb1b229 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -1,119 +1,50 @@ 'use strict'; +const errors = require('internal/errors'); const internalUtil = require('internal/util'); const async_wrap = process.binding('async_wrap'); -const errors = require('internal/errors'); -/* async_hook_fields is a Uint32Array wrapping the uint32_t array of - * Environment::AsyncHooks::fields_[]. Each index tracks the number of active - * hooks for each type. - * - * async_id_fields is a Float64Array wrapping the double array of - * Environment::AsyncHooks::async_id_fields_[]. Each index contains the ids for - * the various asynchronous states of the application. These are: - * kExecutionAsyncId: The async_id assigned to the resource responsible for the - * current execution stack. - * kTriggerAsyncId: The trigger_async_id of the resource responsible for - * the current execution stack. - * kAsyncIdCounter: Incremental counter tracking the next assigned async_id. - * kInitTriggerAsyncId: Written immediately before a resource's constructor - * that sets the value of the init()'s triggerAsyncId. The order of - * retrieving the triggerAsyncId value is passing directly to the - * constructor -> value set in kInitTriggerAsyncId -> executionAsyncId of - * the current resource. - */ -const { async_hook_fields, async_id_fields } = async_wrap; -// Store the pair executionAsyncId and triggerAsyncId in a std::stack on -// Environment::AsyncHooks::ids_stack_ tracks the resource responsible for the -// current execution stack. This is unwound as each resource exits. In the case -// of a fatal exception this stack is emptied after calling each hook's after() -// callback. +const internal_async_hooks = require('internal/async_hooks'); + +// Get functions +// Only used to support a deprecated API. pushAsyncIds, popAsyncIds should +// never be directly in this manner. const { pushAsyncIds, popAsyncIds } = async_wrap; -// For performance reasons, only track Proimses when a hook is enabled. -const { enablePromiseHook, disablePromiseHook } = async_wrap; // For userland AsyncResources, make sure to emit a destroy event when the // resource gets gced. const { registerDestroyHook } = async_wrap; -// Properties in active_hooks are used to keep track of the set of hooks being -// executed in case another hook is enabled/disabled. The new set of hooks is -// then restored once the active set of hooks is finished executing. -const active_hooks = { - // Array of all AsyncHooks that will be iterated whenever an async event - // fires. Using var instead of (preferably const) in order to assign - // active_hooks.tmp_array if a hook is enabled/disabled during hook - // execution. - array: [], - // Use a counter to track nested calls of async hook callbacks and make sure - // the active_hooks.array isn't altered mid execution. - call_depth: 0, - // Use to temporarily store and updated active_hooks.array if the user - // enables or disables a hook while hooks are being processed. If a hook is - // enabled() or disabled() during hook execution then the current set of - // active hooks is duplicated and set equal to active_hooks.tmp_array. Any - // subsequent changes are on the duplicated array. When all hooks have - // completed executing active_hooks.tmp_array is assigned to - // active_hooks.array. - tmp_array: null, - // Keep track of the field counts held in active_hooks.tmp_array. Because the - // async_hook_fields can't be reassigned, store each uint32 in an array that - // is written back to async_hook_fields when active_hooks.array is restored. - tmp_fields: null -}; +const { + // Private API + getHookArrays, + enableHooks, + disableHooks, + // Sensitive Embedder API + newUid, + initTriggerId, + setInitTriggerId, + emitInit, + emitBefore, + emitAfter, + emitDestroy, +} = internal_async_hooks; +// Get fields +const { async_id_fields } = async_wrap; -// Each constant tracks how many callbacks there are for any given step of -// async execution. These are tracked so if the user didn't include callbacks -// for a given step, that step can bail out early. -const { kInit, kBefore, kAfter, kDestroy, kTotals, kPromiseResolve, - kCheck, kExecutionAsyncId, kTriggerAsyncId, kAsyncIdCounter, - kInitTriggerAsyncId } = async_wrap.constants; +// Get symbols +const { + init_symbol, before_symbol, after_symbol, destroy_symbol, + promise_resolve_symbol +} = internal_async_hooks.symbols; -// Symbols used to store the respective ids on both AsyncResource instances and -// internal resources. They will also be assigned to arbitrary objects passed -// in by the user that take place of internally constructed objects. const { async_id_symbol, trigger_async_id_symbol } = async_wrap; -// Used in AsyncHook and AsyncResource. -const init_symbol = Symbol('init'); -const before_symbol = Symbol('before'); -const after_symbol = Symbol('after'); -const destroy_symbol = Symbol('destroy'); -const promise_resolve_symbol = Symbol('promiseResolve'); -const emitBeforeNative = emitHookFactory(before_symbol, 'emitBeforeNative'); -const emitAfterNative = emitHookFactory(after_symbol, 'emitAfterNative'); -const emitDestroyNative = emitHookFactory(destroy_symbol, 'emitDestroyNative'); -const emitPromiseResolveNative = - emitHookFactory(promise_resolve_symbol, 'emitPromiseResolveNative'); - -// TODO(refack): move to node-config.cc -const abort_regex = /^--abort[_-]on[_-]uncaught[_-]exception$/; - -// Setup the callbacks that node::AsyncWrap will call when there are hooks to -// process. They use the same functions as the JS embedder API. These callbacks -// are setup immediately to prevent async_wrap.setupHooks() from being hijacked -// and the cost of doing so is negligible. -async_wrap.setupHooks({ init: emitInitNative, - before: emitBeforeNative, - after: emitAfterNative, - destroy: emitDestroyNative, - promise_resolve: emitPromiseResolveNative }); - -// Used to fatally abort the process if a callback throws. -function fatalError(e) { - if (typeof e.stack === 'string') { - process._rawDebug(e.stack); - } else { - const o = { message: e }; - Error.captureStackTrace(o, fatalError); - process._rawDebug(o.stack); - } - if (process.execArgv.some((e) => abort_regex.test(e))) { - process.abort(); - } - process.exit(1); -} - +// Get constants +const { + kInit, kBefore, kAfter, kDestroy, kTotals, kPromiseResolve, + kExecutionAsyncId, kTriggerAsyncId +} = async_wrap.constants; -// Public API // +// Listener API // class AsyncHook { constructor({ init, before, after, destroy, promiseResolve }) { @@ -161,8 +92,7 @@ class AsyncHook { hooks_array.push(this); if (prev_kTotals === 0 && hook_fields[kTotals] > 0) { - enablePromiseHook(); - hook_fields[kCheck] += 1; + enableHooks(); } return this; @@ -187,8 +117,7 @@ class AsyncHook { hooks_array.splice(index, 1); if (prev_kTotals > 0 && hook_fields[kTotals] === 0) { - disablePromiseHook(); - hook_fields[kCheck] -= 1; + disableHooks(); } return this; @@ -196,47 +125,6 @@ class AsyncHook { } -function getHookArrays() { - if (active_hooks.call_depth === 0) - return [active_hooks.array, async_hook_fields]; - // If this hook is being enabled while in the middle of processing the array - // of currently active hooks then duplicate the current set of active hooks - // and store this there. This shouldn't fire until the next time hooks are - // processed. - if (active_hooks.tmp_array === null) - storeActiveHooks(); - return [active_hooks.tmp_array, active_hooks.tmp_fields]; -} - - -function storeActiveHooks() { - active_hooks.tmp_array = active_hooks.array.slice(); - // Don't want to make the assumption that kInit to kDestroy are indexes 0 to - // 4. So do this the long way. - active_hooks.tmp_fields = []; - active_hooks.tmp_fields[kInit] = async_hook_fields[kInit]; - active_hooks.tmp_fields[kBefore] = async_hook_fields[kBefore]; - active_hooks.tmp_fields[kAfter] = async_hook_fields[kAfter]; - active_hooks.tmp_fields[kDestroy] = async_hook_fields[kDestroy]; - active_hooks.tmp_fields[kPromiseResolve] = async_hook_fields[kPromiseResolve]; -} - - -// Then restore the correct hooks array in case any hooks were added/removed -// during hook callback execution. -function restoreActiveHooks() { - active_hooks.array = active_hooks.tmp_array; - async_hook_fields[kInit] = active_hooks.tmp_fields[kInit]; - async_hook_fields[kBefore] = active_hooks.tmp_fields[kBefore]; - async_hook_fields[kAfter] = active_hooks.tmp_fields[kAfter]; - async_hook_fields[kDestroy] = active_hooks.tmp_fields[kDestroy]; - async_hook_fields[kPromiseResolve] = active_hooks.tmp_fields[kPromiseResolve]; - - active_hooks.tmp_array = null; - active_hooks.tmp_fields = null; -} - - function createHook(fns) { return new AsyncHook(fns); } @@ -251,15 +139,6 @@ function triggerAsyncId() { return async_id_fields[kTriggerAsyncId]; } -function validateAsyncId(asyncId, type) { - // Skip validation when async_hooks is disabled - if (async_hook_fields[kCheck] <= 0) return; - - if (!Number.isSafeInteger(asyncId) || asyncId < -1) { - fatalError(new errors.RangeError('ERR_INVALID_ASYNC_ID', type, asyncId)); - } -} - // Embedder API // @@ -285,12 +164,12 @@ class AsyncResource { triggerAsyncId); } - this[async_id_symbol] = ++async_id_fields[kAsyncIdCounter]; + this[async_id_symbol] = newUid(); this[trigger_async_id_symbol] = triggerAsyncId; // this prop name (destroyed) has to be synchronized with C++ this[destroyedSymbol] = { destroyed: false }; - emitInitScript( + emitInit( this[async_id_symbol], type, this[trigger_async_id_symbol], this ); @@ -300,18 +179,18 @@ class AsyncResource { } emitBefore() { - emitBeforeScript(this[async_id_symbol], this[trigger_async_id_symbol]); + emitBefore(this[async_id_symbol], this[trigger_async_id_symbol]); return this; } emitAfter() { - emitAfterScript(this[async_id_symbol]); + emitAfter(this[async_id_symbol]); return this; } emitDestroy() { this[destroyedSymbol].destroyed = true; - emitDestroyScript(this[async_id_symbol]); + emitDestroy(this[async_id_symbol]); return this; } @@ -348,168 +227,6 @@ function runInAsyncIdScope(asyncId, cb) { } } - -// Sensitive Embedder API // - -// Increment the internal id counter and return the value. Important that the -// counter increment first. Since it's done the same way in -// Environment::new_async_uid() -function newUid() { - return ++async_id_fields[kAsyncIdCounter]; -} - - -// Return the triggerAsyncId meant for the constructor calling it. It's up to -// the user to safeguard this call and make sure it's zero'd out when the -// constructor is complete. -function initTriggerId() { - var triggerAsyncId = async_id_fields[kInitTriggerAsyncId]; - // Reset value after it's been called so the next constructor doesn't - // inherit it by accident. - async_id_fields[kInitTriggerAsyncId] = 0; - if (triggerAsyncId <= 0) - triggerAsyncId = async_id_fields[kExecutionAsyncId]; - return triggerAsyncId; -} - - -function setInitTriggerId(triggerAsyncId) { - // CHECK(Number.isSafeInteger(triggerAsyncId)) - // CHECK(triggerAsyncId > 0) - async_id_fields[kInitTriggerAsyncId] = triggerAsyncId; -} - - -function emitInitScript(asyncId, type, triggerAsyncId, resource) { - validateAsyncId(asyncId, 'asyncId'); - if (triggerAsyncId !== null) - validateAsyncId(triggerAsyncId, 'triggerAsyncId'); - if (async_hook_fields[kCheck] > 0 && - (typeof type !== 'string' || type.length <= 0)) { - throw new errors.TypeError('ERR_ASYNC_TYPE', type); - } - - // Short circuit all checks for the common case. Which is that no hooks have - // been set. Do this to remove performance impact for embedders (and core). - if (async_hook_fields[kInit] === 0) - return; - - // This can run after the early return check b/c running this function - // manually means that the embedder must have used initTriggerId(). - if (triggerAsyncId === null) { - triggerAsyncId = initTriggerId(); - } else { - // If a triggerAsyncId was passed, any kInitTriggerAsyncId still must be - // null'd. - async_id_fields[kInitTriggerAsyncId] = 0; - } - - emitInitNative(asyncId, type, triggerAsyncId, resource); -} - -function emitHookFactory(symbol, name) { - // Called from native. The asyncId stack handling is taken care of there - // before this is called. - // eslint-disable-next-line func-style - const fn = function(asyncId) { - active_hooks.call_depth += 1; - // Use a single try/catch for all hook to avoid setting up one per - // iteration. - try { - for (var i = 0; i < active_hooks.array.length; i++) { - if (typeof active_hooks.array[i][symbol] === 'function') { - active_hooks.array[i][symbol](asyncId); - } - } - } catch (e) { - fatalError(e); - } finally { - active_hooks.call_depth -= 1; - } - - // Hooks can only be restored if there have been no recursive hook calls. - // Also the active hooks do not need to be restored if enable()/disable() - // weren't called during hook execution, in which case - // active_hooks.tmp_array will be null. - if (active_hooks.call_depth === 0 && active_hooks.tmp_array !== null) { - restoreActiveHooks(); - } - }; - - // Set the name property of the anonymous function as it looks good in the - // stack trace. - Object.defineProperty(fn, 'name', { - value: name - }); - return fn; -} - - -function emitBeforeScript(asyncId, triggerAsyncId) { - // Validate the ids. An id of -1 means it was never set and is visible on the - // call graph. An id < -1 should never happen in any circumstance. Throw - // on user calls because async state should still be recoverable. - validateAsyncId(asyncId, 'asyncId'); - validateAsyncId(triggerAsyncId, 'triggerAsyncId'); - - pushAsyncIds(asyncId, triggerAsyncId); - - if (async_hook_fields[kBefore] > 0) - emitBeforeNative(asyncId); -} - - -function emitAfterScript(asyncId) { - validateAsyncId(asyncId, 'asyncId'); - - if (async_hook_fields[kAfter] > 0) - emitAfterNative(asyncId); - - popAsyncIds(asyncId); -} - - -function emitDestroyScript(asyncId) { - validateAsyncId(asyncId, 'asyncId'); - - // Return early if there are no destroy callbacks, or invalid asyncId. - if (async_hook_fields[kDestroy] === 0 || asyncId <= 0) - return; - async_wrap.queueDestroyAsyncId(asyncId); -} - - -// Used by C++ to call all init() callbacks. Because some state can be setup -// from C++ there's no need to perform all the same operations as in -// emitInitScript. -function emitInitNative(asyncId, type, triggerAsyncId, resource) { - active_hooks.call_depth += 1; - // Use a single try/catch for all hook to avoid setting up one per iteration. - try { - for (var i = 0; i < active_hooks.array.length; i++) { - if (typeof active_hooks.array[i][init_symbol] === 'function') { - active_hooks.array[i][init_symbol]( - asyncId, type, triggerAsyncId, - resource - ); - } - } - } catch (e) { - fatalError(e); - } finally { - active_hooks.call_depth -= 1; - } - - // Hooks can only be restored if there have been no recursive hook calls. - // Also the active hooks do not need to be restored if enable()/disable() - // weren't called during hook execution, in which case active_hooks.tmp_array - // will be null. - if (active_hooks.call_depth === 0 && active_hooks.tmp_array !== null) { - restoreActiveHooks(); - } -} - - // Placing all exports down here because the exported classes won't export // otherwise. module.exports = { @@ -519,17 +236,10 @@ module.exports = { triggerAsyncId, // Embedder API AsyncResource, - runInAsyncIdScope, - // Sensitive Embedder API - newUid, - initTriggerId, - setInitTriggerId, - emitInit: emitInitScript, - emitBefore: emitBeforeScript, - emitAfter: emitAfterScript, - emitDestroy: emitDestroyScript, }; +// Deprecated API // + // currentId was renamed to executionAsyncId. This was in 8.2.0 during the // experimental stage so the alias can be removed at any time, we are just // being nice :) @@ -549,3 +259,59 @@ Object.defineProperty(module.exports, 'triggerId', { }, 'async_hooks.triggerId is deprecated. ' + 'Use async_hooks.triggerAsyncId instead.', 'DEP0071') }); + +Object.defineProperty(module.exports, 'runInAsyncIdScope', { + get: internalUtil.deprecate(function() { + return runInAsyncIdScope; + }, 'async_hooks.runInAsyncIdScope is deprecated. ' + + 'Create an AsyncResource instead.', 'DEP0086') +}); + +Object.defineProperty(module.exports, 'newUid', { + get: internalUtil.deprecate(function() { + return newUid; + }, 'async_hooks.newUid is deprecated. ' + + 'Use AsyncResource instead.', 'DEP0085') +}); + +Object.defineProperty(module.exports, 'initTriggerId', { + get: internalUtil.deprecate(function() { + return initTriggerId; + }, 'async_hooks.initTriggerId is deprecated. ' + + 'Use the AsyncResource default instead.', 'DEP0085') +}); + +Object.defineProperty(module.exports, 'setInitTriggerId', { + get: internalUtil.deprecate(function() { + return setInitTriggerId; + }, 'async_hooks.setInitTriggerId is deprecated. ' + + 'Use the triggerAsyncId parameter in AsyncResource instead.', 'DEP0085') +}); + +Object.defineProperty(module.exports, 'emitInit', { + get: internalUtil.deprecate(function() { + return emitInit; + }, 'async_hooks.emitInit is deprecated. ' + + 'Use AsyncResource constructor instead.', 'DEP0085') +}); + +Object.defineProperty(module.exports, 'emitBefore', { + get: internalUtil.deprecate(function() { + return emitBefore; + }, 'async_hooks.emitBefore is deprecated. ' + + 'Use AsyncResource.emitBefore instead.', 'DEP0085') +}); + +Object.defineProperty(module.exports, 'emitAfter', { + get: internalUtil.deprecate(function() { + return emitAfter; + }, 'async_hooks.emitAfter is deprecated. ' + + 'Use AsyncResource.emitAfter instead.', 'DEP0085') +}); + +Object.defineProperty(module.exports, 'emitDestroy', { + get: internalUtil.deprecate(function() { + return emitDestroy; + }, 'async_hooks.emitDestroy is deprecated. ' + + 'Use AsyncResource.emitDestroy instead.', 'DEP0085') +}); diff --git a/lib/dgram.js b/lib/dgram.js index ef07a4f85bdeed..8fe52713bc37aa 100644 --- a/lib/dgram.js +++ b/lib/dgram.js @@ -28,7 +28,7 @@ const dns = require('dns'); const util = require('util'); const { isUint8Array } = require('internal/util/types'); const EventEmitter = require('events'); -const { setInitTriggerId } = require('async_hooks'); +const { setInitTriggerId } = require('internal/async_hooks'); const { UV_UDP_REUSEADDR } = process.binding('constants').os; const { async_id_symbol } = process.binding('async_wrap'); const { nextTick } = require('internal/process/next_tick'); diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js new file mode 100644 index 00000000000000..5964a847fc0a25 --- /dev/null +++ b/lib/internal/async_hooks.js @@ -0,0 +1,349 @@ +'use strict'; + +const errors = require('internal/errors'); +const async_wrap = process.binding('async_wrap'); +/* async_hook_fields is a Uint32Array wrapping the uint32_t array of + * Environment::AsyncHooks::fields_[]. Each index tracks the number of active + * hooks for each type. + * + * async_id_fields is a Float64Array wrapping the double array of + * Environment::AsyncHooks::async_id_fields_[]. Each index contains the ids for + * the various asynchronous states of the application. These are: + * kExecutionAsyncId: The async_id assigned to the resource responsible for the + * current execution stack. + * kTriggerAsyncId: The trigger_async_id of the resource responsible for + * the current execution stack. + * kAsyncIdCounter: Incremental counter tracking the next assigned async_id. + * kInitTriggerAsyncId: Written immediately before a resource's constructor + * that sets the value of the init()'s triggerAsyncId. The order of + * retrieving the triggerAsyncId value is passing directly to the + * constructor -> value set in kInitTriggerAsyncId -> executionAsyncId of + * the current resource. + */ +const { async_hook_fields, async_id_fields } = async_wrap; +// Store the pair executionAsyncId and triggerAsyncId in a std::stack on +// Environment::AsyncHooks::ids_stack_ tracks the resource responsible for the +// current execution stack. This is unwound as each resource exits. In the case +// of a fatal exception this stack is emptied after calling each hook's after() +// callback. +const { pushAsyncIds, popAsyncIds } = async_wrap; +// For performance reasons, only track Proimses when a hook is enabled. +const { enablePromiseHook, disablePromiseHook } = async_wrap; +// Properties in active_hooks are used to keep track of the set of hooks being +// executed in case another hook is enabled/disabled. The new set of hooks is +// then restored once the active set of hooks is finished executing. +const active_hooks = { + // Array of all AsyncHooks that will be iterated whenever an async event + // fires. Using var instead of (preferably const) in order to assign + // active_hooks.tmp_array if a hook is enabled/disabled during hook + // execution. + array: [], + // Use a counter to track nested calls of async hook callbacks and make sure + // the active_hooks.array isn't altered mid execution. + call_depth: 0, + // Use to temporarily store and updated active_hooks.array if the user + // enables or disables a hook while hooks are being processed. If a hook is + // enabled() or disabled() during hook execution then the current set of + // active hooks is duplicated and set equal to active_hooks.tmp_array. Any + // subsequent changes are on the duplicated array. When all hooks have + // completed executing active_hooks.tmp_array is assigned to + // active_hooks.array. + tmp_array: null, + // Keep track of the field counts held in active_hooks.tmp_array. Because the + // async_hook_fields can't be reassigned, store each uint32 in an array that + // is written back to async_hook_fields when active_hooks.array is restored. + tmp_fields: null +}; + + +// Each constant tracks how many callbacks there are for any given step of +// async execution. These are tracked so if the user didn't include callbacks +// for a given step, that step can bail out early. +const { kInit, kBefore, kAfter, kDestroy, kPromiseResolve, + kCheck, kExecutionAsyncId, kAsyncIdCounter, + kInitTriggerAsyncId } = async_wrap.constants; + +// Used in AsyncHook and AsyncResource. +const init_symbol = Symbol('init'); +const before_symbol = Symbol('before'); +const after_symbol = Symbol('after'); +const destroy_symbol = Symbol('destroy'); +const promise_resolve_symbol = Symbol('promiseResolve'); +const emitBeforeNative = emitHookFactory(before_symbol, 'emitBeforeNative'); +const emitAfterNative = emitHookFactory(after_symbol, 'emitAfterNative'); +const emitDestroyNative = emitHookFactory(destroy_symbol, 'emitDestroyNative'); +const emitPromiseResolveNative = + emitHookFactory(promise_resolve_symbol, 'emitPromiseResolveNative'); + +// TODO(refack): move to node-config.cc +const abort_regex = /^--abort[_-]on[_-]uncaught[_-]exception$/; + +// Setup the callbacks that node::AsyncWrap will call when there are hooks to +// process. They use the same functions as the JS embedder API. These callbacks +// are setup immediately to prevent async_wrap.setupHooks() from being hijacked +// and the cost of doing so is negligible. +async_wrap.setupHooks({ init: emitInitNative, + before: emitBeforeNative, + after: emitAfterNative, + destroy: emitDestroyNative, + promise_resolve: emitPromiseResolveNative }); + +// Used to fatally abort the process if a callback throws. +function fatalError(e) { + if (typeof e.stack === 'string') { + process._rawDebug(e.stack); + } else { + const o = { message: e }; + Error.captureStackTrace(o, fatalError); + process._rawDebug(o.stack); + } + if (process.execArgv.some((e) => abort_regex.test(e))) { + process.abort(); + } + process.exit(1); +} + + +function validateAsyncId(asyncId, type) { + // Skip validation when async_hooks is disabled + if (async_hook_fields[kCheck] <= 0) return; + + if (!Number.isSafeInteger(asyncId) || asyncId < -1) { + fatalError(new errors.RangeError('ERR_INVALID_ASYNC_ID', type, asyncId)); + } +} + +// Emit From Native // + +// Used by C++ to call all init() callbacks. Because some state can be setup +// from C++ there's no need to perform all the same operations as in +// emitInitScript. +function emitInitNative(asyncId, type, triggerAsyncId, resource) { + active_hooks.call_depth += 1; + // Use a single try/catch for all hook to avoid setting up one per iteration. + try { + for (var i = 0; i < active_hooks.array.length; i++) { + if (typeof active_hooks.array[i][init_symbol] === 'function') { + active_hooks.array[i][init_symbol]( + asyncId, type, triggerAsyncId, + resource + ); + } + } + } catch (e) { + fatalError(e); + } finally { + active_hooks.call_depth -= 1; + } + + // Hooks can only be restored if there have been no recursive hook calls. + // Also the active hooks do not need to be restored if enable()/disable() + // weren't called during hook execution, in which case active_hooks.tmp_array + // will be null. + if (active_hooks.call_depth === 0 && active_hooks.tmp_array !== null) { + restoreActiveHooks(); + } +} + + +function emitHookFactory(symbol, name) { + // Called from native. The asyncId stack handling is taken care of there + // before this is called. + // eslint-disable-next-line func-style + const fn = function(asyncId) { + active_hooks.call_depth += 1; + // Use a single try/catch for all hook to avoid setting up one per + // iteration. + try { + for (var i = 0; i < active_hooks.array.length; i++) { + if (typeof active_hooks.array[i][symbol] === 'function') { + active_hooks.array[i][symbol](asyncId); + } + } + } catch (e) { + fatalError(e); + } finally { + active_hooks.call_depth -= 1; + } + + // Hooks can only be restored if there have been no recursive hook calls. + // Also the active hooks do not need to be restored if enable()/disable() + // weren't called during hook execution, in which case + // active_hooks.tmp_array will be null. + if (active_hooks.call_depth === 0 && active_hooks.tmp_array !== null) { + restoreActiveHooks(); + } + }; + + // Set the name property of the anonymous function as it looks good in the + // stack trace. + Object.defineProperty(fn, 'name', { + value: name + }); + return fn; +} + +// Manage Active Hooks // + +function getHookArrays() { + if (active_hooks.call_depth === 0) + return [active_hooks.array, async_hook_fields]; + // If this hook is being enabled while in the middle of processing the array + // of currently active hooks then duplicate the current set of active hooks + // and store this there. This shouldn't fire until the next time hooks are + // processed. + if (active_hooks.tmp_array === null) + storeActiveHooks(); + return [active_hooks.tmp_array, active_hooks.tmp_fields]; +} + + +function storeActiveHooks() { + active_hooks.tmp_array = active_hooks.array.slice(); + // Don't want to make the assumption that kInit to kDestroy are indexes 0 to + // 4. So do this the long way. + active_hooks.tmp_fields = []; + active_hooks.tmp_fields[kInit] = async_hook_fields[kInit]; + active_hooks.tmp_fields[kBefore] = async_hook_fields[kBefore]; + active_hooks.tmp_fields[kAfter] = async_hook_fields[kAfter]; + active_hooks.tmp_fields[kDestroy] = async_hook_fields[kDestroy]; + active_hooks.tmp_fields[kPromiseResolve] = async_hook_fields[kPromiseResolve]; +} + + +// Then restore the correct hooks array in case any hooks were added/removed +// during hook callback execution. +function restoreActiveHooks() { + active_hooks.array = active_hooks.tmp_array; + async_hook_fields[kInit] = active_hooks.tmp_fields[kInit]; + async_hook_fields[kBefore] = active_hooks.tmp_fields[kBefore]; + async_hook_fields[kAfter] = active_hooks.tmp_fields[kAfter]; + async_hook_fields[kDestroy] = active_hooks.tmp_fields[kDestroy]; + async_hook_fields[kPromiseResolve] = active_hooks.tmp_fields[kPromiseResolve]; + + active_hooks.tmp_array = null; + active_hooks.tmp_fields = null; +} + + +function enableHooks() { + enablePromiseHook(); + async_hook_fields[kCheck] += 1; +} + +function disableHooks() { + disablePromiseHook(); + async_hook_fields[kCheck] -= 1; +} + +// Sensitive Embedder API // + +// Increment the internal id counter and return the value. Important that the +// counter increment first. Since it's done the same way in +// Environment::new_async_uid() +function newUid() { + return ++async_id_fields[kAsyncIdCounter]; +} + + +// Return the triggerAsyncId meant for the constructor calling it. It's up to +// the user to safeguard this call and make sure it's zero'd out when the +// constructor is complete. +function initTriggerId() { + var triggerAsyncId = async_id_fields[kInitTriggerAsyncId]; + // Reset value after it's been called so the next constructor doesn't + // inherit it by accident. + async_id_fields[kInitTriggerAsyncId] = 0; + if (triggerAsyncId <= 0) + triggerAsyncId = async_id_fields[kExecutionAsyncId]; + return triggerAsyncId; +} + + +function setInitTriggerId(triggerAsyncId) { + // CHECK(Number.isSafeInteger(triggerAsyncId)) + // CHECK(triggerAsyncId > 0) + async_id_fields[kInitTriggerAsyncId] = triggerAsyncId; +} + + +function emitInitScript(asyncId, type, triggerAsyncId, resource) { + validateAsyncId(asyncId, 'asyncId'); + if (triggerAsyncId !== null) + validateAsyncId(triggerAsyncId, 'triggerAsyncId'); + if (async_hook_fields[kCheck] > 0 && + (typeof type !== 'string' || type.length <= 0)) { + throw new errors.TypeError('ERR_ASYNC_TYPE', type); + } + + // Short circuit all checks for the common case. Which is that no hooks have + // been set. Do this to remove performance impact for embedders (and core). + if (async_hook_fields[kInit] === 0) + return; + + // This can run after the early return check b/c running this function + // manually means that the embedder must have used initTriggerId(). + if (triggerAsyncId === null) { + triggerAsyncId = initTriggerId(); + } else { + // If a triggerAsyncId was passed, any kInitTriggerAsyncId still must be + // null'd. + async_id_fields[kInitTriggerAsyncId] = 0; + } + + emitInitNative(asyncId, type, triggerAsyncId, resource); +} + + +function emitBeforeScript(asyncId, triggerAsyncId) { + // Validate the ids. An id of -1 means it was never set and is visible on the + // call graph. An id < -1 should never happen in any circumstance. Throw + // on user calls because async state should still be recoverable. + validateAsyncId(asyncId, 'asyncId'); + validateAsyncId(triggerAsyncId, 'triggerAsyncId'); + + pushAsyncIds(asyncId, triggerAsyncId); + + if (async_hook_fields[kBefore] > 0) + emitBeforeNative(asyncId); +} + + +function emitAfterScript(asyncId) { + validateAsyncId(asyncId, 'asyncId'); + + if (async_hook_fields[kAfter] > 0) + emitAfterNative(asyncId); + + popAsyncIds(asyncId); +} + + +function emitDestroyScript(asyncId) { + validateAsyncId(asyncId, 'asyncId'); + + // Return early if there are no destroy callbacks, or invalid asyncId. + if (async_hook_fields[kDestroy] === 0 || asyncId <= 0) + return; + async_wrap.queueDestroyAsyncId(asyncId); +} + + +module.exports = { + // Private API + getHookArrays, + symbols: { + init_symbol, before_symbol, after_symbol, destroy_symbol, + promise_resolve_symbol + }, + enableHooks, + disableHooks, + // Sensitive Embedder API + newUid, + initTriggerId, + setInitTriggerId, + emitInit: emitInitScript, + emitBefore: emitBeforeScript, + emitAfter: emitAfterScript, + emitDestroy: emitDestroyScript, +}; diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index bebf47e5837860..f751cf45e5878e 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -393,7 +393,7 @@ // Emit the after() hooks now that the exception has been handled. if (async_hook_fields[kAfter] > 0) { do { - NativeModule.require('async_hooks').emitAfter( + NativeModule.require('internal/async_hooks').emitAfter( async_id_fields[kExecutionAsyncId]); } while (asyncIdStackSize() > 0); // Or completely empty the id stack. diff --git a/lib/internal/process/next_tick.js b/lib/internal/process/next_tick.js index fa144c5969b6f9..59a1e4fee1c75f 100644 --- a/lib/internal/process/next_tick.js +++ b/lib/internal/process/next_tick.js @@ -48,7 +48,7 @@ class NextTickQueue { function setupNextTick() { const async_wrap = process.binding('async_wrap'); - const async_hooks = require('async_hooks'); + const async_hooks = require('internal/async_hooks'); const promises = require('internal/process/promises'); const errors = require('internal/errors'); const emitPendingUnhandledRejections = promises.setup(scheduleMicrotasks); diff --git a/lib/net.js b/lib/net.js index 4d373bc1e10c33..43ce4d803e03af 100644 --- a/lib/net.js +++ b/lib/net.js @@ -39,7 +39,7 @@ const { TCPConnectWrap } = process.binding('tcp_wrap'); const { PipeConnectWrap } = process.binding('pipe_wrap'); const { ShutdownWrap, WriteWrap } = process.binding('stream_wrap'); const { async_id_symbol } = process.binding('async_wrap'); -const { newUid, setInitTriggerId } = require('async_hooks'); +const { newUid, setInitTriggerId } = require('internal/async_hooks'); const { nextTick } = require('internal/process/next_tick'); const errors = require('internal/errors'); const dns = require('dns'); diff --git a/lib/timers.js b/lib/timers.js index c7237a25353e5f..d12c080cf7ab5d 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -39,7 +39,7 @@ const { emitBefore, emitAfter, emitDestroy -} = require('async_hooks'); +} = require('internal/async_hooks'); // Grab the constants necessary for working with internal arrays. const { kInit, kDestroy, kAsyncIdCounter } = async_wrap.constants; // Symbols for storing async id state. diff --git a/node.gyp b/node.gyp index 4ec7da6c2f1126..8678093d7442ad 100644 --- a/node.gyp +++ b/node.gyp @@ -77,6 +77,7 @@ 'lib/v8.js', 'lib/vm.js', 'lib/zlib.js', + 'lib/internal/async_hooks.js', 'lib/internal/buffer.js', 'lib/internal/child_process.js', 'lib/internal/cluster/child.js', diff --git a/test/async-hooks/test-callback-error.js b/test/async-hooks/test-callback-error.js index a4b8a99f33e858..09eb2e0b478a6e 100644 --- a/test/async-hooks/test-callback-error.js +++ b/test/async-hooks/test-callback-error.js @@ -11,35 +11,22 @@ switch (arg) { initHooks({ oninit: common.mustCall(() => { throw new Error(arg); }) }).enable(); - async_hooks.emitInit( - async_hooks.newUid(), - `${arg}_type`, - async_hooks.executionAsyncId() - ); + new async_hooks.AsyncResource(`${arg}_type`); return; case 'test_callback': initHooks({ onbefore: common.mustCall(() => { throw new Error(arg); }) }).enable(); - const newAsyncId = async_hooks.newUid(); - async_hooks.emitInit( - newAsyncId, - `${arg}_type`, - async_hooks.executionAsyncId() - ); - async_hooks.emitBefore(newAsyncId, async_hooks.executionAsyncId()); + const resource = new async_hooks.AsyncResource(`${arg}_type`); + resource.emitBefore(); return; case 'test_callback_abort': initHooks({ oninit: common.mustCall(() => { throw new Error(arg); }) }).enable(); - async_hooks.emitInit( - async_hooks.newUid(), - `${arg}_type`, - async_hooks.executionAsyncId() - ); + new async_hooks.AsyncResource(`${arg}_type`); return; } diff --git a/test/async-hooks/test-emit-before-after.js b/test/async-hooks/test-emit-before-after.js index 2b22739fa9478d..1b28c1e42622dd 100644 --- a/test/async-hooks/test-emit-before-after.js +++ b/test/async-hooks/test-emit-before-after.js @@ -1,9 +1,10 @@ 'use strict'; +// Flags: --expose-internals const common = require('../common'); const assert = require('assert'); const spawnSync = require('child_process').spawnSync; -const async_hooks = require('async_hooks'); +const async_hooks = require('internal/async_hooks'); const initHooks = require('./init-hooks'); switch (process.argv[2]) { @@ -17,13 +18,17 @@ switch (process.argv[2]) { assert.ok(!process.argv[2]); -const c1 = spawnSync(process.execPath, [__filename, 'test_invalid_async_id']); +const c1 = spawnSync(process.execPath, [ + '--expose-internals', __filename, 'test_invalid_async_id' +]); assert.strictEqual( c1.stderr.toString().split(/[\r\n]+/g)[0], 'RangeError [ERR_INVALID_ASYNC_ID]: Invalid asyncId value: -2'); assert.strictEqual(c1.status, 1); -const c2 = spawnSync(process.execPath, [__filename, 'test_invalid_trigger_id']); +const c2 = spawnSync(process.execPath, [ + '--expose-internals', __filename, 'test_invalid_trigger_id' +]); assert.strictEqual( c2.stderr.toString().split(/[\r\n]+/g)[0], 'RangeError [ERR_INVALID_ASYNC_ID]: Invalid triggerAsyncId value: -2'); diff --git a/test/async-hooks/test-emit-init.js b/test/async-hooks/test-emit-init.js index 9c61f19dab7784..e69285d4f81c84 100644 --- a/test/async-hooks/test-emit-init.js +++ b/test/async-hooks/test-emit-init.js @@ -1,9 +1,10 @@ 'use strict'; +// Flags: --expose-internals const common = require('../common'); const assert = require('assert'); const spawnSync = require('child_process').spawnSync; -const async_hooks = require('async_hooks'); +const async_hooks = require('internal/async_hooks'); const initHooks = require('./init-hooks'); const expectedId = async_hooks.newUid(); @@ -36,20 +37,24 @@ switch (process.argv[2]) { assert.ok(!process.argv[2]); -const c1 = spawnSync(process.execPath, [__filename, 'test_invalid_async_id']); +const c1 = spawnSync(process.execPath, [ + '--expose-internals', __filename, 'test_invalid_async_id' +]); assert.strictEqual( c1.stderr.toString().split(/[\r\n]+/g)[0], 'RangeError [ERR_INVALID_ASYNC_ID]: Invalid asyncId value: undefined'); assert.strictEqual(c1.status, 1); -const c2 = spawnSync(process.execPath, [__filename, 'test_invalid_trigger_id']); +const c2 = spawnSync(process.execPath, [ + '--expose-internals', __filename, 'test_invalid_trigger_id' +]); assert.strictEqual( c2.stderr.toString().split(/[\r\n]+/g)[0], 'RangeError [ERR_INVALID_ASYNC_ID]: Invalid triggerAsyncId value: undefined'); assert.strictEqual(c2.status, 1); const c3 = spawnSync(process.execPath, [ - __filename, 'test_invalid_trigger_id_negative' + '--expose-internals', __filename, 'test_invalid_trigger_id_negative' ]); assert.strictEqual( c3.stderr.toString().split(/[\r\n]+/g)[0], diff --git a/test/parallel/test-async-hooks-run-in-async-id-scope.js b/test/parallel/test-async-hooks-run-in-async-id-scope.js index 8cef7d214c2b4f..14d1c7423fcf29 100644 --- a/test/parallel/test-async-hooks-run-in-async-id-scope.js +++ b/test/parallel/test-async-hooks-run-in-async-id-scope.js @@ -4,7 +4,7 @@ const common = require('../common'); const assert = require('assert'); const async_hooks = require('async_hooks'); -const asyncId = async_hooks.newUid(); +const asyncId = new async_hooks.AsyncResource('test').asyncId(); assert.notStrictEqual(async_hooks.executionAsyncId(), asyncId); diff --git a/test/parallel/test-http-client-immediate-error.js b/test/parallel/test-http-client-immediate-error.js index 6b9cacb256f927..abbf5c41fc6660 100644 --- a/test/parallel/test-http-client-immediate-error.js +++ b/test/parallel/test-http-client-immediate-error.js @@ -1,4 +1,5 @@ 'use strict'; +// Flags: --expose-internals // Make sure http.request() can catch immediate errors in // net.createConnection(). @@ -9,7 +10,7 @@ const net = require('net'); const http = require('http'); const uv = process.binding('uv'); const { async_id_symbol } = process.binding('async_wrap'); -const { newUid } = require('async_hooks'); +const { newUid } = require('internal/async_hooks'); const agent = new http.Agent(); agent.createConnection = common.mustCall((cfg) => { From c904ce1248c1fefb565dfc5cc04f507870264be8 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 18 Nov 2017 14:24:04 +0100 Subject: [PATCH 09/22] src: introduce internal C++ SetImmediate() mechanism PR-URL: https://github.com/nodejs/node/pull/17117 Reviewed-By: James M Snell --- src/env-inl.h | 7 +++++++ src/env.cc | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/env.h | 13 +++++++++++++ src/node.cc | 33 +------------------------------- 4 files changed, 73 insertions(+), 32 deletions(-) diff --git a/src/env-inl.h b/src/env-inl.h index 66c677dd6ebccf..90297361cf63bc 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -539,6 +539,13 @@ Environment::scheduled_immediate_count() { return scheduled_immediate_count_; } +void Environment::SetImmediate(native_immediate_callback cb, void* data) { + native_immediate_callbacks_.push_back({ cb, data }); + if (scheduled_immediate_count_[0] == 0) + ActivateImmediateCheck(); + scheduled_immediate_count_[0] = scheduled_immediate_count_[0] + 1; +} + inline performance::performance_state* Environment::performance_state() { return performance_state_; } diff --git a/src/env.cc b/src/env.cc index c46f02dd0ec52f..8899fe987ccd02 100644 --- a/src/env.cc +++ b/src/env.cc @@ -223,4 +223,56 @@ void Environment::EnvPromiseHook(v8::PromiseHookType type, } } +void Environment::RunAndClearNativeImmediates() { + size_t count = native_immediate_callbacks_.size(); + if (count > 0) { + std::vector list; + native_immediate_callbacks_.swap(list); + for (const auto& cb : list) { + cb.cb_(this, cb.data_); + } + +#ifdef DEBUG + CHECK_GE(scheduled_immediate_count_[0], count); +#endif + scheduled_immediate_count_[0] = scheduled_immediate_count_[0] - count; + } +} + +static bool MaybeStopImmediate(Environment* env) { + if (env->scheduled_immediate_count()[0] == 0) { + uv_check_stop(env->immediate_check_handle()); + uv_idle_stop(env->immediate_idle_handle()); + return true; + } + return false; +} + + +void Environment::CheckImmediate(uv_check_t* handle) { + Environment* env = Environment::from_immediate_check_handle(handle); + HandleScope scope(env->isolate()); + Context::Scope context_scope(env->context()); + + if (MaybeStopImmediate(env)) + return; + + env->RunAndClearNativeImmediates(); + + MakeCallback(env->isolate(), + env->process_object(), + env->immediate_callback_string(), + 0, + nullptr, + {0, 0}).ToLocalChecked(); + + MaybeStopImmediate(env); +} + +void Environment::ActivateImmediateCheck() { + uv_check_start(&immediate_check_handle_, CheckImmediate); + // Idle handle is needed only to stop the event loop from blocking in poll. + uv_idle_start(&immediate_idle_handle_, [](uv_idle_t*){ }); +} + } // namespace node diff --git a/src/env.h b/src/env.h index ac5ed89a5a7ba8..80be909442b2dd 100644 --- a/src/env.h +++ b/src/env.h @@ -693,6 +693,11 @@ class Environment { bool RemovePromiseHook(promise_hook_func fn, void* arg); bool EmitNapiWarning(); + typedef void (*native_immediate_callback)(Environment* env, void* data); + inline void SetImmediate(native_immediate_callback cb, void* data); + // This needs to be available for the JS-land setImmediate(). + void ActivateImmediateCheck(); + private: inline void ThrowError(v8::Local (*fun)(v8::Local), const char* errmsg); @@ -753,6 +758,14 @@ class Environment { }; std::vector promise_hooks_; + struct NativeImmediateCallback { + native_immediate_callback cb_; + void* data_; + }; + std::vector native_immediate_callbacks_; + void RunAndClearNativeImmediates(); + static void CheckImmediate(uv_check_t* handle); + static void EnvPromiseHook(v8::PromiseHookType type, v8::Local promise, v8::Local parent); diff --git a/src/node.cc b/src/node.cc index 7d8d9bdc424ea1..e37f8bae72c409 100644 --- a/src/node.cc +++ b/src/node.cc @@ -3265,40 +3265,9 @@ static void DebugEnd(const FunctionCallbackInfo& args); namespace { -bool MaybeStopImmediate(Environment* env) { - if (env->scheduled_immediate_count()[0] == 0) { - uv_check_stop(env->immediate_check_handle()); - uv_idle_stop(env->immediate_idle_handle()); - return true; - } - return false; -} - -void CheckImmediate(uv_check_t* handle) { - Environment* env = Environment::from_immediate_check_handle(handle); - HandleScope scope(env->isolate()); - Context::Scope context_scope(env->context()); - - if (MaybeStopImmediate(env)) - return; - - MakeCallback(env->isolate(), - env->process_object(), - env->immediate_callback_string(), - 0, - nullptr, - {0, 0}).ToLocalChecked(); - - MaybeStopImmediate(env); -} - - void ActivateImmediateCheck(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - uv_check_start(env->immediate_check_handle(), CheckImmediate); - // Idle handle is needed only to stop the event loop from blocking in poll. - uv_idle_start(env->immediate_idle_handle(), - [](uv_idle_t*){ /* do nothing, just keep the loop running */ }); + env->ActivateImmediateCheck(); } From 590cf4af5e5f73905f20c69e4fbd7cb4c0a9401a Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 18 Nov 2017 14:54:06 +0100 Subject: [PATCH 10/22] src: remove async_hooks destroy timer handle PR-URL: https://github.com/nodejs/node/pull/17117 Reviewed-By: James M Snell --- src/async_wrap.cc | 9 ++------- src/env-inl.h | 9 --------- src/env.cc | 6 ------ src/env.h | 4 ---- test/async-hooks/test-signalwrap.js | 14 ++++++++------ test/async-hooks/test-tcpwrap.js | 6 ++++-- 6 files changed, 14 insertions(+), 34 deletions(-) diff --git a/src/async_wrap.cc b/src/async_wrap.cc index dc5f1477e21961..f024f6561f40d8 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -138,11 +138,7 @@ RetainedObjectInfo* WrapperInfo(uint16_t class_id, Local wrapper) { // end RetainedAsyncInfo -static void DestroyAsyncIdsCallback(uv_timer_t* handle) { - Environment* env = Environment::from_destroy_async_ids_timer_handle(handle); - - HandleScope handle_scope(env->isolate()); - Context::Scope context_scope(env->context()); +static void DestroyAsyncIdsCallback(Environment* env, void* data) { Local fn = env->async_hooks_destroy_function(); TryCatch try_catch(env->isolate()); @@ -690,8 +686,7 @@ void AsyncWrap::EmitDestroy(Environment* env, double async_id) { return; if (env->destroy_async_id_list()->empty()) { - uv_timer_start(env->destroy_async_ids_timer_handle(), - DestroyAsyncIdsCallback, 0, 0); + env->SetImmediate(DestroyAsyncIdsCallback, nullptr); } env->destroy_async_id_list()->push_back(async_id); diff --git a/src/env-inl.h b/src/env-inl.h index 90297361cf63bc..81086790d2cd9b 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -388,15 +388,6 @@ inline uv_idle_t* Environment::immediate_idle_handle() { return &immediate_idle_handle_; } -inline Environment* Environment::from_destroy_async_ids_timer_handle( - uv_timer_t* handle) { - return ContainerOf(&Environment::destroy_async_ids_timer_handle_, handle); -} - -inline uv_timer_t* Environment::destroy_async_ids_timer_handle() { - return &destroy_async_ids_timer_handle_; -} - inline void Environment::RegisterHandleCleanup(uv_handle_t* handle, HandleCleanupCb cb, void *arg) { diff --git a/src/env.cc b/src/env.cc index 8899fe987ccd02..3f6abd11284a11 100644 --- a/src/env.cc +++ b/src/env.cc @@ -42,8 +42,6 @@ void Environment::Start(int argc, uv_unref(reinterpret_cast(&idle_prepare_handle_)); uv_unref(reinterpret_cast(&idle_check_handle_)); - uv_timer_init(event_loop(), destroy_async_ids_timer_handle()); - auto close_and_finish = [](Environment* env, uv_handle_t* handle, void* arg) { handle->data = env; @@ -68,10 +66,6 @@ void Environment::Start(int argc, reinterpret_cast(&idle_check_handle_), close_and_finish, nullptr); - RegisterHandleCleanup( - reinterpret_cast(&destroy_async_ids_timer_handle_), - close_and_finish, - nullptr); if (start_profiler_idle_notifier) { StartProfilerIdleNotifier(); diff --git a/src/env.h b/src/env.h index 80be909442b2dd..64ddbb57b9e5f3 100644 --- a/src/env.h +++ b/src/env.h @@ -561,11 +561,8 @@ class Environment { inline uint32_t watched_providers() const; static inline Environment* from_immediate_check_handle(uv_check_t* handle); - static inline Environment* from_destroy_async_ids_timer_handle( - uv_timer_t* handle); inline uv_check_t* immediate_check_handle(); inline uv_idle_t* immediate_idle_handle(); - inline uv_timer_t* destroy_async_ids_timer_handle(); // Register clean-up cb to be called on environment destruction. inline void RegisterHandleCleanup(uv_handle_t* handle, @@ -706,7 +703,6 @@ class Environment { IsolateData* const isolate_data_; uv_check_t immediate_check_handle_; uv_idle_t immediate_idle_handle_; - uv_timer_t destroy_async_ids_timer_handle_; uv_prepare_t idle_prepare_handle_; uv_check_t idle_check_handle_; diff --git a/test/async-hooks/test-signalwrap.js b/test/async-hooks/test-signalwrap.js index ff7d08bc1207b0..fa975ff0178f3c 100644 --- a/test/async-hooks/test-signalwrap.js +++ b/test/async-hooks/test-signalwrap.js @@ -66,12 +66,14 @@ function onsigusr2() { } function onsigusr2Again() { - checkInvocations( - signal1, { init: 1, before: 2, after: 2, destroy: 1 }, - 'signal1: when second SIGUSR2 handler is called'); - checkInvocations( - signal2, { init: 1, before: 1 }, - 'signal2: when second SIGUSR2 handler is called'); + setImmediate(() => { + checkInvocations( + signal1, { init: 1, before: 2, after: 2, destroy: 1 }, + 'signal1: when second SIGUSR2 handler is called'); + checkInvocations( + signal2, { init: 1, before: 1 }, + 'signal2: when second SIGUSR2 handler is called'); + }); } process.on('exit', onexit); diff --git a/test/async-hooks/test-tcpwrap.js b/test/async-hooks/test-tcpwrap.js index 1f4fc6af0d6b5f..4693e730bfbb15 100644 --- a/test/async-hooks/test-tcpwrap.js +++ b/test/async-hooks/test-tcpwrap.js @@ -128,8 +128,10 @@ function onconnection(c) { function onserverClosed() { checkInvocations(tcp1, { init: 1, before: 1, after: 1, destroy: 1 }, 'tcp1 when server is closed'); - checkInvocations(tcp2, { init: 1, before: 2, after: 2, destroy: 1 }, - 'tcp2 when server is closed'); + setImmediate(() => { + checkInvocations(tcp2, { init: 1, before: 2, after: 2, destroy: 1 }, + 'tcp2 after server is closed'); + }); checkInvocations(tcp3, { init: 1, before: 1, after: 1 }, 'tcp3 synchronously when server is closed'); tick(2, () => { From d32d180d48f51a8ea13d4e6a6ab8843f100d6ed0 Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Fri, 24 Nov 2017 16:08:47 +0100 Subject: [PATCH 11/22] trace_events: add executionAsyncId to init events async_hooks emits trace_events. This adds the executionAsyncId to the init events. In theory this could be inferred from the before and after events but this is much simpler and doesn't require knowledge of all events. PR-URL: https://github.com/nodejs/node/pull/17196 Reviewed-By: James M Snell Reviewed-By: Franziska Hinkelmann Reviewed-By: Colin Ihrig --- lib/internal/trace_events_async_hooks.js | 4 ++- src/async_wrap.cc | 7 ++++-- src/node_trace_events.cc | 25 +++++++++++++------ .../parallel/test-trace-events-async-hooks.js | 10 +++++++- test/parallel/test-trace-events-binding.js | 19 +++++++++++--- 5 files changed, 51 insertions(+), 14 deletions(-) diff --git a/lib/internal/trace_events_async_hooks.js b/lib/internal/trace_events_async_hooks.js index 9f47f2aa5fd658..6724e0fdef5f15 100644 --- a/lib/internal/trace_events_async_hooks.js +++ b/lib/internal/trace_events_async_hooks.js @@ -28,7 +28,9 @@ const hook = async_hooks.createHook({ typeMemory.set(asyncId, type); trace_events.emit(BEFORE_EVENT, 'node.async_hooks', - type, asyncId, 'triggerAsyncId', triggerAsyncId); + type, asyncId, + 'triggerAsyncId', triggerAsyncId, + 'executionAsyncId', async_hooks.executionAsyncId()); }, before(asyncId) { diff --git a/src/async_wrap.cc b/src/async_wrap.cc index f024f6561f40d8..5f2dc5becebaf4 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -704,9 +704,12 @@ void AsyncWrap::AsyncReset(double execution_async_id, bool silent) { switch (provider_type()) { #define V(PROVIDER) \ case PROVIDER_ ## PROVIDER: \ - TRACE_EVENT_NESTABLE_ASYNC_BEGIN1("node.async_hooks", \ + TRACE_EVENT_NESTABLE_ASYNC_BEGIN2("node.async_hooks", \ #PROVIDER, static_cast(get_async_id()), \ - "triggerAsyncId", static_cast(get_trigger_async_id())); \ + "executionAsyncId", \ + static_cast(env()->execution_async_id()), \ + "triggerAsyncId", \ + static_cast(get_trigger_async_id())); \ break; NODE_ASYNC_PROVIDER_TYPES(V) #undef V diff --git a/src/node_trace_events.cc b/src/node_trace_events.cc index b0ffe68eae3f47..f269b32fbef869 100644 --- a/src/node_trace_events.cc +++ b/src/node_trace_events.cc @@ -70,20 +70,19 @@ static void Emit(const FunctionCallbackInfo& args) { id = args[3]->IntegerValue(context).ToChecked(); } - // TODO(AndreasMadsen): Two extra arguments are not supported. // TODO(AndreasMadsen): String values are not supported. int32_t num_args = 0; - const char* arg_names[1]; - uint8_t arg_types[1]; - uint64_t arg_values[1]; + const char* arg_names[2]; + uint8_t arg_types[2]; + uint64_t arg_values[2]; // Create Utf8Value in the function scope to prevent deallocation. // The c-string will be copied by TRACE_EVENT_API_ADD_TRACE_EVENT at the end. Utf8Value arg1NameValue(env->isolate(), args[4]); + Utf8Value arg2NameValue(env->isolate(), args[6]); - if (args.Length() < 6 || (args[4]->IsUndefined() && args[5]->IsUndefined())) { - num_args = 0; - } else { + if (args.Length() >= 6 && + (!args[4]->IsUndefined() || !args[5]->IsUndefined())) { num_args = 1; arg_types[0] = TRACE_VALUE_TYPE_INT; @@ -94,6 +93,18 @@ static void Emit(const FunctionCallbackInfo& args) { arg_values[0] = args[5]->IntegerValue(context).ToChecked(); } + if (args.Length() >= 8 && + (!args[6]->IsUndefined() || !args[7]->IsUndefined())) { + num_args = 2; + arg_types[1] = TRACE_VALUE_TYPE_INT; + + CHECK(args[6]->IsString()); + arg_names[1] = arg2NameValue.out(); + + CHECK(args[7]->IsNumber()); + arg_values[1] = args[7]->IntegerValue(context).ToChecked(); + } + // The TRACE_EVENT_FLAG_COPY flag indicates that the name and argument // name should be copied thus they don't need to long-lived pointers. // The category name still won't be copied and thus need to be a long-lived diff --git a/test/parallel/test-trace-events-async-hooks.js b/test/parallel/test-trace-events-async-hooks.js index 7f8fb106200b1a..e1f78f791a636d 100644 --- a/test/parallel/test-trace-events-async-hooks.js +++ b/test/parallel/test-trace-events-async-hooks.js @@ -43,7 +43,6 @@ proc.once('exit', common.mustCall(() => { return true; })); - // JavaScript async_hooks trace events should be generated. assert(traces.some((trace) => { if (trace.pid !== proc.pid) @@ -54,5 +53,14 @@ proc.once('exit', common.mustCall(() => { return false; return true; })); + + // Check args in init events + const initEvents = traces.filter((trace) => { + return (trace.ph === 'b' && !trace.name.includes('_CALLBACK')); + }); + assert(initEvents.every((trace) => { + return (trace.args.executionAsyncId > 0 && + trace.args.triggerAsyncId > 0); + })); })); })); diff --git a/test/parallel/test-trace-events-binding.js b/test/parallel/test-trace-events-binding.js index 628c9cace71973..9a182821bac18e 100644 --- a/test/parallel/test-trace-events-binding.js +++ b/test/parallel/test-trace-events-binding.js @@ -10,7 +10,10 @@ const CODE = ` 'type-value', 10, 'extra-value', 20); process.binding("trace_events").emit( 'b'.charCodeAt(0), 'custom', - 'type-value', 20); + 'type-value', 20, 'first-value', 20, 'second-value', 30); + process.binding("trace_events").emit( + 'b'.charCodeAt(0), 'custom', + 'type-value', 30); process.binding("trace_events").emit( 'b'.charCodeAt(0), 'missing', 'type-value', 10, 'extra-value', 20); @@ -29,7 +32,7 @@ proc.once('exit', common.mustCall(() => { assert(common.fileExists(FILE_NAME)); fs.readFile(FILE_NAME, common.mustCall((err, data) => { const traces = JSON.parse(data.toString()).traceEvents; - assert.strictEqual(traces.length, 2); + assert.strictEqual(traces.length, 3); assert.strictEqual(traces[0].pid, proc.pid); assert.strictEqual(traces[0].ph, 'b'); @@ -43,6 +46,16 @@ proc.once('exit', common.mustCall(() => { assert.strictEqual(traces[1].cat, 'custom'); assert.strictEqual(traces[1].name, 'type-value'); assert.strictEqual(traces[1].id, '0x14'); - assert.deepStrictEqual(traces[1].args, { }); + assert.deepStrictEqual(traces[1].args, { + 'first-value': 20, + 'second-value': 30 + }); + + assert.strictEqual(traces[2].pid, proc.pid); + assert.strictEqual(traces[2].ph, 'b'); + assert.strictEqual(traces[2].cat, 'custom'); + assert.strictEqual(traces[2].name, 'type-value'); + assert.strictEqual(traces[2].id, '0x1e'); + assert.deepStrictEqual(traces[2].args, { }); })); })); From 44cbf56783238be968ccc8618612489356eeb09c Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Mon, 20 Nov 2017 17:18:40 +0100 Subject: [PATCH 12/22] async_wrap: add provider types for net server Adds `TCPSERVERWRAP` and `PIPESERVERWRAP` as provider types. This makes it possible to distinguish servers from connections. PR-URL: #17157 Reviewed-By: James M Snell Reviewed-By: Anna Henningsen --- benchmark/net/tcp-raw-c2s.js | 6 +- benchmark/net/tcp-raw-pipe.js | 6 +- benchmark/net/tcp-raw-s2c.js | 6 +- doc/api/async_hooks.md | 12 +-- lib/_tls_wrap.js | 8 +- lib/child_process.js | 4 +- lib/internal/child_process.js | 6 +- lib/net.js | 34 +++++--- src/async_wrap.h | 2 + src/connection_wrap.cc | 4 +- src/pipe_wrap.cc | 48 +++++++++-- src/pipe_wrap.h | 11 ++- src/stream_wrap.cc | 2 +- src/tcp_wrap.cc | 41 +++++++-- src/tcp_wrap.h | 12 ++- src/udp_wrap.cc | 4 +- src/udp_wrap.h | 7 +- test/async-hooks/test-graph.pipeconnect.js | 10 +-- test/async-hooks/test-graph.shutdown.js | 12 +-- test/async-hooks/test-graph.tcp.js | 10 +-- test/async-hooks/test-graph.tls-write.js | 18 ++-- test/async-hooks/test-pipeconnectwrap.js | 44 +++++----- test/async-hooks/test-tcpwrap.js | 83 ++++++++++--------- test/common/index.js | 4 +- test/parallel/test-handle-wrap-isrefed.js | 4 +- test/parallel/test-net-connect-options-fd.js | 4 +- .../parallel/test-net-server-listen-handle.js | 8 +- test/parallel/test-process-wrap.js | 4 +- test/parallel/test-tcp-wrap-connect.js | 4 +- test/parallel/test-tcp-wrap-listen.js | 4 +- test/parallel/test-tcp-wrap.js | 4 +- test/sequential/test-async-wrap-getasyncid.js | 30 ++++--- 32 files changed, 287 insertions(+), 169 deletions(-) diff --git a/benchmark/net/tcp-raw-c2s.js b/benchmark/net/tcp-raw-c2s.js index 9b2e926d690504..bd41be87728308 100644 --- a/benchmark/net/tcp-raw-c2s.js +++ b/benchmark/net/tcp-raw-c2s.js @@ -14,7 +14,7 @@ const bench = common.createBenchmark(main, { dur: [5] }); -const TCP = process.binding('tcp_wrap').TCP; +const { TCP, constants: TCPConstants } = process.binding('tcp_wrap'); const TCPConnectWrap = process.binding('tcp_wrap').TCPConnectWrap; const WriteWrap = process.binding('stream_wrap').WriteWrap; const PORT = common.PORT; @@ -36,7 +36,7 @@ function fail(err, syscall) { } function server() { - const serverHandle = new TCP(); + const serverHandle = new TCP(TCPConstants.SERVER); var err = serverHandle.bind('127.0.0.1', PORT); if (err) fail(err, 'bind'); @@ -92,7 +92,7 @@ function client() { throw new Error(`invalid type: ${type}`); } - const clientHandle = new TCP(); + const clientHandle = new TCP(TCPConstants.SOCKET); const connectReq = new TCPConnectWrap(); const err = clientHandle.connect(connectReq, '127.0.0.1', PORT); diff --git a/benchmark/net/tcp-raw-pipe.js b/benchmark/net/tcp-raw-pipe.js index 204b27b965a340..4dd06ed446d6c1 100644 --- a/benchmark/net/tcp-raw-pipe.js +++ b/benchmark/net/tcp-raw-pipe.js @@ -14,7 +14,7 @@ const bench = common.createBenchmark(main, { dur: [5] }); -const TCP = process.binding('tcp_wrap').TCP; +const { TCP, constants: TCPConstants } = process.binding('tcp_wrap'); const TCPConnectWrap = process.binding('tcp_wrap').TCPConnectWrap; const WriteWrap = process.binding('stream_wrap').WriteWrap; const PORT = common.PORT; @@ -35,7 +35,7 @@ function fail(err, syscall) { } function server() { - const serverHandle = new TCP(); + const serverHandle = new TCP(TCPConstants.SERVER); var err = serverHandle.bind('127.0.0.1', PORT); if (err) fail(err, 'bind'); @@ -89,7 +89,7 @@ function client() { throw new Error(`invalid type: ${type}`); } - const clientHandle = new TCP(); + const clientHandle = new TCP(TCPConstants.SOCKET); const connectReq = new TCPConnectWrap(); const err = clientHandle.connect(connectReq, '127.0.0.1', PORT); var bytes = 0; diff --git a/benchmark/net/tcp-raw-s2c.js b/benchmark/net/tcp-raw-s2c.js index 412ded7355aa43..2ca6016ce017a1 100644 --- a/benchmark/net/tcp-raw-s2c.js +++ b/benchmark/net/tcp-raw-s2c.js @@ -14,7 +14,7 @@ const bench = common.createBenchmark(main, { dur: [5] }); -const TCP = process.binding('tcp_wrap').TCP; +const { TCP, constants: TCPConstants } = process.binding('tcp_wrap'); const TCPConnectWrap = process.binding('tcp_wrap').TCPConnectWrap; const WriteWrap = process.binding('stream_wrap').WriteWrap; const PORT = common.PORT; @@ -35,7 +35,7 @@ function fail(err, syscall) { } function server() { - const serverHandle = new TCP(); + const serverHandle = new TCP(TCPConstants.SERVER); var err = serverHandle.bind('127.0.0.1', PORT); if (err) fail(err, 'bind'); @@ -107,7 +107,7 @@ function server() { } function client() { - const clientHandle = new TCP(); + const clientHandle = new TCP(TCPConstants.SOCKET); const connectReq = new TCPConnectWrap(); const err = clientHandle.connect(connectReq, '127.0.0.1', PORT); diff --git a/doc/api/async_hooks.md b/doc/api/async_hooks.md index 41352820816b3f..54bae4b1387088 100644 --- a/doc/api/async_hooks.md +++ b/doc/api/async_hooks.md @@ -236,7 +236,7 @@ resource's constructor. ```text FSEVENTWRAP, FSREQWRAP, GETADDRINFOREQWRAP, GETNAMEINFOREQWRAP, HTTPPARSER, JSSTREAM, PIPECONNECTWRAP, PIPEWRAP, PROCESSWRAP, QUERYWRAP, SHUTDOWNWRAP, -SIGNALWRAP, STATWATCHER, TCPCONNECTWRAP, TCPWRAP, TIMERWRAP, TTYWRAP, +SIGNALWRAP, STATWATCHER, TCPCONNECTWRAP, TCPSERVER, TCPWRAP, TIMERWRAP, TTYWRAP, UDPSENDWRAP, UDPWRAP, WRITEWRAP, ZLIB, SSLCONNECTION, PBKDF2REQUEST, RANDOMBYTESREQUEST, TLSWRAP, Timeout, Immediate, TickObject ``` @@ -275,13 +275,13 @@ require('net').createServer((conn) => {}).listen(8080); Output when hitting the server with `nc localhost 8080`: ```console -TCPWRAP(2): trigger: 1 execution: 1 +TCPSERVERWRAP(2): trigger: 1 execution: 1 TCPWRAP(4): trigger: 2 execution: 0 ``` -The first `TCPWRAP` is the server which receives the connections. +The `TCPSERVERWRAP` is the server which receives the connections. -The second `TCPWRAP` is the new connection from the client. When a new +The `TCPWRAP` is the new connection from the client. When a new connection is made the `TCPWrap` instance is immediately constructed. This happens outside of any JavaScript stack (side note: a `executionAsyncId()` of `0` means it's being executed from C++, with no JavaScript stack above it). @@ -354,7 +354,7 @@ require('net').createServer(() => {}).listen(8080, () => { Output from only starting the server: ```console -TCPWRAP(2): trigger: 1 execution: 1 +TCPSERVERWRAP(2): trigger: 1 execution: 1 TickObject(3): trigger: 2 execution: 1 before: 3 Timeout(4): trigger: 3 execution: 3 @@ -387,7 +387,7 @@ Only using `execution` to graph resource allocation results in the following: TTYWRAP(6) -> Timeout(4) -> TIMERWRAP(5) -> TickObject(3) -> root(1) ``` -The `TCPWRAP` is not part of this graph, even though it was the reason for +The `TCPSERVERWRAP` is not part of this graph, even though it was the reason for `console.log()` being called. This is because binding to a port without a hostname is a *synchronous* operation, but to maintain a completely asynchronous API the user's callback is placed in a `process.nextTick()`. diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index ea9d857939d918..598bfaa5ae8c3e 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -34,8 +34,8 @@ const { Buffer } = require('buffer'); const debug = util.debuglog('tls'); const { Timer } = process.binding('timer_wrap'); const tls_wrap = process.binding('tls_wrap'); -const { TCP } = process.binding('tcp_wrap'); -const { Pipe } = process.binding('pipe_wrap'); +const { TCP, constants: TCPConstants } = process.binding('tcp_wrap'); +const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap'); const kDisableRenegotiation = Symbol('disable-renegotiation'); function onhandshakestart() { @@ -376,7 +376,9 @@ TLSSocket.prototype._wrapHandle = function(wrap) { var options = this._tlsOptions; if (!handle) { - handle = options.pipe ? new Pipe() : new TCP(); + handle = options.pipe ? + new Pipe(PipeConstants.SOCKET) : + new TCP(TCPConstants.SOCKET); handle.owner = this; } diff --git a/lib/child_process.js b/lib/child_process.js index 545ee3887db6df..2e6360d42b9246 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -31,7 +31,7 @@ const debug = util.debuglog('child_process'); const uv = process.binding('uv'); const spawn_sync = process.binding('spawn_sync'); const { Buffer } = require('buffer'); -const { Pipe } = process.binding('pipe_wrap'); +const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap'); const child_process = require('internal/child_process'); const { _validateStdio, @@ -106,7 +106,7 @@ exports.fork = function(modulePath /*, args, options*/) { exports._forkChild = function(fd) { // set process.send() - var p = new Pipe(true); + var p = new Pipe(PipeConstants.IPC); p.open(fd); p.unref(); const control = setupChannel(process, p); diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index 49975f3347a1c4..726a83c0734473 100644 --- a/lib/internal/child_process.js +++ b/lib/internal/child_process.js @@ -10,7 +10,7 @@ const assert = require('assert'); const uv = process.binding('uv'); const { Process } = process.binding('process_wrap'); const { WriteWrap } = process.binding('stream_wrap'); -const { Pipe } = process.binding('pipe_wrap'); +const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap'); const { TTY } = process.binding('tty_wrap'); const { TCP } = process.binding('tcp_wrap'); const { UDP } = process.binding('udp_wrap'); @@ -845,7 +845,7 @@ function _validateStdio(stdio, sync) { }; if (!sync) - a.handle = new Pipe(); + a.handle = new Pipe(PipeConstants.SOCKET); acc.push(a); } else if (stdio === 'ipc') { @@ -858,7 +858,7 @@ function _validateStdio(stdio, sync) { throw new errors.Error('ERR_IPC_SYNC_FORK'); } - ipc = new Pipe(true); + ipc = new Pipe(PipeConstants.IPC); ipcFd = i; acc.push({ diff --git a/lib/net.js b/lib/net.js index 43ce4d803e03af..d34a9dead3b7e5 100644 --- a/lib/net.js +++ b/lib/net.js @@ -33,8 +33,8 @@ const uv = process.binding('uv'); const { Buffer } = require('buffer'); const TTYWrap = process.binding('tty_wrap'); -const { TCP } = process.binding('tcp_wrap'); -const { Pipe } = process.binding('pipe_wrap'); +const { TCP, constants: TCPConstants } = process.binding('tcp_wrap'); +const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap'); const { TCPConnectWrap } = process.binding('tcp_wrap'); const { PipeConnectWrap } = process.binding('pipe_wrap'); const { ShutdownWrap, WriteWrap } = process.binding('stream_wrap'); @@ -53,10 +53,20 @@ const exceptionWithHostPort = util._exceptionWithHostPort; function noop() {} -function createHandle(fd) { - var type = TTYWrap.guessHandleType(fd); - if (type === 'PIPE') return new Pipe(); - if (type === 'TCP') return new TCP(); +function createHandle(fd, is_server) { + const type = TTYWrap.guessHandleType(fd); + if (type === 'PIPE') { + return new Pipe( + is_server ? PipeConstants.SERVER : PipeConstants.SOCKET + ); + } + + if (type === 'TCP') { + return new TCP( + is_server ? TCPConstants.SERVER : TCPConstants.SOCKET + ); + } + throw new TypeError('Unsupported fd type: ' + type); } @@ -196,7 +206,7 @@ function Socket(options) { this._handle = options.handle; // private this[async_id_symbol] = getNewAsyncId(this._handle); } else if (options.fd !== undefined) { - this._handle = createHandle(options.fd); + this._handle = createHandle(options.fd, false); this._handle.open(options.fd); this[async_id_symbol] = this._handle.getAsyncId(); // options.fd can be string (since it is user-defined), @@ -1003,7 +1013,9 @@ Socket.prototype.connect = function(...args) { debug('pipe', pipe, path); if (!this._handle) { - this._handle = pipe ? new Pipe() : new TCP(); + this._handle = pipe ? + new Pipe(PipeConstants.SOCKET) : + new TCP(TCPConstants.SOCKET); initSocketHandle(this); } @@ -1253,7 +1265,7 @@ function createServerHandle(address, port, addressType, fd) { var isTCP = false; if (typeof fd === 'number' && fd >= 0) { try { - handle = createHandle(fd); + handle = createHandle(fd, true); } catch (e) { // Not a fd we can listen on. This will trigger an error. debug('listen invalid fd=%d:', fd, e.message); @@ -1264,7 +1276,7 @@ function createServerHandle(address, port, addressType, fd) { handle.writable = true; assert(!address && !port); } else if (port === -1 && addressType === -1) { - handle = new Pipe(); + handle = new Pipe(PipeConstants.SERVER); if (process.platform === 'win32') { var instances = parseInt(process.env.NODE_PENDING_PIPE_INSTANCES); if (!isNaN(instances)) { @@ -1272,7 +1284,7 @@ function createServerHandle(address, port, addressType, fd) { } } } else { - handle = new TCP(); + handle = new TCP(TCPConstants.SERVER); isTCP = true; } diff --git a/src/async_wrap.h b/src/async_wrap.h index e6e717c24ed71d..773a0ad15ce9e9 100644 --- a/src/async_wrap.h +++ b/src/async_wrap.h @@ -47,6 +47,7 @@ namespace node { V(HTTPPARSER) \ V(JSSTREAM) \ V(PIPECONNECTWRAP) \ + V(PIPESERVERWRAP) \ V(PIPEWRAP) \ V(PROCESSWRAP) \ V(PROMISE) \ @@ -55,6 +56,7 @@ namespace node { V(SIGNALWRAP) \ V(STATWATCHER) \ V(TCPCONNECTWRAP) \ + V(TCPSERVERWRAP) \ V(TCPWRAP) \ V(TIMERWRAP) \ V(TTYWRAP) \ diff --git a/src/connection_wrap.cc b/src/connection_wrap.cc index b620c387ff1cfc..d82e7195d76579 100644 --- a/src/connection_wrap.cc +++ b/src/connection_wrap.cc @@ -51,7 +51,9 @@ void ConnectionWrap::OnConnection(uv_stream_t* handle, if (status == 0) { env->set_init_trigger_async_id(wrap_data->get_async_id()); // Instantiate the client javascript object and handle. - Local client_obj = WrapType::Instantiate(env, wrap_data); + Local client_obj = WrapType::Instantiate(env, + wrap_data, + WrapType::SOCKET); // Unwrap the client javascript object. WrapType* wrap; diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc index 4cdf7dc6e6b5c6..281ac4d6838dbc 100644 --- a/src/pipe_wrap.cc +++ b/src/pipe_wrap.cc @@ -39,6 +39,7 @@ using v8::Function; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; using v8::HandleScope; +using v8::Int32; using v8::Local; using v8::Object; using v8::String; @@ -47,14 +48,17 @@ using v8::Value; using AsyncHooks = Environment::AsyncHooks; -Local PipeWrap::Instantiate(Environment* env, AsyncWrap* parent) { +Local PipeWrap::Instantiate(Environment* env, + AsyncWrap* parent, + PipeWrap::SocketType type) { EscapableHandleScope handle_scope(env->isolate()); AsyncHooks::InitScope init_scope(env, parent->get_async_id()); CHECK_EQ(false, env->pipe_constructor_template().IsEmpty()); Local constructor = env->pipe_constructor_template()->GetFunction(); CHECK_EQ(false, constructor.IsEmpty()); + Local type_value = Int32::New(env->isolate(), type); Local instance = - constructor->NewInstance(env->context()).ToLocalChecked(); + constructor->NewInstance(env->context(), 1, &type_value).ToLocalChecked(); return handle_scope.Escape(instance); } @@ -106,6 +110,15 @@ void PipeWrap::Initialize(Local target, FIXED_ONE_BYTE_STRING(env->isolate(), "PipeConnectWrap"); cwt->SetClassName(wrapString); target->Set(wrapString, cwt->GetFunction()); + + // Define constants + Local constants = Object::New(env->isolate()); + NODE_DEFINE_CONSTANT(constants, SOCKET); + NODE_DEFINE_CONSTANT(constants, SERVER); + NODE_DEFINE_CONSTANT(constants, IPC); + target->Set(context, + FIXED_ONE_BYTE_STRING(env->isolate(), "constants"), + constants).FromJust(); } @@ -114,17 +127,40 @@ void PipeWrap::New(const FunctionCallbackInfo& args) { // Therefore we assert that we are not trying to call this as a // normal function. CHECK(args.IsConstructCall()); + CHECK(args[0]->IsInt32()); Environment* env = Environment::GetCurrent(args); - new PipeWrap(env, args.This(), args[0]->IsTrue()); + + int type_value = args[0].As()->Value(); + PipeWrap::SocketType type = static_cast(type_value); + + bool ipc; + ProviderType provider; + switch (type) { + case SOCKET: + provider = PROVIDER_PIPEWRAP; + ipc = false; + break; + case SERVER: + provider = PROVIDER_PIPESERVERWRAP; + ipc = false; + break; + case IPC: + provider = PROVIDER_PIPEWRAP; + ipc = true; + break; + default: + UNREACHABLE(); + } + + new PipeWrap(env, args.This(), provider, ipc); } PipeWrap::PipeWrap(Environment* env, Local object, + ProviderType provider, bool ipc) - : ConnectionWrap(env, - object, - AsyncWrap::PROVIDER_PIPEWRAP) { + : ConnectionWrap(env, object, provider) { int r = uv_pipe_init(env->event_loop(), &handle_, ipc); CHECK_EQ(r, 0); // How do we proxy this error up to javascript? // Suggestion: uv_pipe_init() returns void. diff --git a/src/pipe_wrap.h b/src/pipe_wrap.h index 6f22038b918f76..5a611c0f94b639 100644 --- a/src/pipe_wrap.h +++ b/src/pipe_wrap.h @@ -32,7 +32,15 @@ namespace node { class PipeWrap : public ConnectionWrap { public: - static v8::Local Instantiate(Environment* env, AsyncWrap* parent); + enum SocketType { + SOCKET, + SERVER, + IPC + }; + + static v8::Local Instantiate(Environment* env, + AsyncWrap* parent, + SocketType type); static void Initialize(v8::Local target, v8::Local unused, v8::Local context); @@ -42,6 +50,7 @@ class PipeWrap : public ConnectionWrap { private: PipeWrap(Environment* env, v8::Local object, + ProviderType provider, bool ipc); static void New(const v8::FunctionCallbackInfo& args); diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index b089f1506746dc..8516e39f295f03 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -188,7 +188,7 @@ static Local AcceptHandle(Environment* env, LibuvStreamWrap* parent) { Local wrap_obj; UVType* handle; - wrap_obj = WrapType::Instantiate(env, parent); + wrap_obj = WrapType::Instantiate(env, parent, WrapType::SOCKET); if (wrap_obj.IsEmpty()) return Local(); diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index af64f89b546079..8dd14e2e16c18b 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -42,6 +42,7 @@ using v8::Function; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; using v8::HandleScope; +using v8::Int32; using v8::Integer; using v8::Local; using v8::Object; @@ -51,14 +52,17 @@ using v8::Value; using AsyncHooks = Environment::AsyncHooks; -Local TCPWrap::Instantiate(Environment* env, AsyncWrap* parent) { +Local TCPWrap::Instantiate(Environment* env, + AsyncWrap* parent, + TCPWrap::SocketType type) { EscapableHandleScope handle_scope(env->isolate()); AsyncHooks::InitScope init_scope(env, parent->get_async_id()); CHECK_EQ(env->tcp_constructor_template().IsEmpty(), false); Local constructor = env->tcp_constructor_template()->GetFunction(); CHECK_EQ(constructor.IsEmpty(), false); + Local type_value = Int32::New(env->isolate(), type); Local instance = - constructor->NewInstance(env->context()).ToLocalChecked(); + constructor->NewInstance(env->context(), 1, &type_value).ToLocalChecked(); return handle_scope.Escape(instance); } @@ -122,6 +126,14 @@ void TCPWrap::Initialize(Local target, FIXED_ONE_BYTE_STRING(env->isolate(), "TCPConnectWrap"); cwt->SetClassName(wrapString); target->Set(wrapString, cwt->GetFunction()); + + // Define constants + Local constants = Object::New(env->isolate()); + NODE_DEFINE_CONSTANT(constants, SOCKET); + NODE_DEFINE_CONSTANT(constants, SERVER); + target->Set(context, + FIXED_ONE_BYTE_STRING(env->isolate(), "constants"), + constants).FromJust(); } @@ -130,15 +142,30 @@ void TCPWrap::New(const FunctionCallbackInfo& args) { // Therefore we assert that we are not trying to call this as a // normal function. CHECK(args.IsConstructCall()); + CHECK(args[0]->IsInt32()); Environment* env = Environment::GetCurrent(args); - new TCPWrap(env, args.This()); + + int type_value = args[0].As()->Value(); + TCPWrap::SocketType type = static_cast(type_value); + + ProviderType provider; + switch (type) { + case SOCKET: + provider = PROVIDER_TCPWRAP; + break; + case SERVER: + provider = PROVIDER_TCPSERVERWRAP; + break; + default: + UNREACHABLE(); + } + + new TCPWrap(env, args.This(), provider); } -TCPWrap::TCPWrap(Environment* env, Local object) - : ConnectionWrap(env, - object, - AsyncWrap::PROVIDER_TCPWRAP) { +TCPWrap::TCPWrap(Environment* env, Local object, ProviderType provider) + : ConnectionWrap(env, object, provider) { int r = uv_tcp_init(env->event_loop(), &handle_); CHECK_EQ(r, 0); // How do we proxy this error up to javascript? // Suggestion: uv_tcp_init() returns void. diff --git a/src/tcp_wrap.h b/src/tcp_wrap.h index fa6bac01386256..a7f6b1901981f6 100644 --- a/src/tcp_wrap.h +++ b/src/tcp_wrap.h @@ -32,7 +32,14 @@ namespace node { class TCPWrap : public ConnectionWrap { public: - static v8::Local Instantiate(Environment* env, AsyncWrap* parent); + enum SocketType { + SOCKET, + SERVER + }; + + static v8::Local Instantiate(Environment* env, + AsyncWrap* parent, + SocketType type); static void Initialize(v8::Local target, v8::Local unused, v8::Local context); @@ -46,7 +53,8 @@ class TCPWrap : public ConnectionWrap { int (*F)(const typename T::HandleType*, sockaddr*, int*)> friend void GetSockOrPeerName(const v8::FunctionCallbackInfo&); - TCPWrap(Environment* env, v8::Local object); + TCPWrap(Environment* env, v8::Local object, + ProviderType provider); ~TCPWrap(); static void New(const v8::FunctionCallbackInfo& args); diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index 73a976057719fa..4a9eefb9499606 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -493,7 +493,9 @@ void UDPWrap::OnRecv(uv_udp_t* handle, } -Local UDPWrap::Instantiate(Environment* env, AsyncWrap* parent) { +Local UDPWrap::Instantiate(Environment* env, + AsyncWrap* parent, + UDPWrap::SocketType type) { EscapableHandleScope scope(env->isolate()); AsyncHooks::InitScope init_scope(env, parent->get_async_id()); // If this assert fires then Initialize hasn't been called yet. diff --git a/src/udp_wrap.h b/src/udp_wrap.h index 50a7455beb6a9e..f4cf3ad7f566df 100644 --- a/src/udp_wrap.h +++ b/src/udp_wrap.h @@ -35,6 +35,9 @@ namespace node { class UDPWrap: public HandleWrap { public: + enum SocketType { + SOCKET + }; static void Initialize(v8::Local target, v8::Local unused, v8::Local context); @@ -58,7 +61,9 @@ class UDPWrap: public HandleWrap { static void SetTTL(const v8::FunctionCallbackInfo& args); static void BufferSize(const v8::FunctionCallbackInfo& args); - static v8::Local Instantiate(Environment* env, AsyncWrap* parent); + static v8::Local Instantiate(Environment* env, + AsyncWrap* parent, + SocketType type); uv_udp_t* UVHandle(); size_t self_size() const override { return sizeof(*this); } diff --git a/test/async-hooks/test-graph.pipeconnect.js b/test/async-hooks/test-graph.pipeconnect.js index a3486521d5a7ea..b3ea5c6e4219e9 100644 --- a/test/async-hooks/test-graph.pipeconnect.js +++ b/test/async-hooks/test-graph.pipeconnect.js @@ -28,11 +28,11 @@ function onexit() { hooks.disable(); verifyGraph( hooks, - [ { type: 'PIPEWRAP', id: 'pipe:1', triggerAsyncId: null }, - { type: 'PIPEWRAP', id: 'pipe:2', triggerAsyncId: 'pipe:1' }, + [ { type: 'PIPESERVERWRAP', id: 'pipeserver:1', triggerAsyncId: null }, + { type: 'PIPEWRAP', id: 'pipe:1', triggerAsyncId: 'pipeserver:1' }, { type: 'PIPECONNECTWRAP', id: 'pipeconnect:1', - triggerAsyncId: 'pipe:2' }, - { type: 'PIPEWRAP', id: 'pipe:3', triggerAsyncId: 'pipe:1' }, - { type: 'SHUTDOWNWRAP', id: 'shutdown:1', triggerAsyncId: 'pipe:3' } ] + triggerAsyncId: 'pipe:1' }, + { type: 'PIPEWRAP', id: 'pipe:2', triggerAsyncId: 'pipeserver:1' }, + { type: 'SHUTDOWNWRAP', id: 'shutdown:1', triggerAsyncId: 'pipe:2' } ] ); } diff --git a/test/async-hooks/test-graph.shutdown.js b/test/async-hooks/test-graph.shutdown.js index 136f01821217c1..f4f077c0dd1c83 100644 --- a/test/async-hooks/test-graph.shutdown.js +++ b/test/async-hooks/test-graph.shutdown.js @@ -34,13 +34,13 @@ function onexit() { hooks.disable(); verifyGraph( hooks, - [ { type: 'TCPWRAP', id: 'tcp:1', triggerAsyncId: null }, - { type: 'TCPWRAP', id: 'tcp:2', triggerAsyncId: 'tcp:1' }, + [ { type: 'TCPSERVERWRAP', id: 'tcpserver:1', triggerAsyncId: null }, + { type: 'TCPWRAP', id: 'tcp:1', triggerAsyncId: 'tcpserver:1' }, { type: 'GETADDRINFOREQWRAP', - id: 'getaddrinforeq:1', triggerAsyncId: 'tcp:2' }, + id: 'getaddrinforeq:1', triggerAsyncId: 'tcp:1' }, { type: 'TCPCONNECTWRAP', - id: 'tcpconnect:1', triggerAsyncId: 'tcp:2' }, - { type: 'TCPWRAP', id: 'tcp:3', triggerAsyncId: 'tcp:1' }, - { type: 'SHUTDOWNWRAP', id: 'shutdown:1', triggerAsyncId: 'tcp:3' } ] + id: 'tcpconnect:1', triggerAsyncId: 'tcp:1' }, + { type: 'TCPWRAP', id: 'tcp:2', triggerAsyncId: 'tcpserver:1' }, + { type: 'SHUTDOWNWRAP', id: 'shutdown:1', triggerAsyncId: 'tcp:2' } ] ); } diff --git a/test/async-hooks/test-graph.tcp.js b/test/async-hooks/test-graph.tcp.js index 2e0b387cbeaff4..c2458ef1def769 100644 --- a/test/async-hooks/test-graph.tcp.js +++ b/test/async-hooks/test-graph.tcp.js @@ -38,11 +38,11 @@ function onexit() { verifyGraph( hooks, - [ { type: 'TCPWRAP', id: 'tcp:1', triggerAsyncId: null }, - { type: 'TCPWRAP', id: 'tcp:2', triggerAsyncId: null }, + [ { type: 'TCPSERVERWRAP', id: 'tcpserver:1', triggerAsyncId: null }, + { type: 'TCPWRAP', id: 'tcp:1', triggerAsyncId: null }, { type: 'TCPCONNECTWRAP', - id: 'tcpconnect:1', triggerAsyncId: 'tcp:2' }, - { type: 'TCPWRAP', id: 'tcp:3', triggerAsyncId: 'tcp:1' }, - { type: 'SHUTDOWNWRAP', id: 'shutdown:1', triggerAsyncId: 'tcp:3' } ] + id: 'tcpconnect:1', triggerAsyncId: 'tcp:1' }, + { type: 'TCPWRAP', id: 'tcp:2', triggerAsyncId: 'tcpserver:1' }, + { type: 'SHUTDOWNWRAP', id: 'shutdown:1', triggerAsyncId: 'tcp:2' } ] ); } diff --git a/test/async-hooks/test-graph.tls-write.js b/test/async-hooks/test-graph.tls-write.js index 4ba264c808392e..0c725d153d731b 100644 --- a/test/async-hooks/test-graph.tls-write.js +++ b/test/async-hooks/test-graph.tls-write.js @@ -55,21 +55,21 @@ function onexit() { verifyGraph( hooks, - [ { type: 'TCPWRAP', id: 'tcp:1', triggerAsyncId: null }, - { type: 'TCPWRAP', id: 'tcp:2', triggerAsyncId: 'tcp:1' }, - { type: 'TLSWRAP', id: 'tls:1', triggerAsyncId: 'tcp:1' }, + [ { type: 'TCPSERVERWRAP', id: 'tcpserver:1', triggerAsyncId: null }, + { type: 'TCPWRAP', id: 'tcp:1', triggerAsyncId: 'tcpserver:1' }, + { type: 'TLSWRAP', id: 'tls:1', triggerAsyncId: 'tcpserver:1' }, { type: 'GETADDRINFOREQWRAP', id: 'getaddrinforeq:1', triggerAsyncId: 'tls:1' }, { type: 'TCPCONNECTWRAP', - id: 'tcpconnect:1', triggerAsyncId: 'tcp:2' }, + id: 'tcpconnect:1', triggerAsyncId: 'tcp:1' }, { type: 'WRITEWRAP', id: 'write:1', triggerAsyncId: 'tcpconnect:1' }, - { type: 'TCPWRAP', id: 'tcp:3', triggerAsyncId: 'tcp:1' }, - { type: 'TLSWRAP', id: 'tls:2', triggerAsyncId: 'tcp:1' }, - { type: 'TIMERWRAP', id: 'timer:1', triggerAsyncId: 'tcp:1' }, + { type: 'TCPWRAP', id: 'tcp:2', triggerAsyncId: 'tcpserver:1' }, + { type: 'TLSWRAP', id: 'tls:2', triggerAsyncId: 'tcpserver:1' }, + { type: 'TIMERWRAP', id: 'timer:1', triggerAsyncId: 'tcpserver:1' }, { type: 'WRITEWRAP', id: 'write:2', triggerAsyncId: null }, { type: 'WRITEWRAP', id: 'write:3', triggerAsyncId: null }, { type: 'WRITEWRAP', id: 'write:4', triggerAsyncId: null }, - { type: 'Immediate', id: 'immediate:1', triggerAsyncId: 'tcp:2' }, - { type: 'Immediate', id: 'immediate:2', triggerAsyncId: 'tcp:3' } ] + { type: 'Immediate', id: 'immediate:1', triggerAsyncId: 'tcp:1' }, + { type: 'Immediate', id: 'immediate:2', triggerAsyncId: 'tcp:2' } ] ); } diff --git a/test/async-hooks/test-pipeconnectwrap.js b/test/async-hooks/test-pipeconnectwrap.js index bcab601d05952f..81a5abd42a3004 100644 --- a/test/async-hooks/test-pipeconnectwrap.js +++ b/test/async-hooks/test-pipeconnectwrap.js @@ -12,7 +12,8 @@ common.refreshTmpDir(); const hooks = initHooks(); hooks.enable(); -let pipe1, pipe2, pipe3; +let pipe1, pipe2; +let pipeserver; let pipeconnect; net.createServer(common.mustCall(function(c) { @@ -22,27 +23,27 @@ net.createServer(common.mustCall(function(c) { })).listen(common.PIPE, common.mustCall(onlisten)); function onlisten() { - let pipes = hooks.activitiesOfTypes('PIPEWRAP'); + const pipeservers = hooks.activitiesOfTypes('PIPESERVERWRAP'); let pipeconnects = hooks.activitiesOfTypes('PIPECONNECTWRAP'); - assert.strictEqual(pipes.length, 1); + assert.strictEqual(pipeservers.length, 1); assert.strictEqual(pipeconnects.length, 0); net.connect(common.PIPE, common.mustCall(maybeOnconnect.bind(null, 'client'))); - pipes = hooks.activitiesOfTypes('PIPEWRAP'); + const pipes = hooks.activitiesOfTypes('PIPEWRAP'); pipeconnects = hooks.activitiesOfTypes('PIPECONNECTWRAP'); - assert.strictEqual(pipes.length, 2); + assert.strictEqual(pipes.length, 1); assert.strictEqual(pipeconnects.length, 1); + pipeserver = pipeservers[0]; pipe1 = pipes[0]; - pipe2 = pipes[1]; pipeconnect = pipeconnects[0]; + assert.strictEqual(pipeserver.type, 'PIPESERVERWRAP'); assert.strictEqual(pipe1.type, 'PIPEWRAP'); - assert.strictEqual(pipe2.type, 'PIPEWRAP'); assert.strictEqual(pipeconnect.type, 'PIPECONNECTWRAP'); - for (const a of [ pipe1, pipe2, pipeconnect ]) { + for (const a of [ pipeserver, pipe1, pipeconnect ]) { assert.strictEqual(typeof a.uid, 'number'); assert.strictEqual(typeof a.triggerAsyncId, 'number'); checkInvocations(a, { init: 1 }, 'after net.connect'); @@ -60,18 +61,18 @@ function maybeOnconnect(source) { const pipes = hooks.activitiesOfTypes('PIPEWRAP'); const pipeconnects = hooks.activitiesOfTypes('PIPECONNECTWRAP'); - assert.strictEqual(pipes.length, 3); + assert.strictEqual(pipes.length, 2); assert.strictEqual(pipeconnects.length, 1); - pipe3 = pipes[2]; - assert.strictEqual(typeof pipe3.uid, 'number'); - assert.strictEqual(typeof pipe3.triggerAsyncId, 'number'); + pipe2 = pipes[1]; + assert.strictEqual(typeof pipe2.uid, 'number'); + assert.strictEqual(typeof pipe2.triggerAsyncId, 'number'); - checkInvocations(pipe1, { init: 1, before: 1, after: 1 }, - 'pipe1, client connected'); - checkInvocations(pipe2, { init: 1 }, 'pipe2, client connected'); + checkInvocations(pipeserver, { init: 1, before: 1, after: 1 }, + 'pipeserver, client connected'); + checkInvocations(pipe1, { init: 1 }, 'pipe1, client connected'); checkInvocations(pipeconnect, { init: 1, before: 1 }, 'pipeconnect, client connected'); - checkInvocations(pipe3, { init: 1 }, 'pipe3, client connected'); + checkInvocations(pipe2, { init: 1 }, 'pipe2, client connected'); tick(5); } @@ -80,14 +81,15 @@ process.on('exit', onexit); function onexit() { hooks.disable(); hooks.sanityCheck('PIPEWRAP'); + hooks.sanityCheck('PIPESERVERWRAP'); hooks.sanityCheck('PIPECONNECTWRAP'); // TODO(thlorenz) why have some of those 'before' and 'after' called twice - checkInvocations(pipe1, { init: 1, before: 1, after: 1, destroy: 1 }, + checkInvocations(pipeserver, { init: 1, before: 1, after: 1, destroy: 1 }, + 'pipeserver, process exiting'); + checkInvocations(pipe1, { init: 1, before: 2, after: 2, destroy: 1 }, 'pipe1, process exiting'); - checkInvocations(pipe2, { init: 1, before: 2, after: 2, destroy: 1 }, - 'pipe2, process exiting'); checkInvocations(pipeconnect, { init: 1, before: 1, after: 1, destroy: 1 }, 'pipeconnect, process exiting'); - checkInvocations(pipe3, { init: 1, before: 2, after: 2, destroy: 1 }, - 'pipe3, process exiting'); + checkInvocations(pipe2, { init: 1, before: 2, after: 2, destroy: 1 }, + 'pipe2, process exiting'); } diff --git a/test/async-hooks/test-tcpwrap.js b/test/async-hooks/test-tcpwrap.js index 4693e730bfbb15..e7d879caf70551 100644 --- a/test/async-hooks/test-tcpwrap.js +++ b/test/async-hooks/test-tcpwrap.js @@ -11,7 +11,8 @@ const initHooks = require('./init-hooks'); const { checkInvocations } = require('./hook-checks'); const net = require('net'); -let tcp1, tcp2, tcp3; +let tcp1, tcp2; +let tcpserver; let tcpconnect; const hooks = initHooks(); @@ -24,15 +25,15 @@ const server = net // Calling server.listen creates a TCPWRAP synchronously { server.listen(common.PORT); - const tcps = hooks.activitiesOfTypes('TCPWRAP'); + const tcpsservers = hooks.activitiesOfTypes('TCPSERVERWRAP'); const tcpconnects = hooks.activitiesOfTypes('TCPCONNECTWRAP'); - assert.strictEqual(tcps.length, 1); + assert.strictEqual(tcpsservers.length, 1); assert.strictEqual(tcpconnects.length, 0); - tcp1 = tcps[0]; - assert.strictEqual(tcp1.type, 'TCPWRAP'); - assert.strictEqual(typeof tcp1.uid, 'number'); - assert.strictEqual(typeof tcp1.triggerAsyncId, 'number'); - checkInvocations(tcp1, { init: 1 }, 'when calling server.listen'); + tcpserver = tcpsservers[0]; + assert.strictEqual(tcpserver.type, 'TCPSERVERWRAP'); + assert.strictEqual(typeof tcpserver.uid, 'number'); + assert.strictEqual(typeof tcpserver.triggerAsyncId, 'number'); + checkInvocations(tcpserver, { init: 1 }, 'when calling server.listen'); } // Calling net.connect creates another TCPWRAP synchronously @@ -41,24 +42,25 @@ const server = net { port: server.address().port, host: '::1' }, common.mustCall(onconnected)); const tcps = hooks.activitiesOfTypes('TCPWRAP'); - assert.strictEqual(tcps.length, 2); + assert.strictEqual(tcps.length, 1); process.nextTick(() => { const tcpconnects = hooks.activitiesOfTypes('TCPCONNECTWRAP'); assert.strictEqual(tcpconnects.length, 1); }); - tcp2 = tcps[1]; - assert.strictEqual(tcps.length, 2); - assert.strictEqual(tcp2.type, 'TCPWRAP'); - assert.strictEqual(typeof tcp2.uid, 'number'); - assert.strictEqual(typeof tcp2.triggerAsyncId, 'number'); + tcp1 = tcps[0]; + assert.strictEqual(tcps.length, 1); + assert.strictEqual(tcp1.type, 'TCPWRAP'); + assert.strictEqual(typeof tcp1.uid, 'number'); + assert.strictEqual(typeof tcp1.triggerAsyncId, 'number'); + checkInvocations(tcpserver, { init: 1 }, + 'tcpserver when client is connecting'); checkInvocations(tcp1, { init: 1 }, 'tcp1 when client is connecting'); - checkInvocations(tcp2, { init: 1 }, 'tcp2 when client is connecting'); } function onlistening() { - assert.strictEqual(hooks.activitiesOfTypes('TCPWRAP').length, 2); + assert.strictEqual(hooks.activitiesOfTypes('TCPWRAP').length, 1); } // Depending on timing we see client: onconnected or server: onconnection first @@ -99,8 +101,8 @@ function onconnected() { const expected = serverConnected ? { init: 1, before: 1, after: 1 } : { init: 1 }; - checkInvocations(tcp1, expected, 'tcp1 when client connects'); - checkInvocations(tcp2, { init: 1 }, 'tcp2 when client connects'); + checkInvocations(tcpserver, expected, 'tcpserver when client connects'); + checkInvocations(tcp1, { init: 1 }, 'tcp1 when client connects'); } function onconnection(c) { @@ -109,34 +111,35 @@ function onconnection(c) { const tcps = hooks.activitiesOfTypes([ 'TCPWRAP' ]); const tcpconnects = hooks.activitiesOfTypes('TCPCONNECTWRAP'); - assert.strictEqual(tcps.length, 3); + assert.strictEqual(tcps.length, 2); assert.strictEqual(tcpconnects.length, 1); - tcp3 = tcps[2]; - assert.strictEqual(tcp3.type, 'TCPWRAP'); - assert.strictEqual(typeof tcp3.uid, 'number'); - assert.strictEqual(typeof tcp3.triggerAsyncId, 'number'); + tcp2 = tcps[1]; + assert.strictEqual(tcp2.type, 'TCPWRAP'); + assert.strictEqual(typeof tcp2.uid, 'number'); + assert.strictEqual(typeof tcp2.triggerAsyncId, 'number'); - checkInvocations(tcp1, { init: 1, before: 1 }, - 'tcp1 when server receives connection'); + checkInvocations(tcpserver, { init: 1, before: 1 }, + 'tcpserver when server receives connection'); + checkInvocations(tcp1, { init: 1 }, 'tcp1 when server receives connection'); checkInvocations(tcp2, { init: 1 }, 'tcp2 when server receives connection'); - checkInvocations(tcp3, { init: 1 }, 'tcp3 when server receives connection'); c.end(); this.close(common.mustCall(onserverClosed)); } function onserverClosed() { - checkInvocations(tcp1, { init: 1, before: 1, after: 1, destroy: 1 }, - 'tcp1 when server is closed'); + checkInvocations(tcpserver, { init: 1, before: 1, after: 1, destroy: 1 }, + 'tcpserver when server is closed'); setImmediate(() => { - checkInvocations(tcp2, { init: 1, before: 2, after: 2, destroy: 1 }, - 'tcp2 after server is closed'); + checkInvocations(tcp1, { init: 1, before: 2, after: 2, destroy: 1 }, + 'tcp1 after server is closed'); }); - checkInvocations(tcp3, { init: 1, before: 1, after: 1 }, - 'tcp3 synchronously when server is closed'); + checkInvocations(tcp2, { init: 1, before: 1, after: 1 }, + 'tcp2 synchronously when server is closed'); + tick(2, () => { - checkInvocations(tcp3, { init: 1, before: 2, after: 2, destroy: 1 }, - 'tcp3 when server is closed'); + checkInvocations(tcp2, { init: 1, before: 2, after: 2, destroy: 1 }, + 'tcp2 when server is closed'); checkInvocations(tcpconnect, { init: 1, before: 1, after: 1, destroy: 1 }, 'tcpconnect when server is closed'); }); @@ -146,16 +149,16 @@ process.on('exit', onexit); function onexit() { hooks.disable(); - hooks.sanityCheck([ 'TCPWRAP', 'TCPCONNECTWRAP' ]); + hooks.sanityCheck([ 'TCPWRAP', 'TCPSERVERWRAP', 'TCPCONNECTWRAP' ]); - checkInvocations(tcp1, { init: 1, before: 1, after: 1, destroy: 1 }, - 'tcp1 when process exits'); + checkInvocations(tcpserver, { init: 1, before: 1, after: 1, destroy: 1 }, + 'tcpserver when process exits'); + checkInvocations( + tcp1, { init: 1, before: 2, after: 2, destroy: 1 }, + 'tcp1 when process exits'); checkInvocations( tcp2, { init: 1, before: 2, after: 2, destroy: 1 }, 'tcp2 when process exits'); - checkInvocations( - tcp3, { init: 1, before: 2, after: 2, destroy: 1 }, - 'tcp3 when process exits'); checkInvocations( tcpconnect, { init: 1, before: 1, after: 1, destroy: 1 }, 'tcpconnect when process exits'); diff --git a/test/common/index.js b/test/common/index.js index 61ba3d478debc5..8367c150fe669c 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -535,8 +535,8 @@ function _mustCallInner(fn, criteria = 1, field) { } exports.hasMultiLocalhost = function hasMultiLocalhost() { - const TCP = process.binding('tcp_wrap').TCP; - const t = new TCP(); + const { TCP, constants: TCPConstants } = process.binding('tcp_wrap'); + const t = new TCP(TCPConstants.SOCKET); const ret = t.bind('127.0.0.2', 0); t.close(); return ret === 0; diff --git a/test/parallel/test-handle-wrap-isrefed.js b/test/parallel/test-handle-wrap-isrefed.js index b4fccc0529c2fb..e1301b57f6baeb 100644 --- a/test/parallel/test-handle-wrap-isrefed.js +++ b/test/parallel/test-handle-wrap-isrefed.js @@ -66,8 +66,8 @@ const dgram = require('dgram'); // pipe { - const Pipe = process.binding('pipe_wrap').Pipe; - const handle = new Pipe(); + const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap'); + const handle = new Pipe(PipeConstants.SOCKET); strictEqual(Object.getPrototypeOf(handle).hasOwnProperty('hasRef'), true, 'pipe_wrap: hasRef() missing'); strictEqual(handle.hasRef(), diff --git a/test/parallel/test-net-connect-options-fd.js b/test/parallel/test-net-connect-options-fd.js index 1cba6e1251fd21..50c2a08efeb194 100644 --- a/test/parallel/test-net-connect-options-fd.js +++ b/test/parallel/test-net-connect-options-fd.js @@ -6,7 +6,7 @@ if (common.isWindows) const assert = require('assert'); const net = require('net'); const path = require('path'); -const Pipe = process.binding('pipe_wrap').Pipe; +const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap'); common.refreshTmpDir(); @@ -71,7 +71,7 @@ const forAllClients = (cb) => common.mustCall(cb, CLIENT_VARIANTS); }) .listen({ path: serverPath }, common.mustCall(function serverOnListen() { const getSocketOpt = (index) => { - const handle = new Pipe(); + const handle = new Pipe(PipeConstants.SOCKET); const err = handle.bind(`${prefix}-client-${socketCounter++}`); assert(err >= 0, String(err)); assert.notStrictEqual(handle.fd, -1); diff --git a/test/parallel/test-net-server-listen-handle.js b/test/parallel/test-net-server-listen-handle.js index db8e639f94885f..de1f1ca375d98e 100644 --- a/test/parallel/test-net-server-listen-handle.js +++ b/test/parallel/test-net-server-listen-handle.js @@ -5,8 +5,8 @@ const assert = require('assert'); const net = require('net'); const fs = require('fs'); const uv = process.binding('uv'); -const TCP = process.binding('tcp_wrap').TCP; -const Pipe = process.binding('pipe_wrap').Pipe; +const { TCP, constants: TCPConstants } = process.binding('tcp_wrap'); +const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap'); common.refreshTmpDir(); @@ -36,12 +36,12 @@ function randomPipePath() { function randomHandle(type) { let handle, errno, handleName; if (type === 'tcp') { - handle = new TCP(); + handle = new TCP(TCPConstants.SOCKET); errno = handle.bind('0.0.0.0', 0); handleName = 'arbitrary tcp port'; } else { const path = randomPipePath(); - handle = new Pipe(); + handle = new Pipe(PipeConstants.SOCKET); errno = handle.bind(path); handleName = `pipe ${path}`; } diff --git a/test/parallel/test-process-wrap.js b/test/parallel/test-process-wrap.js index 5601328eef3585..b3cca47c463c19 100644 --- a/test/parallel/test-process-wrap.js +++ b/test/parallel/test-process-wrap.js @@ -23,8 +23,8 @@ require('../common'); const assert = require('assert'); const Process = process.binding('process_wrap').Process; -const Pipe = process.binding('pipe_wrap').Pipe; -const pipe = new Pipe(); +const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap'); +const pipe = new Pipe(PipeConstants.SOCKET); const p = new Process(); let processExited = false; diff --git a/test/parallel/test-tcp-wrap-connect.js b/test/parallel/test-tcp-wrap-connect.js index 77f9814db2ebe5..c2746bca64d198 100644 --- a/test/parallel/test-tcp-wrap-connect.js +++ b/test/parallel/test-tcp-wrap-connect.js @@ -1,12 +1,12 @@ 'use strict'; require('../common'); const assert = require('assert'); -const TCP = process.binding('tcp_wrap').TCP; +const { TCP, constants: TCPConstants } = process.binding('tcp_wrap'); const TCPConnectWrap = process.binding('tcp_wrap').TCPConnectWrap; const ShutdownWrap = process.binding('stream_wrap').ShutdownWrap; function makeConnection() { - const client = new TCP(); + const client = new TCP(TCPConstants.SOCKET); const req = new TCPConnectWrap(); const err = client.connect(req, '127.0.0.1', this.address().port); diff --git a/test/parallel/test-tcp-wrap-listen.js b/test/parallel/test-tcp-wrap-listen.js index 7502969c967f86..8203a4771b861f 100644 --- a/test/parallel/test-tcp-wrap-listen.js +++ b/test/parallel/test-tcp-wrap-listen.js @@ -2,10 +2,10 @@ const common = require('../common'); const assert = require('assert'); -const TCP = process.binding('tcp_wrap').TCP; +const { TCP, constants: TCPConstants } = process.binding('tcp_wrap'); const WriteWrap = process.binding('stream_wrap').WriteWrap; -const server = new TCP(); +const server = new TCP(TCPConstants.SOCKET); const r = server.bind('0.0.0.0', 0); assert.strictEqual(0, r); diff --git a/test/parallel/test-tcp-wrap.js b/test/parallel/test-tcp-wrap.js index 96ddd5bfd31ffa..36a45d7606b125 100644 --- a/test/parallel/test-tcp-wrap.js +++ b/test/parallel/test-tcp-wrap.js @@ -23,10 +23,10 @@ require('../common'); const assert = require('assert'); -const TCP = process.binding('tcp_wrap').TCP; +const { TCP, constants: TCPConstants } = process.binding('tcp_wrap'); const uv = process.binding('uv'); -const handle = new TCP(); +const handle = new TCP(TCPConstants.SOCKET); // Should be able to bind to the port let err = handle.bind('0.0.0.0', 0); diff --git a/test/sequential/test-async-wrap-getasyncid.js b/test/sequential/test-async-wrap-getasyncid.js index 46515fc115eda5..bdb2be9dfc387b 100644 --- a/test/sequential/test-async-wrap-getasyncid.js +++ b/test/sequential/test-async-wrap-getasyncid.js @@ -138,19 +138,27 @@ if (common.hasCrypto) { // eslint-disable-line crypto-check testInitialized(new Gzip()._handle, 'Zlib'); } - { const binding = process.binding('pipe_wrap'); - const handle = new binding.Pipe(); + const handle = new binding.Pipe(binding.constants.IPC); testInitialized(handle, 'Pipe'); - const req = new binding.PipeConnectWrap(); - testUninitialized(req, 'PipeConnectWrap'); - req.address = common.PIPE; - req.oncomplete = common.mustCall(() => handle.close()); - handle.connect(req, req.address, req.oncomplete); - testInitialized(req, 'PipeConnectWrap'); } +{ + const server = net.createServer(common.mustCall((socket) => { + server.close(); + })).listen(common.PIPE, common.mustCall(() => { + const binding = process.binding('pipe_wrap'); + const handle = new binding.Pipe(binding.constants.SOCKET); + testInitialized(handle, 'Pipe'); + const req = new binding.PipeConnectWrap(); + testUninitialized(req, 'PipeConnectWrap'); + req.address = common.PIPE; + req.oncomplete = common.mustCall(() => handle.close()); + handle.connect(req, req.address, req.oncomplete); + testInitialized(req, 'PipeConnectWrap'); + })); +} { const Process = process.binding('process_wrap').Process; @@ -179,7 +187,7 @@ if (common.hasCrypto) { // eslint-disable-line crypto-check }); socket.resume(); })).listen(0, common.localhostIPv4, common.mustCall(() => { - const handle = new tcp_wrap.TCP(); + const handle = new tcp_wrap.TCP(tcp_wrap.constants.SOCKET); const req = new tcp_wrap.TCPConnectWrap(); const sreq = new stream_wrap.ShutdownWrap(); const wreq = new stream_wrap.WriteWrap(); @@ -221,8 +229,8 @@ if (common.hasCrypto) { // eslint-disable-line crypto-check if (common.hasCrypto) { // eslint-disable-line crypto-check - const TCP = process.binding('tcp_wrap').TCP; - const tcp = new TCP(); + const { TCP, constants: TCPConstants } = process.binding('tcp_wrap'); + const tcp = new TCP(TCPConstants.SOCKET); const ca = fixtures.readSync('test_ca.pem', 'ascii'); const cert = fixtures.readSync('test_cert.pem', 'ascii'); From 1044e7623cbd772bb54cb3a174b3b818d5df5867 Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Wed, 22 Nov 2017 13:54:38 +0100 Subject: [PATCH 13/22] async_hooks: rename initTriggerId 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 Reviewed-By: James M Snell --- lib/_http_client.js | 2 +- lib/async_hooks.js | 6 +++--- lib/dgram.js | 4 ++-- lib/internal/async_hooks.js | 36 +++++++++++++++---------------- lib/internal/bootstrap_node.js | 8 +++---- lib/internal/process/next_tick.js | 6 +++--- lib/net.js | 10 ++++----- lib/timers.js | 8 +++---- src/async_wrap.cc | 13 +++++------ src/connection_wrap.cc | 2 +- src/env-inl.h | 23 ++++++++++---------- src/env.h | 6 +++--- src/stream_base.cc | 8 +++---- src/tcp_wrap.cc | 4 ++-- src/udp_wrap.cc | 2 +- 15 files changed, 70 insertions(+), 68 deletions(-) diff --git a/lib/_http_client.js b/lib/_http_client.js index 5a999ca112c97b..0170b2c9b0e6fb 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -582,7 +582,7 @@ function responseKeepAlive(res, req) { socket.removeListener('error', socketErrorListener); socket.once('error', freeSocketErrorListener); // There are cases where _handle === null. Avoid those. Passing null to - // nextTick() will call initTriggerId() to retrieve the id. + // nextTick() will call getDefaultTriggerAsyncId() to retrieve the id. const asyncId = socket._handle ? socket._handle.getAsyncId() : null; // Mark this socket as available, AFTER user-added end // handlers have a chance to run. diff --git a/lib/async_hooks.js b/lib/async_hooks.js index 52c887ecb1b229..ad5049a6fe57e1 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -19,7 +19,7 @@ const { disableHooks, // Sensitive Embedder API newUid, - initTriggerId, + getDefaultTriggerAsyncId, setInitTriggerId, emitInit, emitBefore, @@ -152,7 +152,7 @@ class AsyncResource { if (typeof opts === 'number') { opts = { triggerAsyncId: opts, requireManualDestroy: false }; } else if (opts.triggerAsyncId === undefined) { - opts.triggerAsyncId = initTriggerId(); + opts.triggerAsyncId = getDefaultTriggerAsyncId(); } // Unlike emitInitScript, AsyncResource doesn't supports null as the @@ -276,7 +276,7 @@ Object.defineProperty(module.exports, 'newUid', { Object.defineProperty(module.exports, 'initTriggerId', { get: internalUtil.deprecate(function() { - return initTriggerId; + return getDefaultTriggerAsyncId; }, 'async_hooks.initTriggerId is deprecated. ' + 'Use the AsyncResource default instead.', 'DEP0085') }); diff --git a/lib/dgram.js b/lib/dgram.js index 8fe52713bc37aa..f14e6c7890e485 100644 --- a/lib/dgram.js +++ b/lib/dgram.js @@ -28,7 +28,7 @@ const dns = require('dns'); const util = require('util'); const { isUint8Array } = require('internal/util/types'); const EventEmitter = require('events'); -const { setInitTriggerId } = require('internal/async_hooks'); +const { setDefaultTriggerAsyncId } = require('internal/async_hooks'); const { UV_UDP_REUSEADDR } = process.binding('constants').os; const { async_id_symbol } = process.binding('async_wrap'); const { nextTick } = require('internal/process/next_tick'); @@ -479,7 +479,7 @@ function doSend(ex, self, ip, list, address, port, callback) { // node::SendWrap isn't instantiated and attached to the JS instance of // SendWrap above until send() is called. So don't set the init trigger id // until now. - setInitTriggerId(self[async_id_symbol]); + setDefaultTriggerAsyncId(self[async_id_symbol]); var err = self._handle.send(req, list, list.length, diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 5964a847fc0a25..392b44f36ef43c 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -14,10 +14,10 @@ const async_wrap = process.binding('async_wrap'); * kTriggerAsyncId: The trigger_async_id of the resource responsible for * the current execution stack. * kAsyncIdCounter: Incremental counter tracking the next assigned async_id. - * kInitTriggerAsyncId: Written immediately before a resource's constructor + * kDefaultTriggerAsyncId: Written immediately before a resource's constructor * that sets the value of the init()'s triggerAsyncId. The order of * retrieving the triggerAsyncId value is passing directly to the - * constructor -> value set in kInitTriggerAsyncId -> executionAsyncId of + * constructor -> value set in kDefaultTriggerAsyncId -> executionAsyncId of * the current resource. */ const { async_hook_fields, async_id_fields } = async_wrap; @@ -61,7 +61,7 @@ const active_hooks = { // for a given step, that step can bail out early. const { kInit, kBefore, kAfter, kDestroy, kPromiseResolve, kCheck, kExecutionAsyncId, kAsyncIdCounter, - kInitTriggerAsyncId } = async_wrap.constants; + kDefaultTriggerAsyncId } = async_wrap.constants; // Used in AsyncHook and AsyncResource. const init_symbol = Symbol('init'); @@ -245,25 +245,25 @@ function newUid() { return ++async_id_fields[kAsyncIdCounter]; } - // Return the triggerAsyncId meant for the constructor calling it. It's up to // the user to safeguard this call and make sure it's zero'd out when the // constructor is complete. -function initTriggerId() { - var triggerAsyncId = async_id_fields[kInitTriggerAsyncId]; +function getDefaultTriggerAsyncId() { + var defaultTriggerAsyncId = async_id_fields[kDefaultTriggerAsyncId]; // Reset value after it's been called so the next constructor doesn't // inherit it by accident. - async_id_fields[kInitTriggerAsyncId] = 0; - if (triggerAsyncId <= 0) - triggerAsyncId = async_id_fields[kExecutionAsyncId]; - return triggerAsyncId; + async_id_fields[kDefaultTriggerAsyncId] = 0; + // If defaultTriggerAsyncId isn't set, use the executionAsyncId + if (defaultTriggerAsyncId <= 0) + defaultTriggerAsyncId = async_id_fields[kExecutionAsyncId]; + return defaultTriggerAsyncId; } -function setInitTriggerId(triggerAsyncId) { +function setDefaultTriggerAsyncId(triggerAsyncId) { // CHECK(Number.isSafeInteger(triggerAsyncId)) // CHECK(triggerAsyncId > 0) - async_id_fields[kInitTriggerAsyncId] = triggerAsyncId; + async_id_fields[kDefaultTriggerAsyncId] = triggerAsyncId; } @@ -282,13 +282,13 @@ function emitInitScript(asyncId, type, triggerAsyncId, resource) { return; // This can run after the early return check b/c running this function - // manually means that the embedder must have used initTriggerId(). + // manually means that the embedder must have used getDefaultTriggerAsyncId(). if (triggerAsyncId === null) { - triggerAsyncId = initTriggerId(); + triggerAsyncId = getDefaultTriggerAsyncId(); } else { - // If a triggerAsyncId was passed, any kInitTriggerAsyncId still must be + // If a triggerAsyncId was passed, any kDefaultTriggerAsyncId still must be // null'd. - async_id_fields[kInitTriggerAsyncId] = 0; + async_id_fields[kDefaultTriggerAsyncId] = 0; } emitInitNative(asyncId, type, triggerAsyncId, resource); @@ -340,8 +340,8 @@ module.exports = { disableHooks, // Sensitive Embedder API newUid, - initTriggerId, - setInitTriggerId, + getDefaultTriggerAsyncId, + setDefaultTriggerAsyncId, emitInit: emitInitScript, emitBefore: emitBeforeScript, emitAfter: emitAfterScript, diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index f751cf45e5878e..08e3faa67a0abe 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -359,14 +359,14 @@ // Internal functions needed to manipulate the stack. const { clearAsyncIdStack, asyncIdStackSize } = async_wrap; const { kAfter, kExecutionAsyncId, - kInitTriggerAsyncId } = async_wrap.constants; + kDefaultTriggerAsyncId } = async_wrap.constants; process._fatalException = function(er) { var caught; - // It's possible that kInitTriggerAsyncId was set for a constructor call - // that threw and was never cleared. So clear it now. - async_id_fields[kInitTriggerAsyncId] = 0; + // It's possible that kDefaultTriggerAsyncId was set for a constructor + // call that threw and was never cleared. So clear it now. + async_id_fields[kDefaultTriggerAsyncId] = 0; if (process.domain && process.domain._errorHandler) caught = process.domain._errorHandler(er); diff --git a/lib/internal/process/next_tick.js b/lib/internal/process/next_tick.js index 59a1e4fee1c75f..54a6bdd0298c85 100644 --- a/lib/internal/process/next_tick.js +++ b/lib/internal/process/next_tick.js @@ -52,7 +52,7 @@ function setupNextTick() { const promises = require('internal/process/promises'); const errors = require('internal/errors'); const emitPendingUnhandledRejections = promises.setup(scheduleMicrotasks); - const initTriggerId = async_hooks.initTriggerId; + const getDefaultTriggerAsyncId = async_hooks.getDefaultTriggerAsyncId; // Two arrays that share state between C++ and JS. const { async_hook_fields, async_id_fields } = async_wrap; // Used to change the state of the async id stack. @@ -262,7 +262,7 @@ function setupNextTick() { } const asyncId = ++async_id_fields[kAsyncIdCounter]; - const triggerAsyncId = initTriggerId(); + const triggerAsyncId = getDefaultTriggerAsyncId(); const obj = new TickObject(callback, args, asyncId, triggerAsyncId); nextTickQueue.push(obj); ++tickInfo[kLength]; @@ -282,7 +282,7 @@ function setupNextTick() { return; if (triggerAsyncId === null) { - triggerAsyncId = async_hooks.initTriggerId(); + triggerAsyncId = async_hooks.getDefaultTriggerAsyncId(); } var args; diff --git a/lib/net.js b/lib/net.js index d34a9dead3b7e5..bf2a3da97a4dd7 100644 --- a/lib/net.js +++ b/lib/net.js @@ -39,7 +39,7 @@ const { TCPConnectWrap } = process.binding('tcp_wrap'); const { PipeConnectWrap } = process.binding('pipe_wrap'); const { ShutdownWrap, WriteWrap } = process.binding('stream_wrap'); const { async_id_symbol } = process.binding('async_wrap'); -const { newUid, setInitTriggerId } = require('internal/async_hooks'); +const { newUid, setDefaultTriggerAsyncId } = require('internal/async_hooks'); const { nextTick } = require('internal/process/next_tick'); const errors = require('internal/errors'); const dns = require('dns'); @@ -297,7 +297,7 @@ function onSocketFinish() { // node::ShutdownWrap isn't instantiated and attached to the JS instance of // ShutdownWrap above until shutdown() is called. So don't set the init // trigger id until now. - setInitTriggerId(this[async_id_symbol]); + setDefaultTriggerAsyncId(this[async_id_symbol]); var err = this._handle.shutdown(req); if (err) @@ -953,7 +953,7 @@ function internalConnect( // node::TCPConnectWrap isn't instantiated and attached to the JS instance // of TCPConnectWrap above until connect() is called. So don't set the init // trigger id until now. - setInitTriggerId(self[async_id_symbol]); + setDefaultTriggerAsyncId(self[async_id_symbol]); if (addressType === 4) err = self._handle.connect(req, address, port); else @@ -966,7 +966,7 @@ function internalConnect( // node::PipeConnectWrap isn't instantiated and attached to the JS instance // of PipeConnectWrap above until connect() is called. So don't set the // init trigger id until now. - setInitTriggerId(self[async_id_symbol]); + setDefaultTriggerAsyncId(self[async_id_symbol]); err = self._handle.connect(req, address, afterConnect); } @@ -1095,7 +1095,7 @@ function lookupAndConnect(self, options) { debug('connect: dns options', dnsopts); self._host = host; var lookup = options.lookup || dns.lookup; - setInitTriggerId(self[async_id_symbol]); + setDefaultTriggerAsyncId(self[async_id_symbol]); lookup(host, dnsopts, function emitLookup(err, ip, addressType) { self.emit('lookup', err, ip, addressType, host); diff --git a/lib/timers.js b/lib/timers.js index d12c080cf7ab5d..0e6ae45950c5c1 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -33,7 +33,7 @@ const kOnTimeout = TimerWrap.kOnTimeout | 0; // Two arrays that share state between C++ and JS. const { async_hook_fields, async_id_fields } = async_wrap; const { - initTriggerId, + getDefaultTriggerAsyncId, // The needed emit*() functions. emitInit, emitBefore, @@ -180,7 +180,7 @@ function insert(item, unrefed) { if (!item[async_id_symbol] || item._destroyed) { item._destroyed = false; item[async_id_symbol] = ++async_id_fields[kAsyncIdCounter]; - item[trigger_async_id_symbol] = initTriggerId(); + item[trigger_async_id_symbol] = getDefaultTriggerAsyncId(); if (async_hook_fields[kInit] > 0) emitInit( item[async_id_symbol], 'Timeout', item[trigger_async_id_symbol], item @@ -581,7 +581,7 @@ function Timeout(after, callback, args) { this._repeat = null; this._destroyed = false; this[async_id_symbol] = ++async_id_fields[kAsyncIdCounter]; - this[trigger_async_id_symbol] = initTriggerId(); + this[trigger_async_id_symbol] = getDefaultTriggerAsyncId(); if (async_hook_fields[kInit] > 0) emitInit( this[async_id_symbol], 'Timeout', this[trigger_async_id_symbol], this @@ -816,7 +816,7 @@ function Immediate() { this._destroyed = false; this.domain = process.domain; this[async_id_symbol] = ++async_id_fields[kAsyncIdCounter]; - this[trigger_async_id_symbol] = initTriggerId(); + this[trigger_async_id_symbol] = getDefaultTriggerAsyncId(); if (async_hook_fields[kInit] > 0) emitInit( this[async_id_symbol], 'Immediate', this[trigger_async_id_symbol], this diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 5f2dc5becebaf4..897cc5a5518fac 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -332,7 +332,7 @@ static void PromiseHook(PromiseHookType type, Local promise, } // get id from parentWrap double trigger_async_id = parent_wrap->get_async_id(); - env->set_init_trigger_async_id(trigger_async_id); + env->set_default_trigger_async_id(trigger_async_id); } wrap = PromiseWrap::New(env, promise, parent_wrap, silent); @@ -564,9 +564,10 @@ void AsyncWrap::Initialize(Local target, // // kAsyncUid: Maintains the state of the next unique id to be assigned. // - // kInitTriggerAsyncId: Write the id of the resource responsible for a + // kDefaultTriggerAsyncId: Write the id of the resource responsible for a // handle's creation just before calling the new handle's constructor. - // After the new handle is constructed kInitTriggerAsyncId is set back to 0. + // After the new handle is constructed kDefaultTriggerAsyncId is set back + // to 0. FORCE_SET_TARGET_FIELD(target, "async_id_fields", env->async_hooks()->async_id_fields().GetJSArray()); @@ -586,7 +587,7 @@ void AsyncWrap::Initialize(Local target, SET_HOOKS_CONSTANT(kExecutionAsyncId); SET_HOOKS_CONSTANT(kTriggerAsyncId); SET_HOOKS_CONSTANT(kAsyncIdCounter); - SET_HOOKS_CONSTANT(kInitTriggerAsyncId); + SET_HOOKS_CONSTANT(kDefaultTriggerAsyncId); #undef SET_HOOKS_CONSTANT FORCE_SET_TARGET_FIELD(target, "constants", constants); @@ -699,7 +700,7 @@ void AsyncWrap::EmitDestroy(Environment* env, double async_id) { void AsyncWrap::AsyncReset(double execution_async_id, bool silent) { async_id_ = execution_async_id == -1 ? env()->new_async_id() : execution_async_id; - trigger_async_id_ = env()->get_init_trigger_async_id(); + trigger_async_id_ = env()->get_default_trigger_async_id(); switch (provider_type()) { #define V(PROVIDER) \ @@ -815,7 +816,7 @@ async_context EmitAsyncInit(Isolate* isolate, // Initialize async context struct if (trigger_async_id == -1) - trigger_async_id = env->get_init_trigger_async_id(); + trigger_async_id = env->get_default_trigger_async_id(); async_context context = { env->new_async_id(), // async_id_ diff --git a/src/connection_wrap.cc b/src/connection_wrap.cc index d82e7195d76579..b7c1949e11e404 100644 --- a/src/connection_wrap.cc +++ b/src/connection_wrap.cc @@ -49,7 +49,7 @@ void ConnectionWrap::OnConnection(uv_stream_t* handle, }; if (status == 0) { - env->set_init_trigger_async_id(wrap_data->get_async_id()); + env->set_default_trigger_async_id(wrap_data->get_async_id()); // Instantiate the client javascript object and handle. Local client_obj = WrapType::Instantiate(env, wrap_data, diff --git a/src/env-inl.h b/src/env-inl.h index 81086790d2cd9b..905e2d93b90441 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -464,17 +464,18 @@ inline double Environment::trigger_async_id() { return async_hooks()->async_id_fields()[AsyncHooks::kTriggerAsyncId]; } -inline double Environment::get_init_trigger_async_id() { - AliasedBuffer& async_id_fields = - async_hooks()->async_id_fields(); - double tid = async_id_fields[AsyncHooks::kInitTriggerAsyncId]; - async_id_fields[AsyncHooks::kInitTriggerAsyncId] = 0; - if (tid <= 0) tid = execution_async_id(); - return tid; -} - -inline void Environment::set_init_trigger_async_id(const double id) { - async_hooks()->async_id_fields()[AsyncHooks::kInitTriggerAsyncId] = id; +inline double Environment::get_default_trigger_async_id() { + double default_trigger_async_id = + async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId]; + async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId] = 0; + // If defaultTriggerAsyncId isn't set, use the executionAsyncId + if (default_trigger_async_id <= 0) + default_trigger_async_id = execution_async_id(); + return default_trigger_async_id; +} + +inline void Environment::set_default_trigger_async_id(const double id) { + async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId] = id; } inline double* Environment::heap_statistics_buffer() const { diff --git a/src/env.h b/src/env.h index 64ddbb57b9e5f3..a359c255bc59a8 100644 --- a/src/env.h +++ b/src/env.h @@ -401,7 +401,7 @@ class Environment { kExecutionAsyncId, kTriggerAsyncId, kAsyncIdCounter, - kInitTriggerAsyncId, + kDefaultTriggerAsyncId, kUidFieldsCount, }; @@ -593,8 +593,8 @@ class Environment { inline double new_async_id(); inline double execution_async_id(); inline double trigger_async_id(); - inline double get_init_trigger_async_id(); - inline void set_init_trigger_async_id(const double id); + inline double get_default_trigger_async_id(); + inline void set_default_trigger_async_id(const double id); // List of id's that have been destroyed and need the destroy() cb called. inline std::vector* destroy_async_id_list(); diff --git a/src/stream_base.cc b/src/stream_base.cc index c6aca1694f9568..11b83c67957da6 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -52,7 +52,7 @@ int StreamBase::Shutdown(const FunctionCallbackInfo& args) { AsyncWrap* wrap = GetAsyncWrap(); CHECK_NE(wrap, nullptr); - env->set_init_trigger_async_id(wrap->get_async_id()); + env->set_default_trigger_async_id(wrap->get_async_id()); ShutdownWrap* req_wrap = new ShutdownWrap(env, req_wrap_obj, this, @@ -157,7 +157,7 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { wrap = GetAsyncWrap(); CHECK_NE(wrap, nullptr); - env->set_init_trigger_async_id(wrap->get_async_id()); + env->set_default_trigger_async_id(wrap->get_async_id()); req_wrap = WriteWrap::New(env, req_wrap_obj, this, AfterWrite, storage_size); offset = 0; @@ -246,7 +246,7 @@ int StreamBase::WriteBuffer(const FunctionCallbackInfo& args) { wrap = GetAsyncWrap(); if (wrap != nullptr) - env->set_init_trigger_async_id(wrap->get_async_id()); + env->set_default_trigger_async_id(wrap->get_async_id()); // Allocate, or write rest req_wrap = WriteWrap::New(env, req_wrap_obj, this, AfterWrite); @@ -331,7 +331,7 @@ int StreamBase::WriteString(const FunctionCallbackInfo& args) { wrap = GetAsyncWrap(); if (wrap != nullptr) - env->set_init_trigger_async_id(wrap->get_async_id()); + env->set_default_trigger_async_id(wrap->get_async_id()); req_wrap = WriteWrap::New(env, req_wrap_obj, this, AfterWrite, storage_size); data = req_wrap->Extra(); diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index 8dd14e2e16c18b..00853df9baf5c8 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -293,7 +293,7 @@ void TCPWrap::Connect(const FunctionCallbackInfo& args) { int err = uv_ip4_addr(*ip_address, port, &addr); if (err == 0) { - env->set_init_trigger_async_id(wrap->get_async_id()); + env->set_default_trigger_async_id(wrap->get_async_id()); ConnectWrap* req_wrap = new ConnectWrap(env, req_wrap_obj, AsyncWrap::PROVIDER_TCPCONNECTWRAP); err = uv_tcp_connect(req_wrap->req(), @@ -329,7 +329,7 @@ void TCPWrap::Connect6(const FunctionCallbackInfo& args) { int err = uv_ip6_addr(*ip_address, port, &addr); if (err == 0) { - env->set_init_trigger_async_id(wrap->get_async_id()); + env->set_default_trigger_async_id(wrap->get_async_id()); ConnectWrap* req_wrap = new ConnectWrap(env, req_wrap_obj, AsyncWrap::PROVIDER_TCPCONNECTWRAP); err = uv_tcp_connect(req_wrap->req(), diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index 4a9eefb9499606..5ba2be37c7a601 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -347,7 +347,7 @@ void UDPWrap::DoSend(const FunctionCallbackInfo& args, int family) { node::Utf8Value address(env->isolate(), args[4]); const bool have_callback = args[5]->IsTrue(); - env->set_init_trigger_async_id(wrap->get_async_id()); + env->set_default_trigger_async_id(wrap->get_async_id()); SendWrap* req_wrap = new SendWrap(env, req_wrap_obj, have_callback); size_t msg_size = 0; From a04d5ee430313ceb091f0c718276aa344a24936b Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Wed, 22 Nov 2017 15:41:18 +0100 Subject: [PATCH 14/22] async_hooks: separate missing from default context When context is missing the executionAsyncId will be zero. For the default triggerAsyncId the zero value was used to default to the executionAsyncId. While this was not technically wrong because the functions are different themself, it poorly separated the two concepts. PR-URL: https://github.com/nodejs/node/pull/17273 Reviewed-By: Matteo Collina Reviewed-By: James M Snell --- lib/internal/async_hooks.js | 6 +++--- lib/internal/bootstrap_node.js | 2 +- src/env-inl.h | 10 ++++++++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 392b44f36ef43c..6b0420c8c71062 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -252,9 +252,9 @@ function getDefaultTriggerAsyncId() { var defaultTriggerAsyncId = async_id_fields[kDefaultTriggerAsyncId]; // Reset value after it's been called so the next constructor doesn't // inherit it by accident. - async_id_fields[kDefaultTriggerAsyncId] = 0; + async_id_fields[kDefaultTriggerAsyncId] = -1; // If defaultTriggerAsyncId isn't set, use the executionAsyncId - if (defaultTriggerAsyncId <= 0) + if (defaultTriggerAsyncId < 0) defaultTriggerAsyncId = async_id_fields[kExecutionAsyncId]; return defaultTriggerAsyncId; } @@ -288,7 +288,7 @@ function emitInitScript(asyncId, type, triggerAsyncId, resource) { } else { // If a triggerAsyncId was passed, any kDefaultTriggerAsyncId still must be // null'd. - async_id_fields[kDefaultTriggerAsyncId] = 0; + async_id_fields[kDefaultTriggerAsyncId] = -1; } emitInitNative(asyncId, type, triggerAsyncId, resource); diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index 08e3faa67a0abe..6a8713c986b076 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -366,7 +366,7 @@ // It's possible that kDefaultTriggerAsyncId was set for a constructor // call that threw and was never cleared. So clear it now. - async_id_fields[kDefaultTriggerAsyncId] = 0; + async_id_fields[kDefaultTriggerAsyncId] = -1; if (process.domain && process.domain._errorHandler) caught = process.domain._errorHandler(er); diff --git a/src/env-inl.h b/src/env-inl.h index 905e2d93b90441..736271a911c905 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -87,6 +87,12 @@ inline Environment::AsyncHooks::AsyncHooks(v8::Isolate* isolate) async_id_fields_(isolate, kUidFieldsCount) { v8::HandleScope handle_scope(isolate_); + // kDefaultTriggerAsyncId should be -1, this indicates that there is no + // specified default value and it should fallback to the executionAsyncId. + // 0 is not used as the magic value, because that indicates a missing context + // which is different from a default context. + async_id_fields_[AsyncHooks::kDefaultTriggerAsyncId] = -1; + // kAsyncIdCounter should start at 1 because that'll be the id the execution // context during bootstrap (code that runs before entering uv_run()). async_id_fields_[AsyncHooks::kAsyncIdCounter] = 1; @@ -467,9 +473,9 @@ inline double Environment::trigger_async_id() { inline double Environment::get_default_trigger_async_id() { double default_trigger_async_id = async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId]; - async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId] = 0; + async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId] = -1; // If defaultTriggerAsyncId isn't set, use the executionAsyncId - if (default_trigger_async_id <= 0) + if (default_trigger_async_id < 0) default_trigger_async_id = execution_async_id(); return default_trigger_async_id; } From 44f4c7316d29abf83f1ef1bd5756e6c2a7b50295 Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Wed, 22 Nov 2017 18:41:00 +0100 Subject: [PATCH 15/22] async_hooks: use scope for defaultTriggerAsyncId Previously the getter would mutate the kDefaultTriggerAsncId value. This refactor changes the setter to bind the current kDefaultTriggerAsncId to a scope, such that the getter doesn't have to mutate its own value. PR-URL: https://github.com/nodejs/node/pull/17273 Reviewed-By: Matteo Collina Reviewed-By: James M Snell --- lib/async_hooks.js | 8 ---- lib/dgram.js | 21 +++++---- lib/internal/async_hooks.js | 21 +++++---- lib/net.js | 91 +++++++++++++++++++------------------ src/async_wrap.cc | 11 +++-- src/connection_wrap.cc | 1 - src/env-inl.h | 33 +++++++------- src/env.h | 20 ++++---- src/pipe_wrap.cc | 3 +- src/stream_base-inl.h | 3 +- src/stream_base.cc | 40 +++++++++------- src/tcp_wrap.cc | 9 ++-- src/udp_wrap.cc | 12 +++-- 13 files changed, 144 insertions(+), 129 deletions(-) diff --git a/lib/async_hooks.js b/lib/async_hooks.js index ad5049a6fe57e1..bf07358fd5e2c1 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -20,7 +20,6 @@ const { // Sensitive Embedder API newUid, getDefaultTriggerAsyncId, - setInitTriggerId, emitInit, emitBefore, emitAfter, @@ -281,13 +280,6 @@ Object.defineProperty(module.exports, 'initTriggerId', { 'Use the AsyncResource default instead.', 'DEP0085') }); -Object.defineProperty(module.exports, 'setInitTriggerId', { - get: internalUtil.deprecate(function() { - return setInitTriggerId; - }, 'async_hooks.setInitTriggerId is deprecated. ' + - 'Use the triggerAsyncId parameter in AsyncResource instead.', 'DEP0085') -}); - Object.defineProperty(module.exports, 'emitInit', { get: internalUtil.deprecate(function() { return emitInit; diff --git a/lib/dgram.js b/lib/dgram.js index f14e6c7890e485..04828155f3a0d7 100644 --- a/lib/dgram.js +++ b/lib/dgram.js @@ -28,7 +28,7 @@ const dns = require('dns'); const util = require('util'); const { isUint8Array } = require('internal/util/types'); const EventEmitter = require('events'); -const { setDefaultTriggerAsyncId } = require('internal/async_hooks'); +const { defaultTriggerAsyncIdScope } = require('internal/async_hooks'); const { UV_UDP_REUSEADDR } = process.binding('constants').os; const { async_id_symbol } = process.binding('async_wrap'); const { nextTick } = require('internal/process/next_tick'); @@ -448,21 +448,24 @@ Socket.prototype.send = function(buffer, } const afterDns = (ex, ip) => { - doSend(ex, this, ip, list, address, port, callback); + defaultTriggerAsyncIdScope( + this[async_id_symbol], + [ex, this, ip, list, address, port, callback], + doSend + ); }; this._handle.lookup(address, afterDns); }; - function doSend(ex, self, ip, list, address, port, callback) { if (ex) { if (typeof callback === 'function') { - callback(ex); + process.nextTick(callback, ex); return; } - self.emit('error', ex); + process.nextTick(() => self.emit('error', ex)); return; } else if (!self._handle) { return; @@ -476,20 +479,18 @@ function doSend(ex, self, ip, list, address, port, callback) { req.callback = callback; req.oncomplete = afterSend; } - // node::SendWrap isn't instantiated and attached to the JS instance of - // SendWrap above until send() is called. So don't set the init trigger id - // until now. - setDefaultTriggerAsyncId(self[async_id_symbol]); + var err = self._handle.send(req, list, list.length, port, ip, !!callback); + if (err && callback) { // don't emit as error, dgram_legacy.js compatibility const ex = exceptionWithHostPort(err, 'send', address, port); - nextTick(self[async_id_symbol], callback, ex); + process.nextTick(callback, ex); } } diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 6b0420c8c71062..366a218b7a95c2 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -250,9 +250,6 @@ function newUid() { // constructor is complete. function getDefaultTriggerAsyncId() { var defaultTriggerAsyncId = async_id_fields[kDefaultTriggerAsyncId]; - // Reset value after it's been called so the next constructor doesn't - // inherit it by accident. - async_id_fields[kDefaultTriggerAsyncId] = -1; // If defaultTriggerAsyncId isn't set, use the executionAsyncId if (defaultTriggerAsyncId < 0) defaultTriggerAsyncId = async_id_fields[kExecutionAsyncId]; @@ -260,10 +257,20 @@ function getDefaultTriggerAsyncId() { } -function setDefaultTriggerAsyncId(triggerAsyncId) { +function defaultTriggerAsyncIdScope(triggerAsyncId, opaque, block) { // CHECK(Number.isSafeInteger(triggerAsyncId)) // CHECK(triggerAsyncId > 0) + const oldDefaultTriggerAsyncId = async_id_fields[kDefaultTriggerAsyncId]; async_id_fields[kDefaultTriggerAsyncId] = triggerAsyncId; + + var ret; + try { + ret = Reflect.apply(block, null, opaque); + } finally { + async_id_fields[kDefaultTriggerAsyncId] = oldDefaultTriggerAsyncId; + } + + return ret; } @@ -285,10 +292,6 @@ function emitInitScript(asyncId, type, triggerAsyncId, resource) { // manually means that the embedder must have used getDefaultTriggerAsyncId(). if (triggerAsyncId === null) { triggerAsyncId = getDefaultTriggerAsyncId(); - } else { - // If a triggerAsyncId was passed, any kDefaultTriggerAsyncId still must be - // null'd. - async_id_fields[kDefaultTriggerAsyncId] = -1; } emitInitNative(asyncId, type, triggerAsyncId, resource); @@ -341,7 +344,7 @@ module.exports = { // Sensitive Embedder API newUid, getDefaultTriggerAsyncId, - setDefaultTriggerAsyncId, + defaultTriggerAsyncIdScope, emitInit: emitInitScript, emitBefore: emitBeforeScript, emitAfter: emitAfterScript, diff --git a/lib/net.js b/lib/net.js index bf2a3da97a4dd7..43eb15605ff965 100644 --- a/lib/net.js +++ b/lib/net.js @@ -39,7 +39,7 @@ const { TCPConnectWrap } = process.binding('tcp_wrap'); const { PipeConnectWrap } = process.binding('pipe_wrap'); const { ShutdownWrap, WriteWrap } = process.binding('stream_wrap'); const { async_id_symbol } = process.binding('async_wrap'); -const { newUid, setDefaultTriggerAsyncId } = require('internal/async_hooks'); +const { newUid, defaultTriggerAsyncIdScope } = require('internal/async_hooks'); const { nextTick } = require('internal/process/next_tick'); const errors = require('internal/errors'); const dns = require('dns'); @@ -270,6 +270,14 @@ Socket.prototype._unrefTimer = function _unrefTimer() { timers._unrefActive(s); }; + +function shutdownSocket(self, callback) { + var req = new ShutdownWrap(); + req.oncomplete = callback; + req.handle = self._handle; + return self._handle.shutdown(req); +} + // the user has called .end(), and all the bytes have been // sent out to the other side. function onSocketFinish() { @@ -291,14 +299,9 @@ function onSocketFinish() { if (!this._handle || !this._handle.shutdown) return this.destroy(); - var req = new ShutdownWrap(); - req.oncomplete = afterShutdown; - req.handle = this._handle; - // node::ShutdownWrap isn't instantiated and attached to the JS instance of - // ShutdownWrap above until shutdown() is called. So don't set the init - // trigger id until now. - setDefaultTriggerAsyncId(this[async_id_symbol]); - var err = this._handle.shutdown(req); + var err = defaultTriggerAsyncIdScope( + this[async_id_symbol], [this, afterShutdown], shutdownSocket + ); if (err) return this.destroy(errnoException(err, 'shutdown')); @@ -950,23 +953,15 @@ function internalConnect( req.localAddress = localAddress; req.localPort = localPort; - // node::TCPConnectWrap isn't instantiated and attached to the JS instance - // of TCPConnectWrap above until connect() is called. So don't set the init - // trigger id until now. - setDefaultTriggerAsyncId(self[async_id_symbol]); if (addressType === 4) err = self._handle.connect(req, address, port); else err = self._handle.connect6(req, address, port); - } else { const req = new PipeConnectWrap(); req.address = address; req.oncomplete = afterConnect; - // node::PipeConnectWrap isn't instantiated and attached to the JS instance - // of PipeConnectWrap above until connect() is called. So don't set the - // init trigger id until now. - setDefaultTriggerAsyncId(self[async_id_symbol]); + err = self._handle.connect(req, address, afterConnect); } @@ -1035,7 +1030,9 @@ Socket.prototype.connect = function(...args) { 'string', path); } - internalConnect(this, path); + defaultTriggerAsyncIdScope( + this[async_id_symbol], [this, path], internalConnect + ); } else { lookupAndConnect(this, options); } @@ -1074,7 +1071,11 @@ function lookupAndConnect(self, options) { if (addressType) { nextTick(self[async_id_symbol], function() { if (self.connecting) - internalConnect(self, host, port, addressType, localAddress, localPort); + defaultTriggerAsyncIdScope( + self[async_id_symbol], + [self, host, port, addressType, localAddress, localPort], + internalConnect + ); }); return; } @@ -1095,33 +1096,33 @@ function lookupAndConnect(self, options) { debug('connect: dns options', dnsopts); self._host = host; var lookup = options.lookup || dns.lookup; - setDefaultTriggerAsyncId(self[async_id_symbol]); - lookup(host, dnsopts, function emitLookup(err, ip, addressType) { - self.emit('lookup', err, ip, addressType, host); + defaultTriggerAsyncIdScope(self[async_id_symbol], [], function() { + lookup(host, dnsopts, function emitLookup(err, ip, addressType) { + self.emit('lookup', err, ip, addressType, host); - // It's possible we were destroyed while looking this up. - // XXX it would be great if we could cancel the promise returned by - // the look up. - if (!self.connecting) return; + // It's possible we were destroyed while looking this up. + // XXX it would be great if we could cancel the promise returned by + // the look up. + if (!self.connecting) return; - if (err) { - // net.createConnection() creates a net.Socket object and - // immediately calls net.Socket.connect() on it (that's us). - // There are no event listeners registered yet so defer the - // error event to the next tick. - err.host = options.host; - err.port = options.port; - err.message = err.message + ' ' + options.host + ':' + options.port; - process.nextTick(connectErrorNT, self, err); - } else { - self._unrefTimer(); - internalConnect(self, - ip, - port, - addressType, - localAddress, - localPort); - } + if (err) { + // net.createConnection() creates a net.Socket object and + // immediately calls net.Socket.connect() on it (that's us). + // There are no event listeners registered yet so defer the + // error event to the next tick. + err.host = options.host; + err.port = options.port; + err.message = err.message + ' ' + options.host + ':' + options.port; + process.nextTick(connectErrorNT, self, err); + } else { + self._unrefTimer(); + defaultTriggerAsyncIdScope( + self[async_id_symbol], + [self, ip, port, addressType, localAddress, localPort], + internalConnect + ); + } + }); }); } diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 897cc5a5518fac..bfbc8f222987d4 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -330,12 +330,13 @@ static void PromiseHook(PromiseHookType type, Local promise, if (parent_wrap == nullptr) { parent_wrap = PromiseWrap::New(env, parent_promise, nullptr, true); } - // get id from parentWrap - double trigger_async_id = parent_wrap->get_async_id(); - env->set_default_trigger_async_id(trigger_async_id); - } - wrap = PromiseWrap::New(env, promise, parent_wrap, silent); + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope( + env, parent_wrap->get_async_id()); + wrap = PromiseWrap::New(env, promise, parent_wrap, silent); + } else { + wrap = PromiseWrap::New(env, promise, nullptr, silent); + } } CHECK_NE(wrap, nullptr); diff --git a/src/connection_wrap.cc b/src/connection_wrap.cc index b7c1949e11e404..8de77f361dcde4 100644 --- a/src/connection_wrap.cc +++ b/src/connection_wrap.cc @@ -49,7 +49,6 @@ void ConnectionWrap::OnConnection(uv_stream_t* handle, }; if (status == 0) { - env->set_default_trigger_async_id(wrap_data->get_async_id()); // Instantiate the client javascript object and handle. Local client_obj = WrapType::Instantiate(env, wrap_data, diff --git a/src/env-inl.h b/src/env-inl.h index 736271a911c905..51f6ea14ccc666 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -197,23 +197,27 @@ inline void Environment::AsyncHooks::clear_async_id_stack() { async_id_fields_[kTriggerAsyncId] = 0; } -inline Environment::AsyncHooks::InitScope::InitScope( - Environment* env, double init_trigger_async_id) - : env_(env), - async_id_fields_ref_(env->async_hooks()->async_id_fields()) { - if (env_->async_hooks()->fields()[AsyncHooks::kCheck] > 0) { - CHECK_GE(init_trigger_async_id, -1); +inline Environment::AsyncHooks::DefaultTriggerAsyncIdScope + ::DefaultTriggerAsyncIdScope(Environment* env, + double default_trigger_async_id) + : async_id_fields_ref_(env->async_hooks()->async_id_fields()) { + if (env->async_hooks()->fields()[AsyncHooks::kCheck] > 0) { + CHECK_GE(default_trigger_async_id, 0); } - env->async_hooks()->push_async_ids( - async_id_fields_ref_[AsyncHooks::kExecutionAsyncId], - init_trigger_async_id); + + old_default_trigger_async_id_ = + async_id_fields_ref_[AsyncHooks::kDefaultTriggerAsyncId]; + async_id_fields_ref_[AsyncHooks::kDefaultTriggerAsyncId] = + default_trigger_async_id; } -inline Environment::AsyncHooks::InitScope::~InitScope() { - env_->async_hooks()->pop_async_id( - async_id_fields_ref_[AsyncHooks::kExecutionAsyncId]); +inline Environment::AsyncHooks::DefaultTriggerAsyncIdScope + ::~DefaultTriggerAsyncIdScope() { + async_id_fields_ref_[AsyncHooks::kDefaultTriggerAsyncId] = + old_default_trigger_async_id_; } + inline Environment::AsyncCallbackScope::AsyncCallbackScope(Environment* env) : env_(env) { env_->makecallback_cntr_++; @@ -473,17 +477,12 @@ inline double Environment::trigger_async_id() { inline double Environment::get_default_trigger_async_id() { double default_trigger_async_id = async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId]; - async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId] = -1; // If defaultTriggerAsyncId isn't set, use the executionAsyncId if (default_trigger_async_id < 0) default_trigger_async_id = execution_async_id(); return default_trigger_async_id; } -inline void Environment::set_default_trigger_async_id(const double id) { - async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId] = id; -} - inline double* Environment::heap_statistics_buffer() const { CHECK_NE(heap_statistics_buffer_, nullptr); return heap_statistics_buffer_; diff --git a/src/env.h b/src/env.h index a359c255bc59a8..bbeb86f71a7714 100644 --- a/src/env.h +++ b/src/env.h @@ -422,22 +422,23 @@ class Environment { inline size_t stack_size(); inline void clear_async_id_stack(); // Used in fatal exceptions. - // Used to propagate the trigger_async_id to the constructor of any newly - // created resources using RAII. Instead of needing to pass the - // trigger_async_id along with other constructor arguments. - class InitScope { + // Used to set the kDefaultTriggerAsyncId in a scope. This is instead of + // passing the trigger_async_id along with other constructor arguments. + class DefaultTriggerAsyncIdScope { public: - InitScope() = delete; - explicit InitScope(Environment* env, double init_trigger_async_id); - ~InitScope(); + DefaultTriggerAsyncIdScope() = delete; + explicit DefaultTriggerAsyncIdScope(Environment* env, + double init_trigger_async_id); + ~DefaultTriggerAsyncIdScope(); private: - Environment* env_; AliasedBuffer async_id_fields_ref_; + double old_default_trigger_async_id_; - DISALLOW_COPY_AND_ASSIGN(InitScope); + DISALLOW_COPY_AND_ASSIGN(DefaultTriggerAsyncIdScope); }; + private: friend class Environment; // So we can call the constructor. inline explicit AsyncHooks(v8::Isolate* isolate); @@ -594,7 +595,6 @@ class Environment { inline double execution_async_id(); inline double trigger_async_id(); inline double get_default_trigger_async_id(); - inline void set_default_trigger_async_id(const double id); // List of id's that have been destroyed and need the destroy() cb called. inline std::vector* destroy_async_id_list(); diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc index 281ac4d6838dbc..b4f01f08a647a8 100644 --- a/src/pipe_wrap.cc +++ b/src/pipe_wrap.cc @@ -52,7 +52,8 @@ Local PipeWrap::Instantiate(Environment* env, AsyncWrap* parent, PipeWrap::SocketType type) { EscapableHandleScope handle_scope(env->isolate()); - AsyncHooks::InitScope init_scope(env, parent->get_async_id()); + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(env, + parent->get_async_id()); CHECK_EQ(false, env->pipe_constructor_template().IsEmpty()); Local constructor = env->pipe_constructor_template()->GetFunction(); CHECK_EQ(false, constructor.IsEmpty()); diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h index 807e138ef7b6df..eacdb3832c0662 100644 --- a/src/stream_base-inl.h +++ b/src/stream_base-inl.h @@ -138,7 +138,8 @@ void StreamBase::JSMethod(const FunctionCallbackInfo& args) { if (!wrap->IsAlive()) return args.GetReturnValue().Set(UV_EINVAL); - AsyncHooks::InitScope init_scope(handle->env(), handle->get_async_id()); + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope( + handle->env(), handle->get_async_id()); args.GetReturnValue().Set((wrap->*Method)(args)); } diff --git a/src/stream_base.cc b/src/stream_base.cc index 11b83c67957da6..34564cadbe777a 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -52,7 +52,7 @@ int StreamBase::Shutdown(const FunctionCallbackInfo& args) { AsyncWrap* wrap = GetAsyncWrap(); CHECK_NE(wrap, nullptr); - env->set_default_trigger_async_id(wrap->get_async_id()); + AsyncHooks::DefaultTriggerAsyncIdScope(env, wrap->get_async_id()); ShutdownWrap* req_wrap = new ShutdownWrap(env, req_wrap_obj, this, @@ -111,7 +111,6 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { size_t storage_size = 0; uint32_t bytes = 0; size_t offset; - AsyncWrap* wrap; WriteWrap* req_wrap; int err; @@ -155,10 +154,14 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { goto done; } - wrap = GetAsyncWrap(); - CHECK_NE(wrap, nullptr); - env->set_default_trigger_async_id(wrap->get_async_id()); - req_wrap = WriteWrap::New(env, req_wrap_obj, this, AfterWrite, storage_size); + { + AsyncWrap* wrap = GetAsyncWrap(); + CHECK_NE(wrap, nullptr); + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(env, + wrap->get_async_id()); + req_wrap = WriteWrap::New(env, req_wrap_obj, this, AfterWrite, + storage_size); + } offset = 0; if (!all_buffers) { @@ -228,7 +231,6 @@ int StreamBase::WriteBuffer(const FunctionCallbackInfo& args) { const char* data = Buffer::Data(args[1]); size_t length = Buffer::Length(args[1]); - AsyncWrap* wrap; WriteWrap* req_wrap; uv_buf_t buf; buf.base = const_cast(data); @@ -244,11 +246,14 @@ int StreamBase::WriteBuffer(const FunctionCallbackInfo& args) { goto done; CHECK_EQ(count, 1); - wrap = GetAsyncWrap(); - if (wrap != nullptr) - env->set_default_trigger_async_id(wrap->get_async_id()); // Allocate, or write rest - req_wrap = WriteWrap::New(env, req_wrap_obj, this, AfterWrite); + { + AsyncWrap* wrap = GetAsyncWrap(); + CHECK_NE(wrap, nullptr); + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(env, + wrap->get_async_id()); + req_wrap = WriteWrap::New(env, req_wrap_obj, this, AfterWrite); + } err = DoWrite(req_wrap, bufs, count, nullptr); req_wrap_obj->Set(env->async(), True(env->isolate())); @@ -278,7 +283,6 @@ int StreamBase::WriteString(const FunctionCallbackInfo& args) { Local req_wrap_obj = args[0].As(); Local string = args[1].As(); Local send_handle_obj; - AsyncWrap* wrap; if (args[2]->IsObject()) send_handle_obj = args[2].As(); @@ -329,10 +333,14 @@ int StreamBase::WriteString(const FunctionCallbackInfo& args) { CHECK_EQ(count, 1); } - wrap = GetAsyncWrap(); - if (wrap != nullptr) - env->set_default_trigger_async_id(wrap->get_async_id()); - req_wrap = WriteWrap::New(env, req_wrap_obj, this, AfterWrite, storage_size); + { + AsyncWrap* wrap = GetAsyncWrap(); + CHECK_NE(wrap, nullptr); + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(env, + wrap->get_async_id()); + req_wrap = WriteWrap::New(env, req_wrap_obj, this, AfterWrite, + storage_size); + } data = req_wrap->Extra(); diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index 00853df9baf5c8..bdae0ee994360c 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -56,7 +56,8 @@ Local TCPWrap::Instantiate(Environment* env, AsyncWrap* parent, TCPWrap::SocketType type) { EscapableHandleScope handle_scope(env->isolate()); - AsyncHooks::InitScope init_scope(env, parent->get_async_id()); + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope( + env, parent->get_async_id()); CHECK_EQ(env->tcp_constructor_template().IsEmpty(), false); Local constructor = env->tcp_constructor_template()->GetFunction(); CHECK_EQ(constructor.IsEmpty(), false); @@ -293,7 +294,8 @@ void TCPWrap::Connect(const FunctionCallbackInfo& args) { int err = uv_ip4_addr(*ip_address, port, &addr); if (err == 0) { - env->set_default_trigger_async_id(wrap->get_async_id()); + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope( + env, wrap->get_async_id()); ConnectWrap* req_wrap = new ConnectWrap(env, req_wrap_obj, AsyncWrap::PROVIDER_TCPCONNECTWRAP); err = uv_tcp_connect(req_wrap->req(), @@ -329,7 +331,8 @@ void TCPWrap::Connect6(const FunctionCallbackInfo& args) { int err = uv_ip6_addr(*ip_address, port, &addr); if (err == 0) { - env->set_default_trigger_async_id(wrap->get_async_id()); + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope( + env, wrap->get_async_id()); ConnectWrap* req_wrap = new ConnectWrap(env, req_wrap_obj, AsyncWrap::PROVIDER_TCPCONNECTWRAP); err = uv_tcp_connect(req_wrap->req(), diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index 5ba2be37c7a601..784b36fce95b21 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -347,8 +347,12 @@ void UDPWrap::DoSend(const FunctionCallbackInfo& args, int family) { node::Utf8Value address(env->isolate(), args[4]); const bool have_callback = args[5]->IsTrue(); - env->set_default_trigger_async_id(wrap->get_async_id()); - SendWrap* req_wrap = new SendWrap(env, req_wrap_obj, have_callback); + SendWrap* req_wrap; + { + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope( + env, wrap->get_async_id()); + req_wrap = new SendWrap(env, req_wrap_obj, have_callback); + } size_t msg_size = 0; MaybeStackBuffer bufs(count); @@ -497,7 +501,9 @@ Local UDPWrap::Instantiate(Environment* env, AsyncWrap* parent, UDPWrap::SocketType type) { EscapableHandleScope scope(env->isolate()); - AsyncHooks::InitScope init_scope(env, parent->get_async_id()); + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope( + env, parent->get_async_id()); + // If this assert fires then Initialize hasn't been called yet. CHECK_EQ(env->udp_constructor_function().IsEmpty(), false); Local instance = env->udp_constructor_function() From 7310f668f8d2471f39c357564971a474b5dff5c1 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 19 Dec 2017 15:04:07 +0100 Subject: [PATCH 16/22] src: remove unused async hooks methods PR-URL: https://github.com/nodejs/node/pull/17757 Reviewed-By: Anatoli Papirovski Reviewed-By: Colin Ihrig Reviewed-By: Daniel Bevenius Reviewed-By: James M Snell Reviewed-By: Ruben Bridgewater Reviewed-By: Jon Moss Reviewed-By: Yuta Hiroto Reviewed-By: Timothy Gu Reviewed-By: vdeturckheim --- src/env-inl.h | 8 -------- src/env.h | 3 --- 2 files changed, 11 deletions(-) diff --git a/src/env-inl.h b/src/env-inl.h index 51f6ea14ccc666..effe7523cd41fc 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -117,19 +117,11 @@ Environment::AsyncHooks::fields() { return fields_; } -inline int Environment::AsyncHooks::fields_count() const { - return kFieldsCount; -} - inline AliasedBuffer& Environment::AsyncHooks::async_id_fields() { return async_id_fields_; } -inline int Environment::AsyncHooks::async_id_fields_count() const { - return kUidFieldsCount; -} - inline v8::Local Environment::AsyncHooks::provider_string(int idx) { return providers_[idx].Get(isolate_); } diff --git a/src/env.h b/src/env.h index bbeb86f71a7714..9faffce36f776e 100644 --- a/src/env.h +++ b/src/env.h @@ -408,10 +408,7 @@ class Environment { AsyncHooks() = delete; inline AliasedBuffer& fields(); - inline int fields_count() const; - inline AliasedBuffer& async_id_fields(); - inline int async_id_fields_count() const; inline v8::Local provider_string(int idx); From 4e656fd7183acd24511a35531888cfb0e833d74b Mon Sep 17 00:00:00 2001 From: Jon Moss Date: Fri, 22 Dec 2017 22:30:20 -0500 Subject: [PATCH 17/22] async_hooks: use CHECK instead of throwing error SetupHooks is only available via `process.binding('async_wrap')`, so there's no reason it shouldn't be called with the appropriate arguments, since it is an internal-only function. The only place this function is used is `lib/internal/async_hooks.js`. PR-URL: https://github.com/nodejs/node/pull/17832 Reviewed-By: Anna Henningsen Reviewed-By: Anatoli Papirovski Reviewed-By: Colin Ihrig Reviewed-By: Gireesh Punathil Reviewed-By: Luigi Pinca --- src/async_wrap.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/async_wrap.cc b/src/async_wrap.cc index bfbc8f222987d4..3b0727cdfbcf0a 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -365,8 +365,7 @@ static void PromiseHook(PromiseHookType type, Local promise, static void SetupHooks(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - if (!args[0]->IsObject()) - return env->ThrowTypeError("first argument must be an object"); + CHECK(args[0]->IsObject()); // All of init, before, after, destroy are supplied by async_hooks // internally, so this should every only be called once. At which time all From 067bd2ac5b8a402b823ab0bc8024288d26b5b809 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 19 Dec 2017 15:08:18 +0100 Subject: [PATCH 18/22] async_hooks: use typed array stack as fast path - Communicate the current async stack length through a typed array field rather than a native binding method - Add a new fixed-size `async_ids_fast_stack` typed array that contains the async ID stack up to a fixed limit. This increases performance noticeably, since most of the time the async ID stack will not be more than a handful of levels deep. - Make the JS `pushAsyncIds()` and `popAsyncIds()` functions do the same thing as the native ones if the fast path is applicable. Benchmarks: $ ./node benchmark/compare.js --new ./node --old ./node-master --runs 10 --filter next-tick process | Rscript benchmark/compare.R [00:03:25|% 100| 6/6 files | 20/20 runs | 1/1 configs]: Done improvement confidence p.value process/next-tick-breadth-args.js millions=4 19.72 % *** 3.013913e-06 process/next-tick-breadth.js millions=4 27.33 % *** 5.847983e-11 process/next-tick-depth-args.js millions=12 40.08 % *** 1.237127e-13 process/next-tick-depth.js millions=12 77.27 % *** 1.413290e-11 process/next-tick-exec-args.js millions=5 13.58 % *** 1.245180e-07 process/next-tick-exec.js millions=5 16.80 % *** 2.961386e-07 PR-URL: https://github.com/nodejs/node/pull/17780 Reviewed-By: James M Snell --- lib/internal/async_hooks.js | 44 ++++++++++++- lib/internal/bootstrap_node.js | 6 +- src/aliased_buffer.h | 29 ++++++++- src/async_wrap.cc | 14 ++--- src/async_wrap.h | 1 - src/env-inl.h | 62 ++++++++++++------- src/env.cc | 17 +++++ src/env.h | 26 ++++---- .../test-async-hooks-recursive-stack.js | 20 ++++++ 9 files changed, 165 insertions(+), 54 deletions(-) create mode 100644 test/parallel/test-async-hooks-recursive-stack.js diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 366a218b7a95c2..8307f67f66d52a 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -19,6 +19,12 @@ const async_wrap = process.binding('async_wrap'); * retrieving the triggerAsyncId value is passing directly to the * constructor -> value set in kDefaultTriggerAsyncId -> executionAsyncId of * the current resource. + * + * async_ids_fast_stack is a Float64Array that contains part of the async ID + * stack. Each pushAsyncIds() call adds two doubles to it, and each + * popAsyncIds() call removes two doubles from it. + * It has a fixed size, so if that is exceeded, calls to the native + * side are used instead in pushAsyncIds() and popAsyncIds(). */ const { async_hook_fields, async_id_fields } = async_wrap; // Store the pair executionAsyncId and triggerAsyncId in a std::stack on @@ -26,7 +32,7 @@ const { async_hook_fields, async_id_fields } = async_wrap; // current execution stack. This is unwound as each resource exits. In the case // of a fatal exception this stack is emptied after calling each hook's after() // callback. -const { pushAsyncIds, popAsyncIds } = async_wrap; +const { pushAsyncIds: pushAsyncIds_, popAsyncIds: popAsyncIds_ } = async_wrap; // For performance reasons, only track Proimses when a hook is enabled. const { enablePromiseHook, disablePromiseHook } = async_wrap; // Properties in active_hooks are used to keep track of the set of hooks being @@ -60,8 +66,8 @@ const active_hooks = { // async execution. These are tracked so if the user didn't include callbacks // for a given step, that step can bail out early. const { kInit, kBefore, kAfter, kDestroy, kPromiseResolve, - kCheck, kExecutionAsyncId, kAsyncIdCounter, - kDefaultTriggerAsyncId } = async_wrap.constants; + kCheck, kExecutionAsyncId, kAsyncIdCounter, kTriggerAsyncId, + kDefaultTriggerAsyncId, kStackLength } = async_wrap.constants; // Used in AsyncHook and AsyncResource. const init_symbol = Symbol('init'); @@ -332,6 +338,38 @@ function emitDestroyScript(asyncId) { } +// This is the equivalent of the native push_async_ids() call. +function pushAsyncIds(asyncId, triggerAsyncId) { + const offset = async_hook_fields[kStackLength]; + if (offset * 2 >= async_wrap.async_ids_stack.length) + return pushAsyncIds_(asyncId, triggerAsyncId); + async_wrap.async_ids_stack[offset * 2] = async_id_fields[kExecutionAsyncId]; + async_wrap.async_ids_stack[offset * 2 + 1] = async_id_fields[kTriggerAsyncId]; + async_hook_fields[kStackLength]++; + async_id_fields[kExecutionAsyncId] = asyncId; + async_id_fields[kTriggerAsyncId] = triggerAsyncId; +} + + +// This is the equivalent of the native pop_async_ids() call. +function popAsyncIds(asyncId) { + if (async_hook_fields[kStackLength] === 0) return false; + const stackLength = async_hook_fields[kStackLength]; + + if (async_hook_fields[kCheck] > 0 && + async_id_fields[kExecutionAsyncId] !== asyncId) { + // Do the same thing as the native code (i.e. crash hard). + return popAsyncIds_(asyncId); + } + + const offset = stackLength - 1; + async_id_fields[kExecutionAsyncId] = async_wrap.async_ids_stack[2 * offset]; + async_id_fields[kTriggerAsyncId] = async_wrap.async_ids_stack[2 * offset + 1]; + async_hook_fields[kStackLength] = offset; + return offset > 0; +} + + module.exports = { // Private API getHookArrays, diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index 6a8713c986b076..9af547a923022b 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -357,9 +357,9 @@ // Arrays containing hook flags and ids for async_hook calls. const { async_hook_fields, async_id_fields } = async_wrap; // Internal functions needed to manipulate the stack. - const { clearAsyncIdStack, asyncIdStackSize } = async_wrap; + const { clearAsyncIdStack } = async_wrap; const { kAfter, kExecutionAsyncId, - kDefaultTriggerAsyncId } = async_wrap.constants; + kDefaultTriggerAsyncId, kStackLength } = async_wrap.constants; process._fatalException = function(er) { var caught; @@ -395,7 +395,7 @@ do { NativeModule.require('internal/async_hooks').emitAfter( async_id_fields[kExecutionAsyncId]); - } while (asyncIdStackSize() > 0); + } while (async_hook_fields[kStackLength] > 0); // Or completely empty the id stack. } else { clearAsyncIdStack(); diff --git a/src/aliased_buffer.h b/src/aliased_buffer.h index 21aaeb61141c59..b99b01f5d94ca2 100644 --- a/src/aliased_buffer.h +++ b/src/aliased_buffer.h @@ -95,6 +95,21 @@ class AliasedBuffer { js_array_.Reset(); } + AliasedBuffer& operator=(AliasedBuffer&& that) { + this->~AliasedBuffer(); + isolate_ = that.isolate_; + count_ = that.count_; + byte_offset_ = that.byte_offset_; + buffer_ = that.buffer_; + free_buffer_ = that.free_buffer_; + + js_array_.Reset(isolate_, that.js_array_.Get(isolate_)); + + that.buffer_ = nullptr; + that.js_array_.Reset(); + return *this; + } + /** * Helper class that is returned from operator[] to support assignment into * a specified location. @@ -111,11 +126,17 @@ class AliasedBuffer { index_(that.index_) { } - inline Reference& operator=(const NativeT &val) { + template + inline Reference& operator=(const T& val) { aliased_buffer_->SetValue(index_, val); return *this; } + // This is not caught by the template operator= above. + inline Reference& operator=(const Reference& val) { + return *this = static_cast(val); + } + operator NativeT() const { return aliased_buffer_->GetValue(index_); } @@ -186,8 +207,12 @@ class AliasedBuffer { return GetValue(index); } + size_t Length() const { + return count_; + } + private: - v8::Isolate* const isolate_; + v8::Isolate* isolate_; size_t count_; size_t byte_offset_; NativeT* buffer_; diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 3b0727cdfbcf0a..7e3c0c257ab22d 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -489,13 +489,6 @@ void AsyncWrap::PopAsyncIds(const FunctionCallbackInfo& args) { } -void AsyncWrap::AsyncIdStackSize(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - args.GetReturnValue().Set( - static_cast(env->async_hooks()->stack_size())); -} - - void AsyncWrap::ClearAsyncIdStack(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); env->async_hooks()->clear_async_id_stack(); @@ -534,7 +527,6 @@ void AsyncWrap::Initialize(Local target, env->SetMethod(target, "setupHooks", SetupHooks); env->SetMethod(target, "pushAsyncIds", PushAsyncIds); env->SetMethod(target, "popAsyncIds", PopAsyncIds); - env->SetMethod(target, "asyncIdStackSize", AsyncIdStackSize); env->SetMethod(target, "clearAsyncIdStack", ClearAsyncIdStack); env->SetMethod(target, "queueDestroyAsyncId", QueueDestroyAsyncId); env->SetMethod(target, "enablePromiseHook", EnablePromiseHook); @@ -572,6 +564,10 @@ void AsyncWrap::Initialize(Local target, "async_id_fields", env->async_hooks()->async_id_fields().GetJSArray()); + target->Set(context, + env->async_ids_stack_string(), + env->async_hooks()->async_ids_stack().GetJSArray()).FromJust(); + Local constants = Object::New(isolate); #define SET_HOOKS_CONSTANT(name) \ FORCE_SET_TARGET_FIELD( \ @@ -588,6 +584,7 @@ void AsyncWrap::Initialize(Local target, SET_HOOKS_CONSTANT(kTriggerAsyncId); SET_HOOKS_CONSTANT(kAsyncIdCounter); SET_HOOKS_CONSTANT(kDefaultTriggerAsyncId); + SET_HOOKS_CONSTANT(kStackLength); #undef SET_HOOKS_CONSTANT FORCE_SET_TARGET_FIELD(target, "constants", constants); @@ -617,6 +614,7 @@ void AsyncWrap::Initialize(Local target, env->set_async_hooks_after_function(Local()); env->set_async_hooks_destroy_function(Local()); env->set_async_hooks_promise_resolve_function(Local()); + env->set_async_hooks_binding(target); } diff --git a/src/async_wrap.h b/src/async_wrap.h index 773a0ad15ce9e9..bc826768d92736 100644 --- a/src/async_wrap.h +++ b/src/async_wrap.h @@ -123,7 +123,6 @@ class AsyncWrap : public BaseObject { static void GetAsyncId(const v8::FunctionCallbackInfo& args); static void PushAsyncIds(const v8::FunctionCallbackInfo& args); static void PopAsyncIds(const v8::FunctionCallbackInfo& args); - static void AsyncIdStackSize(const v8::FunctionCallbackInfo& args); static void ClearAsyncIdStack( const v8::FunctionCallbackInfo& args); static void AsyncReset(const v8::FunctionCallbackInfo& args); diff --git a/src/env-inl.h b/src/env-inl.h index effe7523cd41fc..b61634774a64ad 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -81,11 +81,11 @@ inline uint32_t* IsolateData::zero_fill_field() const { return zero_fill_field_; } -inline Environment::AsyncHooks::AsyncHooks(v8::Isolate* isolate) - : isolate_(isolate), - fields_(isolate, kFieldsCount), - async_id_fields_(isolate, kUidFieldsCount) { - v8::HandleScope handle_scope(isolate_); +inline Environment::AsyncHooks::AsyncHooks() + : async_ids_stack_(env()->isolate(), 16 * 2), + fields_(env()->isolate(), kFieldsCount), + async_id_fields_(env()->isolate(), kUidFieldsCount) { + v8::HandleScope handle_scope(env()->isolate()); // kDefaultTriggerAsyncId should be -1, this indicates that there is no // specified default value and it should fallback to the executionAsyncId. @@ -102,9 +102,9 @@ inline Environment::AsyncHooks::AsyncHooks(v8::Isolate* isolate) // strings can be retrieved quickly. #define V(Provider) \ providers_[AsyncWrap::PROVIDER_ ## Provider].Set( \ - isolate_, \ + env()->isolate(), \ v8::String::NewFromOneByte( \ - isolate_, \ + env()->isolate(), \ reinterpret_cast(#Provider), \ v8::NewStringType::kInternalized, \ sizeof(#Provider) - 1).ToLocalChecked()); @@ -122,8 +122,13 @@ Environment::AsyncHooks::async_id_fields() { return async_id_fields_; } +inline AliasedBuffer& +Environment::AsyncHooks::async_ids_stack() { + return async_ids_stack_; +} + inline v8::Local Environment::AsyncHooks::provider_string(int idx) { - return providers_[idx].Get(isolate_); + return providers_[idx].Get(env()->isolate()); } inline void Environment::AsyncHooks::force_checks() { @@ -131,6 +136,11 @@ inline void Environment::AsyncHooks::force_checks() { fields_[kCheck] = fields_[kCheck] + 1; } +inline Environment* Environment::AsyncHooks::env() { + return Environment::ForAsyncHooks(this); +} + +// Remember to keep this code aligned with pushAsyncIds() in JS. inline void Environment::AsyncHooks::push_async_ids(double async_id, double trigger_async_id) { // Since async_hooks is experimental, do only perform the check @@ -140,16 +150,21 @@ inline void Environment::AsyncHooks::push_async_ids(double async_id, CHECK_GE(trigger_async_id, -1); } - async_ids_stack_.push({ async_id_fields_[kExecutionAsyncId], - async_id_fields_[kTriggerAsyncId] }); + uint32_t offset = fields_[kStackLength]; + if (offset * 2 >= async_ids_stack_.Length()) + grow_async_ids_stack(); + async_ids_stack_[2 * offset] = async_id_fields_[kExecutionAsyncId]; + async_ids_stack_[2 * offset + 1] = async_id_fields_[kTriggerAsyncId]; + fields_[kStackLength] = fields_[kStackLength] + 1; async_id_fields_[kExecutionAsyncId] = async_id; async_id_fields_[kTriggerAsyncId] = trigger_async_id; } +// Remember to keep this code aligned with popAsyncIds() in JS. inline bool Environment::AsyncHooks::pop_async_id(double async_id) { // In case of an exception then this may have already been reset, if the // stack was multiple MakeCallback()'s deep. - if (async_ids_stack_.empty()) return false; + if (fields_[kStackLength] == 0) return false; // Ask for the async_id to be restored as a check that the stack // hasn't been corrupted. @@ -161,32 +176,27 @@ inline bool Environment::AsyncHooks::pop_async_id(double async_id) { "actual: %.f, expected: %.f)\n", async_id_fields_.GetValue(kExecutionAsyncId), async_id); - Environment* env = Environment::GetCurrent(isolate_); DumpBacktrace(stderr); fflush(stderr); - if (!env->abort_on_uncaught_exception()) + if (!env()->abort_on_uncaught_exception()) exit(1); fprintf(stderr, "\n"); fflush(stderr); ABORT_NO_BACKTRACE(); } - auto async_ids = async_ids_stack_.top(); - async_ids_stack_.pop(); - async_id_fields_[kExecutionAsyncId] = async_ids.async_id; - async_id_fields_[kTriggerAsyncId] = async_ids.trigger_async_id; - return !async_ids_stack_.empty(); -} + uint32_t offset = fields_[kStackLength] - 1; + async_id_fields_[kExecutionAsyncId] = async_ids_stack_[2 * offset]; + async_id_fields_[kTriggerAsyncId] = async_ids_stack_[2 * offset + 1]; + fields_[kStackLength] = offset; -inline size_t Environment::AsyncHooks::stack_size() { - return async_ids_stack_.size(); + return fields_[kStackLength] > 0; } inline void Environment::AsyncHooks::clear_async_id_stack() { - while (!async_ids_stack_.empty()) - async_ids_stack_.pop(); async_id_fields_[kExecutionAsyncId] = 0; async_id_fields_[kTriggerAsyncId] = 0; + fields_[kStackLength] = 0; } inline Environment::AsyncHooks::DefaultTriggerAsyncIdScope @@ -210,6 +220,11 @@ inline Environment::AsyncHooks::DefaultTriggerAsyncIdScope } +Environment* Environment::ForAsyncHooks(AsyncHooks* hooks) { + return ContainerOf(&Environment::async_hooks_, hooks); +} + + inline Environment::AsyncCallbackScope::AsyncCallbackScope(Environment* env) : env_(env) { env_->makecallback_cntr_++; @@ -298,7 +313,6 @@ inline Environment::Environment(IsolateData* isolate_data, v8::Local context) : isolate_(context->GetIsolate()), isolate_data_(isolate_data), - async_hooks_(context->GetIsolate()), timer_base_(uv_now(isolate_data->event_loop())), using_domains_(false), printed_error_(false), diff --git a/src/env.cc b/src/env.cc index 3f6abd11284a11..3212dcc46cd4d0 100644 --- a/src/env.cc +++ b/src/env.cc @@ -269,4 +269,21 @@ void Environment::ActivateImmediateCheck() { uv_idle_start(&immediate_idle_handle_, [](uv_idle_t*){ }); } + +void Environment::AsyncHooks::grow_async_ids_stack() { + const uint32_t old_capacity = async_ids_stack_.Length() / 2; + const uint32_t new_capacity = old_capacity * 1.5; + AliasedBuffer new_buffer( + env()->isolate(), new_capacity * 2); + + for (uint32_t i = 0; i < old_capacity * 2; ++i) + new_buffer[i] = async_ids_stack_[i]; + async_ids_stack_ = std::move(new_buffer); + + env()->async_hooks_binding()->Set( + env()->context(), + env()->async_ids_stack_string(), + async_ids_stack_.GetJSArray()).FromJust(); +} + } // namespace node diff --git a/src/env.h b/src/env.h index 9faffce36f776e..40b8c666b7bdf4 100644 --- a/src/env.h +++ b/src/env.h @@ -41,7 +41,6 @@ #include #include #include -#include #include struct nghttp2_rcbuf; @@ -101,6 +100,7 @@ class ModuleWrap; V(address_string, "address") \ V(args_string, "args") \ V(async, "async") \ + V(async_ids_stack_string, "async_ids_stack") \ V(buffer_string, "buffer") \ V(bytes_string, "bytes") \ V(bytes_parsed_string, "bytesParsed") \ @@ -306,6 +306,7 @@ class ModuleWrap; V(async_hooks_before_function, v8::Function) \ V(async_hooks_after_function, v8::Function) \ V(async_hooks_promise_resolve_function, v8::Function) \ + V(async_hooks_binding, v8::Object) \ V(binding_cache_object, v8::Object) \ V(internal_binding_cache_object, v8::Object) \ V(buffer_prototype_object, v8::Object) \ @@ -339,11 +340,6 @@ class ModuleWrap; class Environment; -struct node_async_ids { - double async_id; - double trigger_async_id; -}; - class IsolateData { public: inline IsolateData(v8::Isolate* isolate, uv_loop_t* event_loop, @@ -394,6 +390,7 @@ class Environment { kPromiseResolve, kTotals, kCheck, + kStackLength, kFieldsCount, }; @@ -405,18 +402,17 @@ class Environment { kUidFieldsCount, }; - AsyncHooks() = delete; - inline AliasedBuffer& fields(); inline AliasedBuffer& async_id_fields(); + inline AliasedBuffer& async_ids_stack(); inline v8::Local provider_string(int idx); inline void force_checks(); + inline Environment* env(); inline void push_async_ids(double async_id, double trigger_async_id); inline bool pop_async_id(double async_id); - inline size_t stack_size(); inline void clear_async_id_stack(); // Used in fatal exceptions. // Used to set the kDefaultTriggerAsyncId in a scope. This is instead of @@ -438,19 +434,21 @@ class Environment { private: friend class Environment; // So we can call the constructor. - inline explicit AsyncHooks(v8::Isolate* isolate); + inline AsyncHooks(); // Keep a list of all Persistent strings used for Provider types. v8::Eternal providers_[AsyncWrap::PROVIDERS_LENGTH]; - // Used by provider_string(). - v8::Isolate* isolate_; + // Keep track of the environment copy itself. + Environment* env_; // Stores the ids of the current execution context stack. - std::stack async_ids_stack_; + AliasedBuffer async_ids_stack_; // Attached to a Uint32Array that tracks the number of active hooks for // each type. AliasedBuffer fields_; // Attached to a Float64Array that tracks the state of async resources. AliasedBuffer async_id_fields_; + void grow_async_ids_stack(); + DISALLOW_COPY_AND_ASSIGN(AsyncHooks); }; @@ -692,6 +690,8 @@ class Environment { // This needs to be available for the JS-land setImmediate(). void ActivateImmediateCheck(); + static inline Environment* ForAsyncHooks(AsyncHooks* hooks); + private: inline void ThrowError(v8::Local (*fun)(v8::Local), const char* errmsg); diff --git a/test/parallel/test-async-hooks-recursive-stack.js b/test/parallel/test-async-hooks-recursive-stack.js new file mode 100644 index 00000000000000..7ab73dc1bf4538 --- /dev/null +++ b/test/parallel/test-async-hooks-recursive-stack.js @@ -0,0 +1,20 @@ +'use strict'; +require('../common'); +const assert = require('assert'); +const async_hooks = require('async_hooks'); + +// This test verifies that the async ID stack can grow indefinitely. + +function recurse(n) { + const a = new async_hooks.AsyncResource('foobar'); + a.emitBefore(); + assert.strictEqual(a.asyncId(), async_hooks.executionAsyncId()); + assert.strictEqual(a.triggerAsyncId(), async_hooks.triggerAsyncId()); + if (n >= 0) + recurse(n - 1); + assert.strictEqual(a.asyncId(), async_hooks.executionAsyncId()); + assert.strictEqual(a.triggerAsyncId(), async_hooks.triggerAsyncId()); + a.emitAfter(); +} + +recurse(1000); From af928ef3b3f368ff439ab29a92da880c6b55a4aa Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Fri, 5 Jan 2018 16:40:18 +0100 Subject: [PATCH 19/22] trace_events: stop tracing agent in process.exit() PR-URL: https://github.com/nodejs/node/pull/18005 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Gireesh Punathil --- src/node.cc | 3 +++ .../test-trace-events-process-exit.js | 22 +++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 test/parallel/test-trace-events-process-exit.js diff --git a/src/node.cc b/src/node.cc index e37f8bae72c409..cebcf100c80306 100644 --- a/src/node.cc +++ b/src/node.cc @@ -2464,6 +2464,9 @@ static void WaitForInspectorDisconnect(Environment* env) { static void Exit(const FunctionCallbackInfo& args) { WaitForInspectorDisconnect(Environment::GetCurrent(args)); + if (trace_enabled) { + v8_platform.StopTracingAgent(); + } exit(args[0]->Int32Value()); } diff --git a/test/parallel/test-trace-events-process-exit.js b/test/parallel/test-trace-events-process-exit.js new file mode 100644 index 00000000000000..be45cb1d3e0f22 --- /dev/null +++ b/test/parallel/test-trace-events-process-exit.js @@ -0,0 +1,22 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cp = require('child_process'); +const fs = require('fs'); + +const FILE_NAME = 'node_trace.1.log'; + +common.refreshTmpDir(); +process.chdir(common.tmpDir); + +const proc = cp.spawn(process.execPath, + [ '--trace-events-enabled', + '-e', 'process.exit()' ]); + +proc.once('exit', common.mustCall(() => { + assert(common.fileExists(FILE_NAME)); + fs.readFile(FILE_NAME, common.mustCall((err, data) => { + const traces = JSON.parse(data.toString()).traceEvents; + assert(traces.length > 0); + })); +})); From 1efac0ed6a50af4e3763c7e949d95478a2cbba80 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Fri, 5 Jan 2018 09:03:10 -0500 Subject: [PATCH 20/22] async_hooks: update defaultTriggerAsyncIdScope for perf The existing version of defaultTriggerAsyncIdScope creates an Array for the callback's arguments which is highly inefficient. Instead, use rest syntax and allow V8 to do that work for us. This yields roughly 2x performance for this particular function. PR-URL: https://github.com/nodejs/node/pull/18004 Reviewed-By: Andreas Madsen Reviewed-By: Luigi Pinca Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig --- lib/dgram.js | 4 ++-- lib/internal/async_hooks.js | 4 ++-- lib/net.js | 14 +++++++------- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/dgram.js b/lib/dgram.js index 04828155f3a0d7..af1fd986fa5cdd 100644 --- a/lib/dgram.js +++ b/lib/dgram.js @@ -450,8 +450,8 @@ Socket.prototype.send = function(buffer, const afterDns = (ex, ip) => { defaultTriggerAsyncIdScope( this[async_id_symbol], - [ex, this, ip, list, address, port, callback], - doSend + doSend, + ex, this, ip, list, address, port, callback ); }; diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 8307f67f66d52a..e0c7a44fc45742 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -263,7 +263,7 @@ function getDefaultTriggerAsyncId() { } -function defaultTriggerAsyncIdScope(triggerAsyncId, opaque, block) { +function defaultTriggerAsyncIdScope(triggerAsyncId, block, ...args) { // CHECK(Number.isSafeInteger(triggerAsyncId)) // CHECK(triggerAsyncId > 0) const oldDefaultTriggerAsyncId = async_id_fields[kDefaultTriggerAsyncId]; @@ -271,7 +271,7 @@ function defaultTriggerAsyncIdScope(triggerAsyncId, opaque, block) { var ret; try { - ret = Reflect.apply(block, null, opaque); + ret = Reflect.apply(block, null, args); } finally { async_id_fields[kDefaultTriggerAsyncId] = oldDefaultTriggerAsyncId; } diff --git a/lib/net.js b/lib/net.js index 43eb15605ff965..fb8a57424351ae 100644 --- a/lib/net.js +++ b/lib/net.js @@ -300,7 +300,7 @@ function onSocketFinish() { return this.destroy(); var err = defaultTriggerAsyncIdScope( - this[async_id_symbol], [this, afterShutdown], shutdownSocket + this[async_id_symbol], shutdownSocket, this, afterShutdown ); if (err) @@ -1031,7 +1031,7 @@ Socket.prototype.connect = function(...args) { path); } defaultTriggerAsyncIdScope( - this[async_id_symbol], [this, path], internalConnect + this[async_id_symbol], internalConnect, this, path ); } else { lookupAndConnect(this, options); @@ -1073,8 +1073,8 @@ function lookupAndConnect(self, options) { if (self.connecting) defaultTriggerAsyncIdScope( self[async_id_symbol], - [self, host, port, addressType, localAddress, localPort], - internalConnect + internalConnect, + self, host, port, addressType, localAddress, localPort ); }); return; @@ -1096,7 +1096,7 @@ function lookupAndConnect(self, options) { debug('connect: dns options', dnsopts); self._host = host; var lookup = options.lookup || dns.lookup; - defaultTriggerAsyncIdScope(self[async_id_symbol], [], function() { + defaultTriggerAsyncIdScope(self[async_id_symbol], function() { lookup(host, dnsopts, function emitLookup(err, ip, addressType) { self.emit('lookup', err, ip, addressType, host); @@ -1118,8 +1118,8 @@ function lookupAndConnect(self, options) { self._unrefTimer(); defaultTriggerAsyncIdScope( self[async_id_symbol], - [self, ip, port, addressType, localAddress, localPort], - internalConnect + internalConnect, + self, ip, port, addressType, localAddress, localPort ); } }); From 3ba594f855ef7af84ecf293ea0ace49cacbf94ec Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Fri, 5 Jan 2018 15:40:47 +0100 Subject: [PATCH 21/22] async_hooks,http: set HTTPParser trigger to socket This allows more easy tracking of where HTTP requests come from. Before this change the HTTPParser would have the HTTPServer as the triggerAsyncId. The HTTPParser will still have the executionAsyncId set to the HTTP Server so that information is still directly available. Indirectly, the TCP socket itself also has its triggerAsyncId set to the TCP Server. PR-URL: https://github.com/nodejs/node/pull/18003 Reviewed-By: Anna Henningsen Reviewed-By: Anatoli Papirovski Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Jon Moss Reviewed-By: Daijiro Wachi --- lib/_http_server.js | 28 ++++++++++----- lib/internal/async_hooks.js | 12 ++++++- test/async-hooks/test-graph.http.js | 56 +++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 10 deletions(-) create mode 100644 test/async-hooks/test-graph.http.js diff --git a/lib/_http_server.js b/lib/_http_server.js index 3a472aa72664ac..dcfea2f1b17e3d 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -37,6 +37,10 @@ const { } = require('_http_common'); const { OutgoingMessage } = require('_http_outgoing'); const { outHeadersKey, ondrain } = require('internal/http'); +const { + defaultTriggerAsyncIdScope, + getOrSetAsyncId +} = require('internal/async_hooks'); const STATUS_CODES = { 100: 'Continue', @@ -289,6 +293,12 @@ Server.prototype.setTimeout = function setTimeout(msecs, callback) { function connectionListener(socket) { + defaultTriggerAsyncIdScope( + getOrSetAsyncId(socket), connectionListenerInternal, this, socket + ); +} + +function connectionListenerInternal(server, socket) { debug('SERVER new http connection'); httpSocketSetup(socket); @@ -296,13 +306,13 @@ function connectionListener(socket) { // Ensure that the server property of the socket is correctly set. // See https://github.com/nodejs/node/issues/13435 if (socket.server === null) - socket.server = this; + socket.server = server; // If the user has added a listener to the server, // request, or response, then it's their responsibility. // otherwise, destroy on timeout by default - if (this.timeout && typeof socket.setTimeout === 'function') - socket.setTimeout(this.timeout); + if (server.timeout && typeof socket.setTimeout === 'function') + socket.setTimeout(server.timeout); socket.on('timeout', socketOnTimeout); var parser = parsers.alloc(); @@ -312,8 +322,8 @@ function connectionListener(socket) { parser.incoming = null; // Propagate headers limit from server instance to parser - if (typeof this.maxHeadersCount === 'number') { - parser.maxHeaderPairs = this.maxHeadersCount << 1; + if (typeof server.maxHeadersCount === 'number') { + parser.maxHeaderPairs = server.maxHeadersCount << 1; } else { // Set default value because parser may be reused from FreeList parser.maxHeaderPairs = 2000; @@ -333,8 +343,8 @@ function connectionListener(socket) { outgoingData: 0, keepAliveTimeoutSet: false }; - state.onData = socketOnData.bind(undefined, this, socket, parser, state); - state.onEnd = socketOnEnd.bind(undefined, this, socket, parser, state); + state.onData = socketOnData.bind(undefined, server, socket, parser, state); + state.onEnd = socketOnEnd.bind(undefined, server, socket, parser, state); state.onClose = socketOnClose.bind(undefined, socket, state); state.onDrain = socketOnDrain.bind(undefined, socket, state); socket.on('data', state.onData); @@ -342,7 +352,7 @@ function connectionListener(socket) { socket.on('end', state.onEnd); socket.on('close', state.onClose); socket.on('drain', state.onDrain); - parser.onIncoming = parserOnIncoming.bind(undefined, this, socket, state); + parser.onIncoming = parserOnIncoming.bind(undefined, server, socket, state); // We are consuming socket, so it won't get any actual data socket.on('resume', onSocketResume); @@ -361,7 +371,7 @@ function connectionListener(socket) { } } parser[kOnExecute] = - onParserExecute.bind(undefined, this, socket, parser, state); + onParserExecute.bind(undefined, server, socket, parser, state); socket._paused = false; } diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index e0c7a44fc45742..06f6b06f093804 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -26,7 +26,7 @@ const async_wrap = process.binding('async_wrap'); * It has a fixed size, so if that is exceeded, calls to the native * side are used instead in pushAsyncIds() and popAsyncIds(). */ -const { async_hook_fields, async_id_fields } = async_wrap; +const { async_id_symbol, async_hook_fields, async_id_fields } = async_wrap; // Store the pair executionAsyncId and triggerAsyncId in a std::stack on // Environment::AsyncHooks::ids_stack_ tracks the resource responsible for the // current execution stack. This is unwound as each resource exits. In the case @@ -251,6 +251,15 @@ function newUid() { return ++async_id_fields[kAsyncIdCounter]; } +function getOrSetAsyncId(object) { + if (object.hasOwnProperty(async_id_symbol)) { + return object[async_id_symbol]; + } + + return object[async_id_symbol] = newUid(); +} + + // Return the triggerAsyncId meant for the constructor calling it. It's up to // the user to safeguard this call and make sure it's zero'd out when the // constructor is complete. @@ -381,6 +390,7 @@ module.exports = { disableHooks, // Sensitive Embedder API newUid, + getOrSetAsyncId, getDefaultTriggerAsyncId, defaultTriggerAsyncIdScope, emitInit: emitInitScript, diff --git a/test/async-hooks/test-graph.http.js b/test/async-hooks/test-graph.http.js new file mode 100644 index 00000000000000..5c0c99f408c824 --- /dev/null +++ b/test/async-hooks/test-graph.http.js @@ -0,0 +1,56 @@ +'use strict'; + +const common = require('../common'); +const initHooks = require('./init-hooks'); +const verifyGraph = require('./verify-graph'); +const http = require('http'); + +const hooks = initHooks(); +hooks.enable(); + +const server = http.createServer(common.mustCall(function(req, res) { + res.end(); + this.close(common.mustCall()); +})); +server.listen(0, common.mustCall(function() { + http.get(`http://127.0.0.1:${server.address().port}`, common.mustCall()); +})); + +process.on('exit', function() { + hooks.disable(); + + verifyGraph( + hooks, + [ { type: 'TCPSERVERWRAP', + id: 'tcpserver:1', + triggerAsyncId: null }, + { type: 'TCPWRAP', id: 'tcp:1', triggerAsyncId: 'tcpserver:1' }, + { type: 'TCPCONNECTWRAP', + id: 'tcpconnect:1', + triggerAsyncId: 'tcp:1' }, + { type: 'HTTPPARSER', + id: 'httpparser:1', + triggerAsyncId: 'tcpserver:1' }, + { type: 'HTTPPARSER', + id: 'httpparser:2', + triggerAsyncId: 'tcpserver:1' }, + { type: 'TCPWRAP', id: 'tcp:2', triggerAsyncId: 'tcpserver:1' }, + { type: 'Timeout', id: 'timeout:1', triggerAsyncId: 'tcp:2' }, + { type: 'TIMERWRAP', id: 'timer:1', triggerAsyncId: 'tcp:2' }, + { type: 'HTTPPARSER', + id: 'httpparser:3', + triggerAsyncId: 'tcp:2' }, + { type: 'HTTPPARSER', + id: 'httpparser:4', + triggerAsyncId: 'tcp:2' }, + { type: 'Timeout', + id: 'timeout:2', + triggerAsyncId: 'httpparser:4' }, + { type: 'TIMERWRAP', + id: 'timer:2', + triggerAsyncId: 'httpparser:4' }, + { type: 'SHUTDOWNWRAP', + id: 'shutdown:1', + triggerAsyncId: 'tcp:2' } ] + ); +}); From 8937440286ec16c4d184656e1ca123fdc3fefd98 Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Sun, 14 Jan 2018 11:09:24 +0100 Subject: [PATCH 22/22] async_hooks,test: only use IPv6 in http test If IPv6 is not supported on a machine, the IPv6 handle will first be created, this will then fail and default to an IPv4 handle. This causes the graph to change, as there now is an extra handle. PR-URL: https://github.com/nodejs/node/pull/18143 Fixes: https://github.com/nodejs/node/issues/18003 Reviewed-By: Anna Henningsen Reviewed-By: Rich Trott Reviewed-By: Anatoli Papirovski Reviewed-By: James M Snell --- test/async-hooks/test-graph.http.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/async-hooks/test-graph.http.js b/test/async-hooks/test-graph.http.js index 5c0c99f408c824..eea72ca3bac72c 100644 --- a/test/async-hooks/test-graph.http.js +++ b/test/async-hooks/test-graph.http.js @@ -1,6 +1,9 @@ 'use strict'; const common = require('../common'); +if (!common.hasIPv6) + common.skip('IPv6 support required'); + const initHooks = require('./init-hooks'); const verifyGraph = require('./verify-graph'); const http = require('http'); @@ -13,7 +16,11 @@ const server = http.createServer(common.mustCall(function(req, res) { this.close(common.mustCall()); })); server.listen(0, common.mustCall(function() { - http.get(`http://127.0.0.1:${server.address().port}`, common.mustCall()); + http.get({ + host: '::1', + family: 6, + port: server.address().port + }, common.mustCall()); })); process.on('exit', function() {