From 0ba8346f57b9b0fdf291d9395789a12c5aea375b Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Sun, 11 Feb 2018 16:35:59 -0500 Subject: [PATCH 1/3] async_hooks: clean up usage in internal code Instead of exposing internals of async_hooks & async_wrap throughout the code base, create necessary helper methods within the internal async_hooks that allows easy usage by Node.js internals. This stops every single internal user of async_hooks from importing a ton of functions, constants and internal Aliased Buffers from C++ async_wrap. Adds functions initHooksExist, afterHooksExist, and destroyHooksExist to determine whether the related emit methods need to be triggered. Adds clearDefaultTriggerAsyncId and clearAsyncIdStack on the JS side as an alternative to always calling C++. Moves async_id_symbol and trigger_async_id_symbol to internal async_hooks as they are never used in C++. Adjusts usage throughout the codebase, as well as in a couple of tests. --- lib/_http_agent.js | 2 +- lib/_http_outgoing.js | 2 +- lib/async_hooks.js | 11 ++--- lib/dgram.js | 6 ++- lib/internal/async_hooks.js | 47 ++++++++++++++++++- lib/internal/bootstrap_node.js | 26 +++++----- lib/internal/http2/core.js | 2 +- lib/internal/process/next_tick.js | 27 ++++++----- lib/internal/timers.js | 12 ++--- lib/net.js | 7 ++- lib/timers.js | 45 ++++++++---------- src/async_wrap.cc | 19 -------- src/async_wrap.h | 2 - test/parallel/test-async-hooks-http-agent.js | 3 +- .../test-http-client-immediate-error.js | 6 ++- 15 files changed, 118 insertions(+), 99 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 5f1e56caeab981..13abe9c210d55d 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -25,7 +25,7 @@ const net = require('net'); const util = require('util'); const EventEmitter = require('events'); const debug = util.debuglog('http'); -const { async_id_symbol } = process.binding('async_wrap'); +const { async_id_symbol } = require('internal/async_hooks').symbols; const { nextTick } = require('internal/process/next_tick'); // New Agent code. diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 425cfbcc2991b1..f513315b7cd421 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -31,7 +31,7 @@ const common = require('_http_common'); const checkIsHttpToken = common._checkIsHttpToken; const checkInvalidHeaderChar = common._checkInvalidHeaderChar; const { outHeadersKey } = require('internal/http'); -const { async_id_symbol } = process.binding('async_wrap'); +const { async_id_symbol } = require('internal/async_hooks').symbols; const { nextTick } = require('internal/process/next_tick'); const errors = require('internal/errors'); diff --git a/lib/async_hooks.js b/lib/async_hooks.js index cc9e893885a9ec..9ffd19cddd9ad1 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -9,6 +9,7 @@ const internal_async_hooks = require('internal/async_hooks'); // resource gets gced. const { registerDestroyHook } = async_wrap; const { + executionAsyncId, // Private API getHookArrays, enableHooks, @@ -27,16 +28,15 @@ const { async_id_fields } = async_wrap; // Get symbols const { + async_id_symbol, trigger_async_id_symbol, init_symbol, before_symbol, after_symbol, destroy_symbol, promise_resolve_symbol } = internal_async_hooks.symbols; -const { async_id_symbol, trigger_async_id_symbol } = async_wrap; - // Get constants const { kInit, kBefore, kAfter, kDestroy, kTotals, kPromiseResolve, - kExecutionAsyncId, kTriggerAsyncId + kTriggerAsyncId } = async_wrap.constants; // Listener API // @@ -125,11 +125,6 @@ function createHook(fns) { } -function executionAsyncId() { - return async_id_fields[kExecutionAsyncId]; -} - - function triggerAsyncId() { return async_id_fields[kTriggerAsyncId]; } diff --git a/lib/dgram.js b/lib/dgram.js index bed6129b47be11..a31384a586f4c1 100644 --- a/lib/dgram.js +++ b/lib/dgram.js @@ -28,9 +28,11 @@ const dns = require('dns'); const util = require('util'); const { isUint8Array } = require('internal/util/types'); const EventEmitter = require('events'); -const { defaultTriggerAsyncIdScope } = require('internal/async_hooks'); +const { + defaultTriggerAsyncIdScope, + symbols: { async_id_symbol } +} = 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'); const { UDP, SendWrap } = process.binding('udp_wrap'); diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index d1310ad2cac005..d28e25c6bf9073 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -27,7 +27,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_id_symbol, async_hook_fields, async_id_fields } = async_wrap; +const { async_hook_fields, async_id_fields } = async_wrap; // Store the pair executionAsyncId and triggerAsyncId in a std::stack on // Environment::AsyncHooks::async_ids_stack_ tracks the resource responsible for // the current execution stack. This is unwound as each resource exits. In the @@ -71,6 +71,8 @@ const { kInit, kBefore, kAfter, kDestroy, kPromiseResolve, kDefaultTriggerAsyncId, kStackLength } = async_wrap.constants; // Used in AsyncHook and AsyncResource. +const async_id_symbol = Symbol('asyncId'); +const trigger_async_id_symbol = Symbol('triggerAsyncId'); const init_symbol = Symbol('init'); const before_symbol = Symbol('before'); const after_symbol = Symbol('after'); @@ -270,6 +272,11 @@ function getDefaultTriggerAsyncId() { } +function clearDefaultTriggerAsyncId() { + async_id_fields[kDefaultTriggerAsyncId] = -1; +} + + function defaultTriggerAsyncIdScope(triggerAsyncId, block, ...args) { // CHECK(Number.isSafeInteger(triggerAsyncId)) // CHECK(triggerAsyncId > 0) @@ -287,6 +294,19 @@ function defaultTriggerAsyncIdScope(triggerAsyncId, block, ...args) { } +function initHooksExist() { + return async_hook_fields[kInit] > 0; +} + +function afterHooksExist() { + return async_hook_fields[kAfter] > 0; +} + +function destroyHooksExist() { + return async_hook_fields[kDestroy] > 0; +} + + function emitInitScript(asyncId, type, triggerAsyncId, resource) { validateAsyncId(asyncId, 'asyncId'); if (triggerAsyncId !== null) @@ -345,6 +365,18 @@ function emitDestroyScript(asyncId) { } +function clearAsyncIdStack() { + async_id_fields[kExecutionAsyncId] = 0; + async_id_fields[kTriggerAsyncId] = 0; + async_hook_fields[kStackLength] = 0; +} + + +function hasAsyncIdStack() { + return async_hook_fields[kStackLength] > 0; +} + + // This is the equivalent of the native push_async_ids() call. function pushAsyncIds(asyncId, triggerAsyncId) { const offset = async_hook_fields[kStackLength]; @@ -377,20 +409,33 @@ function popAsyncIds(asyncId) { } +function executionAsyncId() { + return async_id_fields[kExecutionAsyncId]; +} + + module.exports = { + executionAsyncId, // Private API getHookArrays, symbols: { + async_id_symbol, trigger_async_id_symbol, init_symbol, before_symbol, after_symbol, destroy_symbol, promise_resolve_symbol }, enableHooks, disableHooks, + clearDefaultTriggerAsyncId, + clearAsyncIdStack, + hasAsyncIdStack, // Internal Embedder API newUid, getOrSetAsyncId, getDefaultTriggerAsyncId, defaultTriggerAsyncIdScope, + initHooksExist, + afterHooksExist, + destroyHooksExist, emitInit: emitInitScript, emitBefore: emitBeforeScript, emitAfter: emitAfterScript, diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index 0c2109fab36cc0..a16d1935fbb6b5 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -424,18 +424,19 @@ function noop() {} function setupProcessFatal() { - const async_wrap = process.binding('async_wrap'); - // 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 } = async_wrap; - const { kAfter, kExecutionAsyncId, - kDefaultTriggerAsyncId, kStackLength } = async_wrap.constants; + const { + executionAsyncId, + clearDefaultTriggerAsyncId, + clearAsyncIdStack, + hasAsyncIdStack, + afterHooksExist, + emitAfter + } = NativeModule.require('internal/async_hooks'); process._fatalException = function(er) { - // It's possible that kDefaultTriggerAsyncId was set for a constructor + // It's possible that defaultTriggerAsyncId was set for a constructor // call that threw and was never cleared. So clear it now. - async_id_fields[kDefaultTriggerAsyncId] = -1; + clearDefaultTriggerAsyncId(); if (exceptionHandlerState.captureFn !== null) { exceptionHandlerState.captureFn(er); @@ -458,11 +459,10 @@ NativeModule.require('timers').setImmediate(noop); // Emit the after() hooks now that the exception has been handled. - if (async_hook_fields[kAfter] > 0) { - const { emitAfter } = NativeModule.require('internal/async_hooks'); + if (afterHooksExist()) { do { - emitAfter(async_id_fields[kExecutionAsyncId]); - } while (async_hook_fields[kStackLength] > 0); + emitAfter(executionAsyncId()); + } while (hasAsyncIdStack()); // Or completely empty the id stack. } else { clearAsyncIdStack(); diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index dc01fb14e6e263..b7a56850a308c8 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -4,7 +4,7 @@ require('internal/util').assertCrypto(); -const { async_id_symbol } = process.binding('async_wrap'); +const { async_id_symbol } = require('internal/async_hooks').symbols; const http = require('http'); const binding = process.binding('http2'); const assert = require('assert'); diff --git a/lib/internal/process/next_tick.js b/lib/internal/process/next_tick.js index 84a7402117c5c2..ad3d98c83913e5 100644 --- a/lib/internal/process/next_tick.js +++ b/lib/internal/process/next_tick.js @@ -5,19 +5,20 @@ exports.setup = setupNextTick; exports.nextTick = null; function setupNextTick() { - const async_wrap = process.binding('async_wrap'); - const async_hooks = require('internal/async_hooks'); + const { + getDefaultTriggerAsyncId, + newUid, + initHooksExist, + destroyHooksExist, + emitInit, + emitBefore, + emitAfter, + emitDestroy, + symbols: { async_id_symbol, trigger_async_id_symbol } + } = require('internal/async_hooks'); const promises = require('internal/process/promises'); const errors = require('internal/errors'); const { emitPromiseRejectionWarnings } = promises; - 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. - const { emitInit, emitBefore, emitAfter, emitDestroy } = async_hooks; - // Grab the constants necessary for working with internal arrays. - const { kInit, kDestroy, kAsyncIdCounter } = async_wrap.constants; - const { async_id_symbol, trigger_async_id_symbol } = async_wrap; // tickInfo is used so that the C++ code in src/node.cc can // have easy access to our nextTick state, and avoid unnecessary @@ -104,7 +105,7 @@ function setupNextTick() { // that nextTick() doesn't allow the event loop to proceed, but if // any async hooks are enabled during the callback's execution then // this tock's after hook will be called, but not its destroy hook. - if (async_hook_fields[kDestroy] > 0) + if (destroyHooksExist()) emitDestroy(asyncId); const callback = tock.callback; @@ -128,11 +129,11 @@ function setupNextTick() { this.callback = callback; this.args = args; - const asyncId = ++async_id_fields[kAsyncIdCounter]; + const asyncId = newUid(); this[async_id_symbol] = asyncId; this[trigger_async_id_symbol] = triggerAsyncId; - if (async_hook_fields[kInit] > 0) { + if (initHooksExist()) { emitInit(asyncId, 'TickObject', triggerAsyncId, diff --git a/lib/internal/timers.js b/lib/internal/timers.js index df2a88558fd6e3..32e686d997e45f 100644 --- a/lib/internal/timers.js +++ b/lib/internal/timers.js @@ -1,15 +1,11 @@ 'use strict'; -const async_wrap = process.binding('async_wrap'); -// Two arrays that share state between C++ and JS. -const { async_hook_fields, async_id_fields } = async_wrap; const { getDefaultTriggerAsyncId, - // The needed emit*() functions. + newUid, + initHooksExist, emitInit } = require('internal/async_hooks'); -// Grab the constants necessary for working with internal arrays. -const { kInit, kAsyncIdCounter } = async_wrap.constants; // Symbols for storing async id state. const async_id_symbol = Symbol('asyncId'); const trigger_async_id_symbol = Symbol('triggerId'); @@ -70,9 +66,9 @@ function Timeout(callback, after, args, isRepeat, isUnrefed) { this[unrefedSymbol] = isUnrefed; - this[async_id_symbol] = ++async_id_fields[kAsyncIdCounter]; + this[async_id_symbol] = newUid(); this[trigger_async_id_symbol] = getDefaultTriggerAsyncId(); - if (async_hook_fields[kInit] > 0) { + if (initHooksExist()) { emitInit(this[async_id_symbol], 'Timeout', this[trigger_async_id_symbol], diff --git a/lib/net.js b/lib/net.js index 1bc28e856df5bd..fc1903f3ac83a4 100644 --- a/lib/net.js +++ b/lib/net.js @@ -47,8 +47,11 @@ 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'); -const { async_id_symbol } = process.binding('async_wrap'); -const { newUid, defaultTriggerAsyncIdScope } = require('internal/async_hooks'); +const { + newUid, + defaultTriggerAsyncIdScope, + symbols: { async_id_symbol } +} = 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 08ddf16785685d..8ff30a08ada5d4 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -21,34 +21,34 @@ 'use strict'; -const async_wrap = process.binding('async_wrap'); const { Timer: TimerWrap, setupTimers, } = process.binding('timer_wrap'); const L = require('internal/linkedlist'); -const timerInternals = require('internal/timers'); +const { + async_id_symbol, + trigger_async_id_symbol, + Timeout, + validateTimerDuration +} = require('internal/timers'); const internalUtil = require('internal/util'); const { createPromise, promiseResolve } = process.binding('util'); const assert = require('assert'); const util = require('util'); const errors = require('internal/errors'); const debug = util.debuglog('timer'); -// Two arrays that share state between C++ and JS. -const { async_hook_fields, async_id_fields } = async_wrap; const { getDefaultTriggerAsyncId, + newUid, + initHooksExist, + destroyHooksExist, // The needed emit*() functions. emitInit, emitBefore, emitAfter, emitDestroy } = 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. -const async_id_symbol = timerInternals.async_id_symbol; -const trigger_async_id_symbol = timerInternals.trigger_async_id_symbol; // *Must* match Environment::ImmediateInfo::Fields in src/env.h. const kCount = 0; @@ -60,10 +60,6 @@ const [immediateInfo, toggleImmediateRef] = const kRefed = Symbol('refed'); -// The Timeout class -const Timeout = timerInternals.Timeout; - - // HOW and WHY the timers implementation works the way it does. // // Timers are crucial to Node.js. Internally, any TCP I/O connection creates a @@ -192,9 +188,9 @@ function insert(item, unrefed, start) { if (!item[async_id_symbol] || item._destroyed) { item._destroyed = false; - item[async_id_symbol] = ++async_id_fields[kAsyncIdCounter]; + item[async_id_symbol] = newUid(); item[trigger_async_id_symbol] = getDefaultTriggerAsyncId(); - if (async_hook_fields[kInit] > 0) { + if (initHooksExist()) { emitInit(item[async_id_symbol], 'Timeout', item[trigger_async_id_symbol], @@ -255,7 +251,7 @@ function listOnTimeout(handle, now) { assert(timer !== L.peek(list)); if (!timer._onTimeout) { - if (async_hook_fields[kDestroy] > 0 && !timer._destroyed && + if (destroyHooksExist() && !timer._destroyed && typeof timer[async_id_symbol] === 'number') { emitDestroy(timer[async_id_symbol]); timer._destroyed = true; @@ -306,7 +302,7 @@ function tryOnTimeout(timer, start) { if (timerAsyncId !== null) { if (!threw) emitAfter(timerAsyncId); - if (!timer._repeat && async_hook_fields[kDestroy] > 0 && + if (!timer._repeat && destroyHooksExist() && !timer._destroyed) { emitDestroy(timerAsyncId); timer._destroyed = true; @@ -341,8 +337,7 @@ function reuse(item) { // Remove a timer. Cancels the timeout and resets the relevant timer properties. function unenroll(item) { // Fewer checks may be possible, but these cover everything. - if (async_hook_fields[kDestroy] > 0 && - item && + if (destroyHooksExist() && typeof item[async_id_symbol] === 'number' && !item._destroyed) { emitDestroy(item[async_id_symbol]); @@ -368,7 +363,7 @@ exports.unenroll = util.deprecate(unenroll, // This function does not start the timer, see `active()`. // Using existing objects as timers slightly reduces object overhead. function enroll(item, msecs) { - item._idleTimeout = timerInternals.validateTimerDuration(msecs); + item._idleTimeout = validateTimerDuration(msecs); // if this item was already in a list somewhere // then we should unenroll it from that @@ -574,7 +569,7 @@ Timeout.prototype.ref = function() { Timeout.prototype.close = function() { this._onTimeout = null; if (this._handle) { - if (async_hook_fields[kDestroy] > 0 && + if (destroyHooksExist() && typeof this[async_id_symbol] === 'number' && !this._destroyed) { emitDestroy(this[async_id_symbol]); @@ -684,7 +679,7 @@ function tryOnImmediate(immediate, oldTail, count, refCount) { } finally { immediate._onImmediate = null; - if (async_hook_fields[kDestroy] > 0) { + if (destroyHooksExist()) { emitDestroy(immediate[async_id_symbol]); } @@ -725,9 +720,9 @@ const Immediate = class Immediate { this._destroyed = false; this[kRefed] = false; - this[async_id_symbol] = ++async_id_fields[kAsyncIdCounter]; + this[async_id_symbol] = newUid(); this[trigger_async_id_symbol] = getDefaultTriggerAsyncId(); - if (async_hook_fields[kInit] > 0) { + if (initHooksExist()) { emitInit(this[async_id_symbol], 'Immediate', this[trigger_async_id_symbol], @@ -807,7 +802,7 @@ exports.clearImmediate = function(immediate) { toggleImmediateRef(false); immediate[kRefed] = undefined; - if (async_hook_fields[kDestroy] > 0) { + if (destroyHooksExist()) { emitDestroy(immediate[async_id_symbol]); } diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 387f3012480c63..7fa5f0ade9ade4 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -47,7 +47,6 @@ using v8::PromiseHookType; using v8::PropertyCallbackInfo; using v8::RetainedObjectInfo; using v8::String; -using v8::Symbol; using v8::TryCatch; using v8::Undefined; using v8::Value; @@ -472,12 +471,6 @@ void AsyncWrap::PopAsyncIds(const FunctionCallbackInfo& args) { } -void AsyncWrap::ClearAsyncIdStack(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - env->async_hooks()->clear_async_id_stack(); -} - - void AsyncWrap::AsyncReset(const FunctionCallbackInfo& args) { AsyncWrap* wrap; ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder()); @@ -512,7 +505,6 @@ void AsyncWrap::Initialize(Local target, env->SetMethod(target, "setupHooks", SetupHooks); env->SetMethod(target, "pushAsyncIds", PushAsyncIds); env->SetMethod(target, "popAsyncIds", PopAsyncIds); - env->SetMethod(target, "clearAsyncIdStack", ClearAsyncIdStack); env->SetMethod(target, "queueDestroyAsyncId", QueueDestroyAsyncId); env->SetMethod(target, "enablePromiseHook", EnablePromiseHook); env->SetMethod(target, "disablePromiseHook", DisablePromiseHook); @@ -581,17 +573,6 @@ void AsyncWrap::Initialize(Local target, #undef V FORCE_SET_TARGET_FIELD(target, "Providers", async_providers); - // These Symbols are used throughout node so the stored values on each object - // can be accessed easily across files. - FORCE_SET_TARGET_FIELD( - target, - "async_id_symbol", - Symbol::New(isolate, FIXED_ONE_BYTE_STRING(isolate, "asyncId"))); - FORCE_SET_TARGET_FIELD( - target, - "trigger_async_id_symbol", - Symbol::New(isolate, FIXED_ONE_BYTE_STRING(isolate, "triggerAsyncId"))); - #undef FORCE_SET_TARGET_FIELD env->set_async_hooks_init_function(Local()); diff --git a/src/async_wrap.h b/src/async_wrap.h index 1a5a347ba6519b..b7aed5d789754b 100644 --- a/src/async_wrap.h +++ b/src/async_wrap.h @@ -125,8 +125,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 ClearAsyncIdStack( - const v8::FunctionCallbackInfo& args); static void AsyncReset(const v8::FunctionCallbackInfo& args); static void QueueDestroyAsyncId( const v8::FunctionCallbackInfo& args); diff --git a/test/parallel/test-async-hooks-http-agent.js b/test/parallel/test-async-hooks-http-agent.js index e10820c3c202b6..a55858947337b2 100644 --- a/test/parallel/test-async-hooks-http-agent.js +++ b/test/parallel/test-async-hooks-http-agent.js @@ -1,7 +1,8 @@ 'use strict'; +// Flags: --expose-internals const common = require('../common'); const assert = require('assert'); -const async_id_symbol = process.binding('async_wrap').async_id_symbol; +const { async_id_symbol } = require('internal/async_hooks').symbols; const http = require('http'); // Regression test for https://github.com/nodejs/node/issues/13325 diff --git a/test/parallel/test-http-client-immediate-error.js b/test/parallel/test-http-client-immediate-error.js index abbf5c41fc6660..749c2579c6cf80 100644 --- a/test/parallel/test-http-client-immediate-error.js +++ b/test/parallel/test-http-client-immediate-error.js @@ -9,8 +9,10 @@ const assert = require('assert'); const net = require('net'); const http = require('http'); const uv = process.binding('uv'); -const { async_id_symbol } = process.binding('async_wrap'); -const { newUid } = require('internal/async_hooks'); +const { + newUid, + symbols: { async_id_symbol } +} = require('internal/async_hooks'); const agent = new http.Agent(); agent.createConnection = common.mustCall((cfg) => { From 029cd572d41ac5542ff6693be3c454ad1ac129ea Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Thu, 15 Feb 2018 16:53:15 -0500 Subject: [PATCH 2/3] fixup: ofrobots feedback --- lib/async_hooks.js | 10 +--------- lib/internal/async_hooks.js | 7 +++++++ src/env-inl.h | 1 + 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/async_hooks.js b/lib/async_hooks.js index 9ffd19cddd9ad1..d4c3be3d75d129 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -10,6 +10,7 @@ const internal_async_hooks = require('internal/async_hooks'); const { registerDestroyHook } = async_wrap; const { executionAsyncId, + triggerAsyncId, // Private API getHookArrays, enableHooks, @@ -23,9 +24,6 @@ const { emitDestroy, } = internal_async_hooks; -// Get fields -const { async_id_fields } = async_wrap; - // Get symbols const { async_id_symbol, trigger_async_id_symbol, @@ -36,7 +34,6 @@ const { // Get constants const { kInit, kBefore, kAfter, kDestroy, kTotals, kPromiseResolve, - kTriggerAsyncId } = async_wrap.constants; // Listener API // @@ -125,11 +122,6 @@ function createHook(fns) { } -function triggerAsyncId() { - return async_id_fields[kTriggerAsyncId]; -} - - // Embedder API // const destroyedSymbol = Symbol('destroyed'); diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index d28e25c6bf9073..e633692a2e0faf 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -365,6 +365,8 @@ function emitDestroyScript(asyncId) { } +// Keep in sync with Environment::AsyncHooks::clear_async_id_stack +// in src/env-inl.h. function clearAsyncIdStack() { async_id_fields[kExecutionAsyncId] = 0; async_id_fields[kTriggerAsyncId] = 0; @@ -413,9 +415,14 @@ function executionAsyncId() { return async_id_fields[kExecutionAsyncId]; } +function triggerAsyncId() { + return async_id_fields[kTriggerAsyncId]; +} + module.exports = { executionAsyncId, + triggerAsyncId, // Private API getHookArrays, symbols: { diff --git a/src/env-inl.h b/src/env-inl.h index 5643fffb6f8b6a..20e81dd5614b7c 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -172,6 +172,7 @@ inline bool Environment::AsyncHooks::pop_async_id(double async_id) { return fields_[kStackLength] > 0; } +// Keep in sync with clearAsyncIdStack in lib/internal/async_hooks.js. inline void Environment::AsyncHooks::clear_async_id_stack() { async_id_fields_[kExecutionAsyncId] = 0; async_id_fields_[kTriggerAsyncId] = 0; From 2abd4a7142f76000147aaf2c98d0e9ba71660722 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Fri, 16 Feb 2018 13:09:04 -0500 Subject: [PATCH 3/3] fixup: rename newUid to newAsyncId --- lib/async_hooks.js | 4 ++-- lib/internal/async_hooks.js | 6 +++--- lib/internal/process/next_tick.js | 4 ++-- lib/internal/timers.js | 4 ++-- lib/net.js | 4 ++-- lib/timers.js | 6 +++--- test/async-hooks/test-emit-before-after.js | 4 ++-- test/async-hooks/test-emit-init.js | 4 ++-- test/parallel/test-http-client-immediate-error.js | 4 ++-- test/parallel/test-process-geteuid-getegid.js | 4 ++-- test/parallel/test-process-setuid-setgid.js | 4 ++-- 11 files changed, 24 insertions(+), 24 deletions(-) diff --git a/lib/async_hooks.js b/lib/async_hooks.js index d4c3be3d75d129..babb705aad6851 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -16,7 +16,7 @@ const { enableHooks, disableHooks, // Internal Embedder API - newUid, + newAsyncId, getDefaultTriggerAsyncId, emitInit, emitBefore, @@ -157,7 +157,7 @@ class AsyncResource { triggerAsyncId); } - this[async_id_symbol] = newUid(); + this[async_id_symbol] = newAsyncId(); this[trigger_async_id_symbol] = triggerAsyncId; // this prop name (destroyed) has to be synchronized with C++ this[destroyedSymbol] = { destroyed: false }; diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index e633692a2e0faf..dc720a223052bf 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -247,7 +247,7 @@ function disableHooks() { // 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() { +function newAsyncId() { return ++async_id_fields[kAsyncIdCounter]; } @@ -256,7 +256,7 @@ function getOrSetAsyncId(object) { return object[async_id_symbol]; } - return object[async_id_symbol] = newUid(); + return object[async_id_symbol] = newAsyncId(); } @@ -436,7 +436,7 @@ module.exports = { clearAsyncIdStack, hasAsyncIdStack, // Internal Embedder API - newUid, + newAsyncId, getOrSetAsyncId, getDefaultTriggerAsyncId, defaultTriggerAsyncIdScope, diff --git a/lib/internal/process/next_tick.js b/lib/internal/process/next_tick.js index ad3d98c83913e5..f95d9cc1b82ebf 100644 --- a/lib/internal/process/next_tick.js +++ b/lib/internal/process/next_tick.js @@ -7,7 +7,7 @@ exports.nextTick = null; function setupNextTick() { const { getDefaultTriggerAsyncId, - newUid, + newAsyncId, initHooksExist, destroyHooksExist, emitInit, @@ -129,7 +129,7 @@ function setupNextTick() { this.callback = callback; this.args = args; - const asyncId = newUid(); + const asyncId = newAsyncId(); this[async_id_symbol] = asyncId; this[trigger_async_id_symbol] = triggerAsyncId; diff --git a/lib/internal/timers.js b/lib/internal/timers.js index 32e686d997e45f..0c140811f8ce88 100644 --- a/lib/internal/timers.js +++ b/lib/internal/timers.js @@ -2,7 +2,7 @@ const { getDefaultTriggerAsyncId, - newUid, + newAsyncId, initHooksExist, emitInit } = require('internal/async_hooks'); @@ -66,7 +66,7 @@ function Timeout(callback, after, args, isRepeat, isUnrefed) { this[unrefedSymbol] = isUnrefed; - this[async_id_symbol] = newUid(); + this[async_id_symbol] = newAsyncId(); this[trigger_async_id_symbol] = getDefaultTriggerAsyncId(); if (initHooksExist()) { emitInit(this[async_id_symbol], diff --git a/lib/net.js b/lib/net.js index fc1903f3ac83a4..5966119275dc81 100644 --- a/lib/net.js +++ b/lib/net.js @@ -48,7 +48,7 @@ const { TCPConnectWrap } = process.binding('tcp_wrap'); const { PipeConnectWrap } = process.binding('pipe_wrap'); const { ShutdownWrap, WriteWrap } = process.binding('stream_wrap'); const { - newUid, + newAsyncId, defaultTriggerAsyncIdScope, symbols: { async_id_symbol } } = require('internal/async_hooks'); @@ -94,7 +94,7 @@ function createHandle(fd, is_server) { function getNewAsyncId(handle) { return (!handle || typeof handle.getAsyncId !== 'function') ? - newUid() : handle.getAsyncId(); + newAsyncId() : handle.getAsyncId(); } diff --git a/lib/timers.js b/lib/timers.js index 8ff30a08ada5d4..145550b7b5666b 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -40,7 +40,7 @@ const errors = require('internal/errors'); const debug = util.debuglog('timer'); const { getDefaultTriggerAsyncId, - newUid, + newAsyncId, initHooksExist, destroyHooksExist, // The needed emit*() functions. @@ -188,7 +188,7 @@ function insert(item, unrefed, start) { if (!item[async_id_symbol] || item._destroyed) { item._destroyed = false; - item[async_id_symbol] = newUid(); + item[async_id_symbol] = newAsyncId(); item[trigger_async_id_symbol] = getDefaultTriggerAsyncId(); if (initHooksExist()) { emitInit(item[async_id_symbol], @@ -720,7 +720,7 @@ const Immediate = class Immediate { this._destroyed = false; this[kRefed] = false; - this[async_id_symbol] = newUid(); + this[async_id_symbol] = newAsyncId(); this[trigger_async_id_symbol] = getDefaultTriggerAsyncId(); if (initHooksExist()) { emitInit(this[async_id_symbol], diff --git a/test/async-hooks/test-emit-before-after.js b/test/async-hooks/test-emit-before-after.js index 1b28c1e42622dd..e7744eb4b7bdf0 100644 --- a/test/async-hooks/test-emit-before-after.js +++ b/test/async-hooks/test-emit-before-after.js @@ -34,8 +34,8 @@ assert.strictEqual( 'RangeError [ERR_INVALID_ASYNC_ID]: Invalid triggerAsyncId value: -2'); assert.strictEqual(c2.status, 1); -const expectedId = async_hooks.newUid(); -const expectedTriggerId = async_hooks.newUid(); +const expectedId = async_hooks.newAsyncId(); +const expectedTriggerId = async_hooks.newAsyncId(); const expectedType = 'test_emit_before_after_type'; // Verify that if there is no registered hook, then nothing will happen. diff --git a/test/async-hooks/test-emit-init.js b/test/async-hooks/test-emit-init.js index e69285d4f81c84..f5d5687cb4d11d 100644 --- a/test/async-hooks/test-emit-init.js +++ b/test/async-hooks/test-emit-init.js @@ -7,8 +7,8 @@ const spawnSync = require('child_process').spawnSync; const async_hooks = require('internal/async_hooks'); const initHooks = require('./init-hooks'); -const expectedId = async_hooks.newUid(); -const expectedTriggerId = async_hooks.newUid(); +const expectedId = async_hooks.newAsyncId(); +const expectedTriggerId = async_hooks.newAsyncId(); const expectedType = 'test_emit_init_type'; const expectedResource = { key: 'test_emit_init_resource' }; diff --git a/test/parallel/test-http-client-immediate-error.js b/test/parallel/test-http-client-immediate-error.js index 749c2579c6cf80..1c65d07ca50bfe 100644 --- a/test/parallel/test-http-client-immediate-error.js +++ b/test/parallel/test-http-client-immediate-error.js @@ -10,7 +10,7 @@ const net = require('net'); const http = require('http'); const uv = process.binding('uv'); const { - newUid, + newAsyncId, symbols: { async_id_symbol } } = require('internal/async_hooks'); @@ -28,7 +28,7 @@ agent.createConnection = common.mustCall((cfg) => { }; // Simulate just enough socket handle initialization - sock[async_id_symbol] = newUid(); + sock[async_id_symbol] = newAsyncId(); sock.connect(cfg); return sock; diff --git a/test/parallel/test-process-geteuid-getegid.js b/test/parallel/test-process-geteuid-getegid.js index 8dab52389b0384..a76382682214ef 100644 --- a/test/parallel/test-process-geteuid-getegid.js +++ b/test/parallel/test-process-geteuid-getegid.js @@ -45,5 +45,5 @@ assert.notStrictEqual(newgid, oldgid); const olduid = process.geteuid(); process.seteuid('nobody'); -const newuid = process.geteuid(); -assert.notStrictEqual(newuid, olduid); +const newAsyncId = process.geteuid(); +assert.notStrictEqual(newAsyncId, olduid); diff --git a/test/parallel/test-process-setuid-setgid.js b/test/parallel/test-process-setuid-setgid.js index e0db8ee00222dd..7a041acdee47fc 100644 --- a/test/parallel/test-process-setuid-setgid.js +++ b/test/parallel/test-process-setuid-setgid.js @@ -64,5 +64,5 @@ assert.notStrictEqual(newgid, oldgid); const olduid = process.getuid(); process.setuid('nobody'); -const newuid = process.getuid(); -assert.notStrictEqual(newuid, olduid); +const newAsyncId = process.getuid(); +assert.notStrictEqual(newAsyncId, olduid);