From d4ae984bd106a62a9c40195fea081f2d9c394243 Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Wed, 22 Nov 2017 13:54:38 +0100 Subject: [PATCH 1/7] async_hooks: rename initTriggerId rename initTriggerId to defaultTriggerAsyncId such it matches the rest of our naming. --- lib/_http_client.js | 2 +- lib/async_hooks.js | 4 ++-- 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, 69 insertions(+), 67 deletions(-) diff --git a/lib/_http_client.js b/lib/_http_client.js index d9a2d10ae24fd6..e2aaf86a5ee1ae 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -547,7 +547,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 cf3a40c1274a14..55a16a569e908b 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -15,7 +15,7 @@ const { disableHooks, // Internal Embedder API newUid, - initTriggerId, + getDefaultTriggerAsyncId, emitInit, emitBefore, emitAfter, @@ -147,7 +147,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 diff --git a/lib/dgram.js b/lib/dgram.js index ec60765aa48cf3..679368ec181496 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'); @@ -481,7 +481,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 002cbccb03218b..7e29d53606a2b3 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, // Internal 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 3f43c23e8d7c0b..b7da5bd3801f5a 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -369,14 +369,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 (exceptionHandlerState.captureFn !== null) { exceptionHandlerState.captureFn(er); diff --git a/lib/internal/process/next_tick.js b/lib/internal/process/next_tick.js index 260aa70b431b86..bf7d0bc94dc4ce 100644 --- a/lib/internal/process/next_tick.js +++ b/lib/internal/process/next_tick.js @@ -48,7 +48,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. @@ -210,7 +210,7 @@ function setupNextTick() { nextTickQueue.push(new TickObject(callback, args, ++async_id_fields[kAsyncIdCounter], - initTriggerId())); + getDefaultTriggerAsyncId())); } // `internalNextTick()` will not enqueue any callback when the process is @@ -237,7 +237,7 @@ function setupNextTick() { } if (triggerAsyncId === null) - triggerAsyncId = initTriggerId(); + triggerAsyncId = getDefaultTriggerAsyncId(); // In V8 6.2, moving tickInfo & async_id_fields[kAsyncIdCounter] into the // TickObject incurs a significant performance penalty in the // next-tick-breadth-args benchmark (revisit later) diff --git a/lib/net.js b/lib/net.js index 245415d87c63a1..33c41d108532d5 100644 --- a/lib/net.js +++ b/lib/net.js @@ -43,7 +43,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'); @@ -304,7 +304,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) @@ -948,7 +948,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 @@ -961,7 +961,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); } @@ -1097,7 +1097,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 c9202cc0908bb9..479463d2d0ad4c 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -34,7 +34,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, @@ -181,7 +181,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', @@ -560,7 +560,7 @@ function Timeout(callback, after, args, isRepeat) { 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', @@ -786,7 +786,7 @@ function Immediate(callback, args) { 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], 'Immediate', diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 3f77169321a919..555d616426f6e7 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -310,7 +310,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); @@ -542,9 +542,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()); @@ -564,7 +565,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); @@ -677,7 +678,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) \ @@ -778,7 +779,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 f9923936fe46d7..a692e8e08e452d 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -447,17 +447,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 2ef4a25f5802bf..7d7986bce43464 100644 --- a/src/env.h +++ b/src/env.h @@ -381,7 +381,7 @@ class Environment { kExecutionAsyncId, kTriggerAsyncId, kAsyncIdCounter, - kInitTriggerAsyncId, + kDefaultTriggerAsyncId, kUidFieldsCount, }; @@ -558,8 +558,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 a48e77063ef451..f4bfb72f3f77a7 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); @@ -155,7 +155,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, storage_size); offset = 0; @@ -245,7 +245,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); @@ -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, storage_size); data = req_wrap->Extra(); diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index aa0cb7ed1789b5..233e3a1a15fc9e 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -292,7 +292,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(), @@ -328,7 +328,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 9d4e7f7ab41678..55e12e4278bcbe 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -357,7 +357,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 c2cd04b7f104c1ce6561cff2bdf14e8e0f1b5ad1 Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Wed, 22 Nov 2017 15:41:18 +0100 Subject: [PATCH 2/7] 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. --- 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 7e29d53606a2b3..2cd4137fd08b40 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 b7da5bd3801f5a..f8a1fa39618610 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -376,7 +376,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 (exceptionHandlerState.captureFn !== null) { exceptionHandlerState.captureFn(er); diff --git a/src/env-inl.h b/src/env-inl.h index a692e8e08e452d..2d4c0c18e96b5d 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -66,6 +66,12 @@ inline Environment::AsyncHooks::AsyncHooks(v8::Isolate* isolate) // and flag changes won't be included. fields_[kCheck] = 1; + // 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; @@ -450,9 +456,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 8a50186d4bcd9784401051c650785e749a5ac851 Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Wed, 22 Nov 2017 18:41:00 +0100 Subject: [PATCH 3/7] 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. --- lib/dgram.js | 20 ++++++---- lib/internal/async_hooks.js | 21 ++++++----- lib/net.js | 73 +++++++++++++++++++------------------ 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 | 38 +++++++++++-------- src/tcp_wrap.cc | 9 +++-- src/udp_wrap.cc | 12 ++++-- 12 files changed, 135 insertions(+), 109 deletions(-) diff --git a/lib/dgram.js b/lib/dgram.js index 679368ec181496..2c5facef9f3162 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'); @@ -481,13 +481,17 @@ 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. - setDefaultTriggerAsyncId(self[async_id_symbol]); - var err = self._handle.send(req, - list, - list.length, - port, - ip, - !!callback); + var err = defaultTriggerAsyncIdScope( + self[async_id_symbol], function() { + return 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); diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 2cd4137fd08b40..5a6a20ae5b76ee 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, block) { // CHECK(Number.isSafeInteger(triggerAsyncId)) // CHECK(triggerAsyncId > 0) + const oldDefaultTriggerAsyncId = async_id_fields[kDefaultTriggerAsyncId]; async_id_fields[kDefaultTriggerAsyncId] = triggerAsyncId; + + var ret; + try { + ret = block(); + } 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 = { // Internal Embedder API newUid, getDefaultTriggerAsyncId, - setDefaultTriggerAsyncId, + defaultTriggerAsyncIdScope, emitInit: emitInitScript, emitBefore: emitBeforeScript, emitAfter: emitAfterScript, diff --git a/lib/net.js b/lib/net.js index 33c41d108532d5..d4921bc8dc3e83 100644 --- a/lib/net.js +++ b/lib/net.js @@ -43,7 +43,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'); @@ -304,8 +304,8 @@ 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. - setDefaultTriggerAsyncId(this[async_id_symbol]); - var err = this._handle.shutdown(req); + var err = defaultTriggerAsyncIdScope(this[async_id_symbol], + () => this._handle.shutdown(req)); if (err) return this.destroy(errnoException(err, 'shutdown')); @@ -948,11 +948,12 @@ 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. - setDefaultTriggerAsyncId(self[async_id_symbol]); - if (addressType === 4) - err = self._handle.connect(req, address, port); - else - err = self._handle.connect6(req, address, port); + defaultTriggerAsyncIdScope(self[async_id_symbol], function() { + if (addressType === 4) + err = self._handle.connect(req, address, port); + else + err = self._handle.connect6(req, address, port); + }); } else { const req = new PipeConnectWrap(); @@ -961,8 +962,9 @@ 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. - setDefaultTriggerAsyncId(self[async_id_symbol]); - err = self._handle.connect(req, address, afterConnect); + err = defaultTriggerAsyncIdScope( + self[async_id_symbol], + () => self._handle.connect(req, address, afterConnect)); } if (err) { @@ -1097,33 +1099,34 @@ 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(); + internalConnect(self, + ip, + port, + addressType, + localAddress, + localPort); + } + }); }); } diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 555d616426f6e7..79d54b1000d342 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -308,12 +308,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 2d4c0c18e96b5d..145163172851a0 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -176,23 +176,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_++; @@ -456,17 +460,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 7d7986bce43464..814622994329f0 100644 --- a/src/env.h +++ b/src/env.h @@ -402,22 +402,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); @@ -559,7 +560,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 c6dce756cea829..c5958a2271a83e 100644 --- a/src/pipe_wrap.cc +++ b/src/pipe_wrap.cc @@ -53,7 +53,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 cc89a11bac249c..cdcff67cc55e66 100644 --- a/src/stream_base-inl.h +++ b/src/stream_base-inl.h @@ -143,7 +143,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 f4bfb72f3f77a7..b1aea79d52f762 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); @@ -109,7 +109,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; @@ -153,10 +152,13 @@ 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, 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, storage_size); + } offset = 0; if (!all_buffers) { @@ -227,7 +229,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); @@ -243,11 +244,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); + { + 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); + } err = DoWrite(req_wrap, bufs, count, nullptr); if (HasWriteQueue()) @@ -278,7 +282,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 +332,13 @@ 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, 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, storage_size); + } data = req_wrap->Extra(); diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index 233e3a1a15fc9e..3a0a3f295e2c72 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); @@ -292,7 +293,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(), @@ -328,7 +330,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 55e12e4278bcbe..27be93abd0e5d8 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -357,8 +357,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); @@ -507,7 +511,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 2b3db6e6a818ef404352d81b8c6f933401823ebd Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Thu, 7 Dec 2017 12:43:12 +0100 Subject: [PATCH 4/7] [squash] support opaque parameter --- lib/dgram.js | 23 +++++++------ lib/internal/async_hooks.js | 4 +-- lib/net.js | 64 +++++++++++++++++-------------------- 3 files changed, 43 insertions(+), 48 deletions(-) diff --git a/lib/dgram.js b/lib/dgram.js index 2c5facef9f3162..adcd1440059a4b 100644 --- a/lib/dgram.js +++ b/lib/dgram.js @@ -470,19 +470,18 @@ function doSend(ex, self, ip, list, address, port, callback) { return; } - var req = new SendWrap(); - req.list = list; // Keep reference alive. - req.address = address; - req.port = port; - if (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. var err = defaultTriggerAsyncIdScope( - self[async_id_symbol], function() { + self[async_id_symbol], [list, port, ip, callback], + function([list, port, ip, callback]) { + var req = new SendWrap(); + req.list = list; // Keep reference alive. + req.address = address; + req.port = port; + if (callback) { + req.callback = callback; + req.oncomplete = afterSend; + } + return self._handle.send(req, list, list.length, diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 5a6a20ae5b76ee..ac66a315050e78 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -257,7 +257,7 @@ function getDefaultTriggerAsyncId() { } -function defaultTriggerAsyncIdScope(triggerAsyncId, block) { +function defaultTriggerAsyncIdScope(triggerAsyncId, opaque, block) { // CHECK(Number.isSafeInteger(triggerAsyncId)) // CHECK(triggerAsyncId > 0) const oldDefaultTriggerAsyncId = async_id_fields[kDefaultTriggerAsyncId]; @@ -265,7 +265,7 @@ function defaultTriggerAsyncIdScope(triggerAsyncId, block) { var ret; try { - ret = block(); + ret = block(opaque); } finally { async_id_fields[kDefaultTriggerAsyncId] = oldDefaultTriggerAsyncId; } diff --git a/lib/net.js b/lib/net.js index d4921bc8dc3e83..cb1ea3df00aba7 100644 --- a/lib/net.js +++ b/lib/net.js @@ -298,14 +298,15 @@ 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. - var err = defaultTriggerAsyncIdScope(this[async_id_symbol], - () => this._handle.shutdown(req)); + var err = defaultTriggerAsyncIdScope( + this[async_id_symbol], [this, afterShutdown], + function([self, afterShutdown]) { + var req = new ShutdownWrap(); + req.oncomplete = afterShutdown; + req.handle = self._handle; + return self._handle.shutdown(req); + } + ); if (err) return this.destroy(errnoException(err, 'shutdown')); @@ -907,7 +908,7 @@ function checkBindError(err, port, handle) { function internalConnect( - self, address, port, addressType, localAddress, localPort) { + [self, address, port, addressType, localAddress, localPort]) { // TODO return promise from Socket.prototype.connect which // wraps _connectReq. @@ -945,26 +946,16 @@ 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. - defaultTriggerAsyncIdScope(self[async_id_symbol], function() { - if (addressType === 4) - err = self._handle.connect(req, address, port); - else - err = self._handle.connect6(req, address, port); - }); - + 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. - err = defaultTriggerAsyncIdScope( - self[async_id_symbol], - () => self._handle.connect(req, address, afterConnect)); + + err = self._handle.connect(req, address, afterConnect); } if (err) { @@ -1032,7 +1023,9 @@ Socket.prototype.connect = function(...args) { 'string', path); } - internalConnect(this, path); + defaultTriggerAsyncIdScope( + this[async_id_symbol], [this, path], internalConnect + ); } else { lookupAndConnect(this, options); } @@ -1075,7 +1068,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; } @@ -1099,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], null, function() { lookup(host, dnsopts, function emitLookup(err, ip, addressType) { self.emit('lookup', err, ip, addressType, host); @@ -1119,12 +1116,11 @@ function lookupAndConnect(self, options) { process.nextTick(connectErrorNT, self, err); } else { self._unrefTimer(); - internalConnect(self, - ip, - port, - addressType, - localAddress, - localPort); + defaultTriggerAsyncIdScope( + self[async_id_symbol], + [self, ip, port, addressType, localAddress, localPort], + internalConnect + ); } }); }); From 019b3657e9e636acae954057aee9131a5e7fb36c Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Thu, 7 Dec 2017 13:40:24 +0100 Subject: [PATCH 5/7] [squash] move shutdown to outer function --- lib/net.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/net.js b/lib/net.js index cb1ea3df00aba7..37603b93b77ffb 100644 --- a/lib/net.js +++ b/lib/net.js @@ -277,6 +277,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() { @@ -299,13 +307,7 @@ function onSocketFinish() { return this.destroy(); var err = defaultTriggerAsyncIdScope( - this[async_id_symbol], [this, afterShutdown], - function([self, afterShutdown]) { - var req = new ShutdownWrap(); - req.oncomplete = afterShutdown; - req.handle = self._handle; - return self._handle.shutdown(req); - } + this[async_id_symbol], [this, afterShutdown], shutdownSocket ); if (err) From e67a43d96159ce052250e8ca743edb3c531e50b9 Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Thu, 7 Dec 2017 14:01:48 +0100 Subject: [PATCH 6/7] [squash] avoid closure in dgram --- lib/dgram.js | 48 +++++++++++++++++++++++------------------------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/lib/dgram.js b/lib/dgram.js index adcd1440059a4b..16a43a3505b9ce 100644 --- a/lib/dgram.js +++ b/lib/dgram.js @@ -450,51 +450,49 @@ 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) { +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; } - var err = defaultTriggerAsyncIdScope( - self[async_id_symbol], [list, port, ip, callback], - function([list, port, ip, callback]) { - var req = new SendWrap(); - req.list = list; // Keep reference alive. - req.address = address; - req.port = port; - if (callback) { - req.callback = callback; - req.oncomplete = afterSend; - } + var req = new SendWrap(); + req.list = list; // Keep reference alive. + req.address = address; + req.port = port; + if (callback) { + req.callback = callback; + req.oncomplete = afterSend; + } - return self._handle.send(req, - list, - list.length, - port, - ip, - !!callback); - } - ); + 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); } } From cc459696f7943b726a1e4c2b4670dee39483d959 Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Thu, 7 Dec 2017 14:13:19 +0100 Subject: [PATCH 7/7] [squash] use apply instead of destructing --- lib/dgram.js | 2 +- lib/internal/async_hooks.js | 2 +- lib/net.js | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/dgram.js b/lib/dgram.js index 16a43a3505b9ce..d2cec9dd4e1cf4 100644 --- a/lib/dgram.js +++ b/lib/dgram.js @@ -460,7 +460,7 @@ Socket.prototype.send = function(buffer, this._handle.lookup(address, afterDns); }; -function doSend([ex, self, ip, list, address, port, callback]) { +function doSend(ex, self, ip, list, address, port, callback) { if (ex) { if (typeof callback === 'function') { process.nextTick(callback, ex); diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index ac66a315050e78..4bd5f3191b6e4a 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -265,7 +265,7 @@ function defaultTriggerAsyncIdScope(triggerAsyncId, opaque, block) { var ret; try { - ret = block(opaque); + ret = Reflect.apply(block, null, opaque); } finally { async_id_fields[kDefaultTriggerAsyncId] = oldDefaultTriggerAsyncId; } diff --git a/lib/net.js b/lib/net.js index 37603b93b77ffb..ce87a60ce0e78b 100644 --- a/lib/net.js +++ b/lib/net.js @@ -278,7 +278,7 @@ Socket.prototype._unrefTimer = function _unrefTimer() { }; -function shutdownSocket([self, callback]) { +function shutdownSocket(self, callback) { var req = new ShutdownWrap(); req.oncomplete = callback; req.handle = self._handle; @@ -910,7 +910,7 @@ function checkBindError(err, port, handle) { function internalConnect( - [self, address, port, addressType, localAddress, localPort]) { + self, address, port, addressType, localAddress, localPort) { // TODO return promise from Socket.prototype.connect which // wraps _connectReq. @@ -1098,7 +1098,7 @@ function lookupAndConnect(self, options) { debug('connect: dns options', dnsopts); self._host = host; var lookup = options.lookup || dns.lookup; - defaultTriggerAsyncIdScope(self[async_id_symbol], null, function() { + defaultTriggerAsyncIdScope(self[async_id_symbol], [], function() { lookup(host, dnsopts, function emitLookup(err, ip, addressType) { self.emit('lookup', err, ip, addressType, host);