From 1baee72998cbd2e76e0ef7fc76379c0a7b9c6809 Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Tue, 26 Sep 2017 15:50:10 +0200 Subject: [PATCH 1/3] async_hooks: consistent internal naming PR-URL: https://github.com/ayojs/ayo/pull/87 Reviewed-By: Timothy Gu Original-PR-URL: https://github.com/nodejs/node/pull/15569 Original-Reviewed-By: Refael Ackermann Original-Reviewed-By: Anna Henningsen Original-Reviewed-By: Ruben Bridgewater Original-Reviewed-By: James M Snell --- lib/async_hooks.js | 66 +++++----- lib/internal/bootstrap_node.js | 17 +-- lib/internal/process/next_tick.js | 26 ++-- lib/timers.js | 34 +++--- src/async-wrap-inl.h | 6 +- src/async-wrap.cc | 134 +++++++++++---------- src/async-wrap.h | 14 ++- src/connection_wrap.cc | 2 +- src/env-inl.h | 114 +++++++++--------- src/env.cc | 4 +- src/env.h | 53 ++++---- src/node.cc | 16 +-- src/node.h | 2 +- src/node_http_parser.cc | 2 +- src/pipe_wrap.cc | 2 +- src/stream_base-inl.h | 2 +- src/stream_base.cc | 8 +- src/tcp_wrap.cc | 6 +- src/udp_wrap.cc | 4 +- test/common/index.js | 6 +- test/parallel/test-async-wrap-destroyid.js | 2 +- 21 files changed, 270 insertions(+), 250 deletions(-) diff --git a/lib/async_hooks.js b/lib/async_hooks.js index 37ff565064..9bb3a993af 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -6,20 +6,21 @@ const errors = require('internal/errors'); * Environment::AsyncHooks::fields_[]. Each index tracks the number of active * hooks for each type. * - * async_uid_fields is a Float64Array wrapping the double array of + * async_id_fields is a Float64Array wrapping the double array of * Environment::AsyncHooks::uid_fields_[]. Each index contains the ids for the * various asynchronous states of the application. These are: - * kCurrentAsyncId: The async_id assigned to the resource responsible for the + * kExecutionAsyncId: The async_id assigned to the resource responsible for the * current execution stack. - * kCurrentTriggerId: The trigger_async_id of the resource responsible for the - * current execution stack. - * kAsyncUidCntr: Incremental counter tracking the next assigned async_id. - * kInitTriggerId: 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 kInitTriggerId -> executionAsyncId of the current resource. + * 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_uid_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::ids_stack_ tracks the resource responsible for the // current execution stack. This is unwound as each resource exits. In the case @@ -58,14 +59,14 @@ const active_hooks = { // 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, kTotals, - kCurrentAsyncId, kCurrentTriggerId, kAsyncUidCntr, - kInitTriggerId } = async_wrap.constants; +const { kInit, kBefore, kAfter, kDestroy, kTotals, kPromiseResolve, + kExecutionAsyncId, kTriggerAsyncId, kAsyncIdCounter, + kInitTriggerAsyncId } = async_wrap.constants; // 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_id_symbol } = async_wrap; +const { async_id_symbol, trigger_async_id_symbol } = async_wrap; // Used in AsyncHook and AsyncResource. const init_symbol = Symbol('init'); @@ -234,12 +235,12 @@ function createHook(fns) { function executionAsyncId() { - return async_uid_fields[kCurrentAsyncId]; + return async_id_fields[kExecutionAsyncId]; } function triggerAsyncId() { - return async_uid_fields[kCurrentTriggerId]; + return async_id_fields[kTriggerAsyncId]; } @@ -258,14 +259,16 @@ class AsyncResource { triggerAsyncId); } - this[async_id_symbol] = ++async_uid_fields[kAsyncUidCntr]; - this[trigger_id_symbol] = triggerAsyncId; + this[async_id_symbol] = ++async_id_fields[kAsyncIdCounter]; + this[trigger_async_id_symbol] = triggerAsyncId; - emitInitScript(this[async_id_symbol], type, this[trigger_id_symbol], this); + emitInitScript( + this[async_id_symbol], type, this[trigger_async_id_symbol], this + ); } emitBefore() { - emitBeforeScript(this[async_id_symbol], this[trigger_id_symbol]); + emitBeforeScript(this[async_id_symbol], this[trigger_async_id_symbol]); return this; } @@ -284,7 +287,7 @@ class AsyncResource { } triggerAsyncId() { - return this[trigger_id_symbol]; + return this[trigger_async_id_symbol]; } } @@ -308,7 +311,7 @@ function runInAsyncIdScope(asyncId, cb) { // counter increment first. Since it's done the same way in // Environment::new_async_uid() function newUid() { - return ++async_uid_fields[kAsyncUidCntr]; + return ++async_id_fields[kAsyncIdCounter]; } @@ -316,20 +319,20 @@ function newUid() { // the user to safeguard this call and make sure it's zero'd out when the // constructor is complete. function initTriggerId() { - var tId = async_uid_fields[kInitTriggerId]; + var triggerAsyncId = async_id_fields[kInitTriggerAsyncId]; // Reset value after it's been called so the next constructor doesn't // inherit it by accident. - async_uid_fields[kInitTriggerId] = 0; - if (tId <= 0) - tId = async_uid_fields[kCurrentAsyncId]; - return tId; + 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_uid_fields[kInitTriggerId] = triggerAsyncId; + async_id_fields[kInitTriggerAsyncId] = triggerAsyncId; } @@ -346,8 +349,9 @@ function emitInitScript(asyncId, type, triggerAsyncId, resource) { if (triggerAsyncId === null) { triggerAsyncId = initTriggerId(); } else { - // If a triggerAsyncId was passed, any kInitTriggerId still must be null'd. - async_uid_fields[kInitTriggerId] = 0; + // If a triggerAsyncId was passed, any kInitTriggerAsyncId still must be + // null'd. + async_id_fields[kInitTriggerAsyncId] = 0; } if (!Number.isSafeInteger(asyncId) || asyncId < -1) { @@ -446,7 +450,7 @@ function emitDestroyScript(asyncId) { // Return early if there are no destroy callbacks, or invalid asyncId. if (async_hook_fields[kDestroy] === 0 || asyncId <= 0) return; - async_wrap.addIdToDestroyList(asyncId); + async_wrap.queueDestroyAsyncId(asyncId); } diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index 9abaaf348d..e7676c0867 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -354,17 +354,18 @@ function setupProcessFatal() { const async_wrap = process.binding('async_wrap'); // Arrays containing hook flags and ids for async_hook calls. - const { async_hook_fields, async_uid_fields } = async_wrap; + const { async_hook_fields, async_id_fields } = async_wrap; // Internal functions needed to manipulate the stack. - const { clearIdStack, asyncIdStackSize } = async_wrap; - const { kAfter, kCurrentAsyncId, kInitTriggerId } = async_wrap.constants; + const { clearAsyncIdStack, asyncIdStackSize } = async_wrap; + const { kAfter, kExecutionAsyncId, + kInitTriggerAsyncId } = async_wrap.constants; process._fatalException = function(er) { var caught; - // It's possible that kInitTriggerId was set for a constructor call that - // threw and was never cleared. So clear it now. - async_uid_fields[kInitTriggerId] = 0; + // 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; if (process.domain && process.domain._errorHandler) caught = process.domain._errorHandler(er); @@ -392,11 +393,11 @@ if (async_hook_fields[kAfter] > 0) { do { NativeModule.require('async_hooks').emitAfter( - async_uid_fields[kCurrentAsyncId]); + async_id_fields[kExecutionAsyncId]); } while (asyncIdStackSize() > 0); // Or completely empty the id stack. } else { - clearIdStack(); + clearAsyncIdStack(); } } diff --git a/lib/internal/process/next_tick.js b/lib/internal/process/next_tick.js index 00698aee23..3854389a0e 100644 --- a/lib/internal/process/next_tick.js +++ b/lib/internal/process/next_tick.js @@ -54,12 +54,12 @@ function setupNextTick() { const emitPendingUnhandledRejections = promises.setup(scheduleMicrotasks); const initTriggerId = async_hooks.initTriggerId; // Two arrays that share state between C++ and JS. - const { async_hook_fields, async_uid_fields } = async_wrap; + 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, kAsyncUidCntr } = async_wrap.constants; - const { async_id_symbol, trigger_id_symbol } = async_wrap; + const { kInit, kDestroy, kAsyncIdCounter } = async_wrap.constants; + const { async_id_symbol, trigger_async_id_symbol } = async_wrap; var nextTickQueue = new NextTickQueue(); var microtasksScheduled = false; @@ -102,7 +102,7 @@ function setupNextTick() { args: undefined, domain: null, [async_id_symbol]: 0, - [trigger_id_symbol]: 0 + [trigger_async_id_symbol]: 0 }; function scheduleMicrotasks() { if (microtasksScheduled) @@ -158,10 +158,10 @@ function setupNextTick() { // CHECK(Number.isSafeInteger(tock[async_id_symbol])) // CHECK(tock[async_id_symbol] > 0) - // CHECK(Number.isSafeInteger(tock[trigger_id_symbol])) - // CHECK(tock[trigger_id_symbol] > 0) + // CHECK(Number.isSafeInteger(tock[trigger_async_id_symbol])) + // CHECK(tock[trigger_async_id_symbol] > 0) - emitBefore(tock[async_id_symbol], tock[trigger_id_symbol]); + emitBefore(tock[async_id_symbol], tock[trigger_async_id_symbol]); // emitDestroy() places the async_id_symbol into an asynchronous queue // that calls the destroy callback in the future. It's called before // calling tock.callback so destroy will be called even if the callback @@ -203,10 +203,10 @@ function setupNextTick() { // CHECK(Number.isSafeInteger(tock[async_id_symbol])) // CHECK(tock[async_id_symbol] > 0) - // CHECK(Number.isSafeInteger(tock[trigger_id_symbol])) - // CHECK(tock[trigger_id_symbol] > 0) + // CHECK(Number.isSafeInteger(tock[trigger_async_id_symbol])) + // CHECK(tock[trigger_async_id_symbol] > 0) - emitBefore(tock[async_id_symbol], tock[trigger_id_symbol]); + emitBefore(tock[async_id_symbol], tock[trigger_async_id_symbol]); // TODO(trevnorris): See comment in _tickCallback() as to why this // isn't a good solution. if (async_hook_fields[kDestroy] > 0) @@ -236,7 +236,7 @@ function setupNextTick() { this.args = args; this.domain = process.domain || null; this[async_id_symbol] = asyncId; - this[trigger_id_symbol] = triggerAsyncId; + this[trigger_async_id_symbol] = triggerAsyncId; } } @@ -261,7 +261,7 @@ function setupNextTick() { args[i - 1] = arguments[i]; } - const asyncId = ++async_uid_fields[kAsyncUidCntr]; + const asyncId = ++async_id_fields[kAsyncIdCounter]; const triggerAsyncId = initTriggerId(); const obj = new TickObject(callback, args, asyncId, triggerAsyncId); nextTickQueue.push(obj); @@ -297,7 +297,7 @@ function setupNextTick() { args[i - 2] = arguments[i]; } - const asyncId = ++async_uid_fields[kAsyncUidCntr]; + const asyncId = ++async_id_fields[kAsyncIdCounter]; const obj = new TickObject(callback, args, asyncId, triggerAsyncId); nextTickQueue.push(obj); ++tickInfo[kLength]; diff --git a/lib/timers.js b/lib/timers.js index 910671ce25..5727de159a 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -34,14 +34,14 @@ const debug = util.debuglog('timer'); const kOnTimeout = TimerWrap.kOnTimeout | 0; const initTriggerId = async_hooks.initTriggerId; // Two arrays that share state between C++ and JS. -const { async_hook_fields, async_uid_fields } = async_wrap; +const { async_hook_fields, async_id_fields } = async_wrap; // The needed emit*() functions. const { emitInit, emitBefore, emitAfter, emitDestroy } = async_hooks; // Grab the constants necessary for working with internal arrays. -const { kInit, kDestroy, kAsyncUidCntr } = async_wrap.constants; +const { kInit, kDestroy, kAsyncIdCounter } = async_wrap.constants; // Symbols for storing async id state. const async_id_symbol = Symbol('asyncId'); -const trigger_id_symbol = Symbol('triggerAsyncId'); +const trigger_async_id_symbol = Symbol('triggerAsyncId'); // Timeout values > TIMEOUT_MAX are set to 1. const TIMEOUT_MAX = 2147483647; // 2^31-1 @@ -169,10 +169,12 @@ function insert(item, unrefed) { if (!item[async_id_symbol] || item._destroyed) { item._destroyed = false; - item[async_id_symbol] = ++async_uid_fields[kAsyncUidCntr]; - item[trigger_id_symbol] = initTriggerId(); + item[async_id_symbol] = ++async_id_fields[kAsyncIdCounter]; + item[trigger_async_id_symbol] = initTriggerId(); if (async_hook_fields[kInit] > 0) - emitInit(item[async_id_symbol], 'Timeout', item[trigger_id_symbol], item); + emitInit( + item[async_id_symbol], 'Timeout', item[trigger_async_id_symbol], item + ); } L.append(list, item); @@ -291,7 +293,7 @@ function tryOnTimeout(timer, list) { timer[async_id_symbol] : null; var threw = true; if (timerAsyncId !== null) - emitBefore(timerAsyncId, timer[trigger_id_symbol]); + emitBefore(timerAsyncId, timer[trigger_async_id_symbol]); try { ontimeout(timer); threw = false; @@ -577,10 +579,12 @@ function Timeout(after, callback, args) { this._timerArgs = args; this._repeat = null; this._destroyed = false; - this[async_id_symbol] = ++async_uid_fields[kAsyncUidCntr]; - this[trigger_id_symbol] = initTriggerId(); + this[async_id_symbol] = ++async_id_fields[kAsyncIdCounter]; + this[trigger_async_id_symbol] = initTriggerId(); if (async_hook_fields[kInit] > 0) - emitInit(this[async_id_symbol], 'Timeout', this[trigger_id_symbol], this); + emitInit( + this[async_id_symbol], 'Timeout', this[trigger_async_id_symbol], this + ); } @@ -750,7 +754,7 @@ function processImmediate() { // 4.7) what is in this smaller function. function tryOnImmediate(immediate, oldTail) { var threw = true; - emitBefore(immediate[async_id_symbol], immediate[trigger_id_symbol]); + emitBefore(immediate[async_id_symbol], immediate[trigger_async_id_symbol]); try { // make the actual call outside the try/finally to allow it to be optimized runCallback(immediate); @@ -815,10 +819,12 @@ function Immediate() { this._onImmediate = null; this._destroyed = false; this.domain = process.domain; - this[async_id_symbol] = ++async_uid_fields[kAsyncUidCntr]; - this[trigger_id_symbol] = initTriggerId(); + this[async_id_symbol] = ++async_id_fields[kAsyncIdCounter]; + this[trigger_async_id_symbol] = initTriggerId(); if (async_hook_fields[kInit] > 0) - emitInit(this[async_id_symbol], 'Immediate', this[trigger_id_symbol], this); + emitInit( + this[async_id_symbol], 'Immediate', this[trigger_async_id_symbol], this + ); } function setImmediate(callback, arg1, arg2, arg3) { diff --git a/src/async-wrap-inl.h b/src/async-wrap-inl.h index 4fcfe2e757..2d10a6d5f7 100644 --- a/src/async-wrap-inl.h +++ b/src/async-wrap-inl.h @@ -36,13 +36,13 @@ inline AsyncWrap::ProviderType AsyncWrap::provider_type() const { } -inline double AsyncWrap::get_id() const { +inline double AsyncWrap::get_async_id() const { return async_id_; } -inline double AsyncWrap::get_trigger_id() const { - return trigger_id_; +inline double AsyncWrap::get_trigger_async_id() const { + return trigger_async_id_; } diff --git a/src/async-wrap.cc b/src/async-wrap.cc index beb04343e0..7ca3f6c604 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -140,8 +140,8 @@ RetainedObjectInfo* WrapperInfo(uint16_t class_id, Local wrapper) { // end RetainedAsyncInfo -static void DestroyIdsCb(uv_timer_t* handle) { - Environment* env = Environment::from_destroy_ids_timer_handle(handle); +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()); @@ -150,16 +150,16 @@ static void DestroyIdsCb(uv_timer_t* handle) { TryCatch try_catch(env->isolate()); do { - std::vector destroy_ids_list; - destroy_ids_list.swap(*env->destroy_ids_list()); + std::vector destroy_async_id_list; if (!env->can_call_into_js()) return; - for (auto current_id : destroy_ids_list) { + destroy_async_id_list.swap(*env->destroy_async_id_list()); + for (auto async_id : destroy_async_id_list) { // Want each callback to be cleaned up after itself, instead of cleaning // them all up after the while() loop completes. HandleScope scope(env->isolate()); - Local argv = Number::New(env->isolate(), current_id); + Local async_id_value = Number::New(env->isolate(), async_id); MaybeLocal ret = fn->Call( - env->context(), Undefined(env->isolate()), 1, &argv); + env->context(), Undefined(env->isolate()), 1, &async_id_value); if (ret.IsEmpty()) { ClearFatalExceptionHandlers(env); @@ -167,21 +167,22 @@ static void DestroyIdsCb(uv_timer_t* handle) { UNREACHABLE(); } } - } while (!env->destroy_ids_list()->empty()); + } while (!env->destroy_async_id_list()->empty()); } -static void PushBackDestroyId(Environment* env, double id) { +static void PushBackDestroyAsyncId(Environment* env, double id) { if (env->async_hooks()->fields()[AsyncHooks::kDestroy] == 0) return; if (!env->can_call_into_js()) return; - if (env->destroy_ids_list()->empty()) - uv_timer_start(env->destroy_ids_timer_handle(), DestroyIdsCb, 0, 0); + if (env->destroy_async_id_list()->empty()) + uv_timer_start(env->destroy_async_ids_timer_handle(), + DestroyAsyncIdsCallback, 0, 0); - env->destroy_ids_list()->push_back(id); + env->destroy_async_id_list()->push_back(id); } @@ -191,11 +192,11 @@ void AsyncWrap::EmitPromiseResolve(Environment* env, double async_id) { if (async_hooks->fields()[AsyncHooks::kPromiseResolve] == 0) return; - Local uid = Number::New(env->isolate(), async_id); + Local async_id_value = Number::New(env->isolate(), async_id); Local fn = env->async_hooks_promise_resolve_function(); TryCatch try_catch(env->isolate()); MaybeLocal ar = fn->Call( - env->context(), Undefined(env->isolate()), 1, &uid); + env->context(), Undefined(env->isolate()), 1, &async_id_value); if (ar.IsEmpty()) { ClearFatalExceptionHandlers(env); FatalException(env->isolate(), try_catch); @@ -210,11 +211,11 @@ void AsyncWrap::EmitBefore(Environment* env, double async_id) { if (async_hooks->fields()[AsyncHooks::kBefore] == 0) return; - Local uid = Number::New(env->isolate(), async_id); + Local async_id_value = Number::New(env->isolate(), async_id); Local fn = env->async_hooks_before_function(); TryCatch try_catch(env->isolate()); MaybeLocal ar = fn->Call( - env->context(), Undefined(env->isolate()), 1, &uid); + env->context(), Undefined(env->isolate()), 1, &async_id_value); if (ar.IsEmpty()) { ClearFatalExceptionHandlers(env); FatalException(env->isolate(), try_catch); @@ -231,11 +232,11 @@ void AsyncWrap::EmitAfter(Environment* env, double async_id) { // If the user's callback failed then the after() hooks will be called at the // end of _fatalException(). - Local uid = Number::New(env->isolate(), async_id); + Local async_id_value = Number::New(env->isolate(), async_id); Local fn = env->async_hooks_after_function(); TryCatch try_catch(env->isolate()); MaybeLocal ar = fn->Call( - env->context(), Undefined(env->isolate()), 1, &uid); + env->context(), Undefined(env->isolate()), 1, &async_id_value); if (ar.IsEmpty()) { ClearFatalExceptionHandlers(env); FatalException(env->isolate(), try_catch); @@ -252,7 +253,7 @@ class PromiseWrap : public AsyncWrap { size_t self_size() const override { return sizeof(*this); } static constexpr int kPromiseField = 1; - static constexpr int kParentIdField = 2; + static constexpr int kParentAsyncIdField = 2; static constexpr int kInternalFieldCount = 3; static PromiseWrap* New(Environment* env, @@ -261,7 +262,7 @@ class PromiseWrap : public AsyncWrap { bool silent); static void GetPromise(Local property, const PropertyCallbackInfo& info); - static void GetParentId(Local property, + static void getParentAsyncId(Local property, const PropertyCallbackInfo& info); }; @@ -273,9 +274,9 @@ PromiseWrap* PromiseWrap::New(Environment* env, ->NewInstance(env->context()).ToLocalChecked(); object->SetInternalField(PromiseWrap::kPromiseField, promise); if (parent_wrap != nullptr) { - object->SetInternalField(PromiseWrap::kParentIdField, + object->SetInternalField(PromiseWrap::kParentAsyncIdField, Number::New(env->isolate(), - parent_wrap->get_id())); + parent_wrap->get_async_id())); } CHECK_EQ(promise->GetAlignedPointerFromInternalField(0), nullptr); promise->SetInternalField(0, object); @@ -287,9 +288,10 @@ void PromiseWrap::GetPromise(Local property, info.GetReturnValue().Set(info.Holder()->GetInternalField(kPromiseField)); } -void PromiseWrap::GetParentId(Local property, +void PromiseWrap::getParentAsyncId(Local property, const PropertyCallbackInfo& info) { - info.GetReturnValue().Set(info.Holder()->GetInternalField(kParentIdField)); + info.GetReturnValue().Set( + info.Holder()->GetInternalField(kParentAsyncIdField)); } static void PromiseHook(PromiseHookType type, Local promise, @@ -321,8 +323,8 @@ static void PromiseHook(PromiseHookType type, Local promise, parent_wrap = PromiseWrap::New(env, parent_promise, nullptr, true); } // get id from parentWrap - double trigger_id = parent_wrap->get_id(); - env->set_init_trigger_id(trigger_id); + double trigger_async_id = parent_wrap->get_async_id(); + env->set_init_trigger_async_id(trigger_async_id); } wrap = PromiseWrap::New(env, promise, parent_wrap, silent); @@ -330,20 +332,21 @@ static void PromiseHook(PromiseHookType type, Local promise, CHECK_NE(wrap, nullptr); if (type == PromiseHookType::kBefore) { - env->async_hooks()->push_ids(wrap->get_id(), wrap->get_trigger_id()); - AsyncWrap::EmitBefore(wrap->env(), wrap->get_id()); + env->async_hooks()->push_async_ids( + wrap->get_async_id(), wrap->get_trigger_async_id()); + AsyncWrap::EmitBefore(wrap->env(), wrap->get_async_id()); } else if (type == PromiseHookType::kAfter) { - AsyncWrap::EmitAfter(wrap->env(), wrap->get_id()); - if (env->current_async_id() == wrap->get_id()) { + 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 // the promise callback execution. // Popping it off the stack can be skipped in that case, because is is // known that it would correspond to exactly one call with // PromiseHookType::kBefore that was not witnessed by the PromiseHook. - env->async_hooks()->pop_ids(wrap->get_id()); + env->async_hooks()->pop_async_id(wrap->get_async_id()); } } else if (type == PromiseHookType::kResolve) { - AsyncWrap::EmitPromiseResolve(wrap->env(), wrap->get_id()); + AsyncWrap::EmitPromiseResolve(wrap->env(), wrap->get_async_id()); } } @@ -387,7 +390,7 @@ static void SetupHooks(const FunctionCallbackInfo& args) { PromiseWrap::GetPromise); promise_wrap_template->SetAccessor( FIXED_ONE_BYTE_STRING(env->isolate(), "parentId"), - PromiseWrap::GetParentId); + PromiseWrap::getParentAsyncId); env->set_promise_wrap_template(promise_wrap_template); } } @@ -415,24 +418,24 @@ void AsyncWrap::GetAsyncId(const FunctionCallbackInfo& args) { AsyncWrap* wrap; args.GetReturnValue().Set(-1); ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder()); - args.GetReturnValue().Set(wrap->get_id()); + args.GetReturnValue().Set(wrap->get_async_id()); } void AsyncWrap::PushAsyncIds(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); // No need for CHECK(IsNumber()) on args because if FromJust() doesn't fail - // then the checks in push_ids() and pop_ids() will. + // then the checks in push_async_ids() and pop_async_id() will. double async_id = args[0]->NumberValue(env->context()).FromJust(); - double trigger_id = args[1]->NumberValue(env->context()).FromJust(); - env->async_hooks()->push_ids(async_id, trigger_id); + double trigger_async_id = args[1]->NumberValue(env->context()).FromJust(); + env->async_hooks()->push_async_ids(async_id, trigger_async_id); } void AsyncWrap::PopAsyncIds(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); double async_id = args[0]->NumberValue(env->context()).FromJust(); - args.GetReturnValue().Set(env->async_hooks()->pop_ids(async_id)); + args.GetReturnValue().Set(env->async_hooks()->pop_async_id(async_id)); } @@ -443,9 +446,9 @@ void AsyncWrap::AsyncIdStackSize(const FunctionCallbackInfo& args) { } -void AsyncWrap::ClearIdStack(const FunctionCallbackInfo& args) { +void AsyncWrap::ClearAsyncIdStack(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - env->async_hooks()->clear_id_stack(); + env->async_hooks()->clear_async_id_stack(); } @@ -456,9 +459,9 @@ void AsyncWrap::AsyncReset(const FunctionCallbackInfo& args) { } -void AsyncWrap::QueueDestroyId(const FunctionCallbackInfo& args) { +void AsyncWrap::QueueDestroyAsyncId(const FunctionCallbackInfo& args) { CHECK(args[0]->IsNumber()); - PushBackDestroyId(Environment::GetCurrent(args), args[0]->NumberValue()); + PushBackDestroyAsyncId(Environment::GetCurrent(args), args[0]->NumberValue()); } void AsyncWrap::AddWrapMethods(Environment* env, @@ -480,8 +483,8 @@ void AsyncWrap::Initialize(Local target, env->SetMethod(target, "pushAsyncIds", PushAsyncIds); env->SetMethod(target, "popAsyncIds", PopAsyncIds); env->SetMethod(target, "asyncIdStackSize", AsyncIdStackSize); - env->SetMethod(target, "clearIdStack", ClearIdStack); - env->SetMethod(target, "addIdToDestroyList", QueueDestroyId); + env->SetMethod(target, "clearAsyncIdStack", ClearAsyncIdStack); + env->SetMethod(target, "queueDestroyAsyncId", QueueDestroyAsyncId); env->SetMethod(target, "enablePromiseHook", EnablePromiseHook); env->SetMethod(target, "disablePromiseHook", DisablePromiseHook); @@ -508,12 +511,12 @@ void AsyncWrap::Initialize(Local target, // // kAsyncUid: Maintains the state of the next unique id to be assigned. // - // kInitTriggerId: 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 kInitTriggerId is set back to 0. + // kInitTriggerAsyncId: 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. FORCE_SET_TARGET_FIELD(target, - "async_uid_fields", - env->async_hooks()->uid_fields().GetJSArray()); + "async_id_fields", + env->async_hooks()->async_id_fields().GetJSArray()); Local constants = Object::New(isolate); #define SET_HOOKS_CONSTANT(name) \ @@ -526,10 +529,10 @@ void AsyncWrap::Initialize(Local target, SET_HOOKS_CONSTANT(kDestroy); SET_HOOKS_CONSTANT(kPromiseResolve); SET_HOOKS_CONSTANT(kTotals); - SET_HOOKS_CONSTANT(kCurrentAsyncId); - SET_HOOKS_CONSTANT(kCurrentTriggerId); - SET_HOOKS_CONSTANT(kAsyncUidCntr); - SET_HOOKS_CONSTANT(kInitTriggerId); + SET_HOOKS_CONSTANT(kExecutionAsyncId); + SET_HOOKS_CONSTANT(kTriggerAsyncId); + SET_HOOKS_CONSTANT(kAsyncIdCounter); + SET_HOOKS_CONSTANT(kInitTriggerAsyncId); #undef SET_HOOKS_CONSTANT FORCE_SET_TARGET_FIELD(target, "constants", constants); @@ -549,8 +552,8 @@ void AsyncWrap::Initialize(Local target, Symbol::New(isolate, FIXED_ONE_BYTE_STRING(isolate, "asyncId"))); FORCE_SET_TARGET_FIELD( target, - "trigger_id_symbol", - Symbol::New(isolate, FIXED_ONE_BYTE_STRING(isolate, "triggerId"))); + "trigger_async_id_symbol", + Symbol::New(isolate, FIXED_ONE_BYTE_STRING(isolate, "triggerAsyncId"))); #undef FORCE_SET_TARGET_FIELD @@ -605,7 +608,7 @@ AsyncWrap::AsyncWrap(Environment* env, AsyncWrap::~AsyncWrap() { - PushBackDestroyId(env(), get_id()); + PushBackDestroyAsyncId(env(), get_async_id()); } @@ -614,13 +617,13 @@ AsyncWrap::~AsyncWrap() { // the resource is pulled out of the pool and put back into use. void AsyncWrap::AsyncReset(bool silent) { async_id_ = env()->new_async_id(); - trigger_id_ = env()->get_init_trigger_id(); + trigger_async_id_ = env()->get_init_trigger_async_id(); if (silent) return; EmitAsyncInit(env(), object(), env()->async_hooks()->provider_string(provider_type()), - async_id_, trigger_id_); + async_id_, trigger_async_id_); } @@ -628,7 +631,7 @@ void AsyncWrap::EmitAsyncInit(Environment* env, Local object, Local type, double async_id, - double trigger_id) { + double trigger_async_id) { CHECK(!object.IsEmpty()); CHECK(!type.IsEmpty()); AsyncHooks* async_hooks = env->async_hooks(); @@ -644,7 +647,7 @@ void AsyncWrap::EmitAsyncInit(Environment* env, Local argv[] = { Number::New(env->isolate(), async_id), type, - Number::New(env->isolate(), trigger_id), + Number::New(env->isolate(), trigger_async_id), object, }; @@ -662,7 +665,7 @@ void AsyncWrap::EmitAsyncInit(Environment* env, MaybeLocal AsyncWrap::MakeCallback(const Local cb, int argc, Local* argv) { - async_context context { get_id(), get_trigger_id() }; + async_context context { get_async_id(), get_trigger_async_id() }; return InternalMakeCallback(env(), object(), cb, argc, argv, context); } @@ -671,12 +674,12 @@ MaybeLocal AsyncWrap::MakeCallback(const Local cb, async_id AsyncHooksGetExecutionAsyncId(Isolate* isolate) { - return Environment::GetCurrent(isolate)->current_async_id(); + return Environment::GetCurrent(isolate)->execution_async_id(); } async_id AsyncHooksGetTriggerAsyncId(Isolate* isolate) { - return Environment::GetCurrent(isolate)->trigger_id(); + return Environment::GetCurrent(isolate)->trigger_async_id(); } @@ -698,7 +701,7 @@ async_context EmitAsyncInit(Isolate* isolate, // Initialize async context struct if (trigger_async_id == -1) - trigger_async_id = env->get_init_trigger_id(); + trigger_async_id = env->get_init_trigger_async_id(); async_context context = { env->new_async_id(), // async_id_ @@ -713,7 +716,8 @@ async_context EmitAsyncInit(Isolate* isolate, } void EmitAsyncDestroy(Isolate* isolate, async_context asyncContext) { - PushBackDestroyId(Environment::GetCurrent(isolate), asyncContext.async_id); + PushBackDestroyAsyncId(Environment::GetCurrent(isolate), + asyncContext.async_id); } } // namespace node diff --git a/src/async-wrap.h b/src/async-wrap.h index 386c39537a..0e0415da17 100644 --- a/src/async-wrap.h +++ b/src/async-wrap.h @@ -110,15 +110,17 @@ class AsyncWrap : public BaseObject { static void PushAsyncIds(const v8::FunctionCallbackInfo& args); static void PopAsyncIds(const v8::FunctionCallbackInfo& args); static void AsyncIdStackSize(const v8::FunctionCallbackInfo& args); - static void ClearIdStack(const v8::FunctionCallbackInfo& args); + static void ClearAsyncIdStack( + const v8::FunctionCallbackInfo& args); static void AsyncReset(const v8::FunctionCallbackInfo& args); - static void QueueDestroyId(const v8::FunctionCallbackInfo& args); + static void QueueDestroyAsyncId( + const v8::FunctionCallbackInfo& args); static void EmitAsyncInit(Environment* env, v8::Local object, v8::Local type, double id, - double trigger_id); + double trigger_async_id); static void EmitBefore(Environment* env, double id); static void EmitAfter(Environment* env, double id); @@ -126,9 +128,9 @@ class AsyncWrap : public BaseObject { inline ProviderType provider_type() const; - inline double get_id() const; + inline double get_async_id() const; - inline double get_trigger_id() const; + inline double get_trigger_async_id() const; void AsyncReset(bool silent = false); @@ -155,7 +157,7 @@ class AsyncWrap : public BaseObject { const ProviderType provider_type_; // Because the values may be Reset(), cannot be made const. double async_id_; - double trigger_id_; + double trigger_async_id_; }; void LoadAsyncWrapperInfo(Environment* env); diff --git a/src/connection_wrap.cc b/src/connection_wrap.cc index da65c49316..d894ef6cc0 100644 --- a/src/connection_wrap.cc +++ b/src/connection_wrap.cc @@ -51,7 +51,7 @@ void ConnectionWrap::OnConnection(uv_stream_t* handle, }; if (status == 0) { - env->set_init_trigger_id(wrap_data->get_id()); + 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); diff --git a/src/env-inl.h b/src/env-inl.h index 5372c34a92..bed897298a 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -85,12 +85,12 @@ inline uint32_t* IsolateData::zero_fill_field() const { inline Environment::AsyncHooks::AsyncHooks(v8::Isolate* isolate) : isolate_(isolate), fields_(isolate, kFieldsCount), - uid_fields_(isolate, kUidFieldsCount) { + async_id_fields_(isolate, kUidFieldsCount) { v8::HandleScope handle_scope(isolate_); - // kAsyncUidCntr should start at 1 because that'll be the id the execution + // kAsyncIdCounter should start at 1 because that'll be the id the execution // context during bootstrap (code that runs before entering uv_run()). - uid_fields_[AsyncHooks::kAsyncUidCntr] = 1; + async_id_fields_[AsyncHooks::kAsyncIdCounter] = 1; // Create all the provider strings that will be passed to JS. Place them in // an array so the array index matches the PROVIDER id offset. This way the @@ -117,11 +117,11 @@ inline int Environment::AsyncHooks::fields_count() const { } inline AliasedBuffer& -Environment::AsyncHooks::uid_fields() { - return uid_fields_; +Environment::AsyncHooks::async_id_fields() { + return async_id_fields_; } -inline int Environment::AsyncHooks::uid_fields_count() const { +inline int Environment::AsyncHooks::async_id_fields_count() const { return kUidFieldsCount; } @@ -129,29 +129,29 @@ inline v8::Local Environment::AsyncHooks::provider_string(int idx) { return providers_[idx].Get(isolate_); } -inline void Environment::AsyncHooks::push_ids(double async_id, - double trigger_id) { +inline void Environment::AsyncHooks::push_async_ids(double async_id, + double trigger_async_id) { CHECK_GE(async_id, -1); - CHECK_GE(trigger_id, -1); + CHECK_GE(trigger_async_id, -1); - ids_stack_.push({ uid_fields_[kCurrentAsyncId], - uid_fields_[kCurrentTriggerId] }); - uid_fields_[kCurrentAsyncId] = async_id; - uid_fields_[kCurrentTriggerId] = trigger_id; + async_ids_stack_.push({ async_id_fields_[kExecutionAsyncId], + async_id_fields_[kTriggerAsyncId] }); + async_id_fields_[kExecutionAsyncId] = async_id; + async_id_fields_[kTriggerAsyncId] = trigger_async_id; } -inline bool Environment::AsyncHooks::pop_ids(double async_id) { +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 (ids_stack_.empty()) return false; + if (async_ids_stack_.empty()) return false; // Ask for the async_id to be restored as a sanity check that the stack // hasn't been corrupted. - if (uid_fields_[kCurrentAsyncId] != async_id) { + if (async_id_fields_[kExecutionAsyncId] != async_id) { fprintf(stderr, "Error: async hook stack has become corrupted (" "actual: %.f, expected: %.f)\n", - uid_fields_.GetValue(kCurrentAsyncId), + async_id_fields_.GetValue(kExecutionAsyncId), async_id); Environment* env = Environment::GetCurrent(isolate_); DumpBacktrace(stderr); @@ -163,35 +163,37 @@ inline bool Environment::AsyncHooks::pop_ids(double async_id) { ABORT_NO_BACKTRACE(); } - auto ids = ids_stack_.top(); - ids_stack_.pop(); - uid_fields_[kCurrentAsyncId] = ids.async_id; - uid_fields_[kCurrentTriggerId] = ids.trigger_id; - return !ids_stack_.empty(); + 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(); } inline size_t Environment::AsyncHooks::stack_size() { - return ids_stack_.size(); + return async_ids_stack_.size(); } -inline void Environment::AsyncHooks::clear_id_stack() { - while (!ids_stack_.empty()) - ids_stack_.pop(); - uid_fields_[kCurrentAsyncId] = 0; - uid_fields_[kCurrentTriggerId] = 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; } inline Environment::AsyncHooks::InitScope::InitScope( - Environment* env, double init_trigger_id) + Environment* env, double init_trigger_async_id) : env_(env), - uid_fields_ref_(env->async_hooks()->uid_fields()) { - CHECK_GE(init_trigger_id, -1); - env->async_hooks()->push_ids(uid_fields_ref_[AsyncHooks::kCurrentAsyncId], - init_trigger_id); + async_id_fields_ref_(env->async_hooks()->async_id_fields()) { + CHECK_GE(init_trigger_async_id, -1); + env->async_hooks()->push_async_ids( + async_id_fields_ref_[AsyncHooks::kExecutionAsyncId], + init_trigger_async_id); } inline Environment::AsyncHooks::InitScope::~InitScope() { - env_->async_hooks()->pop_ids(uid_fields_ref_[AsyncHooks::kCurrentAsyncId]); + env_->async_hooks()->pop_async_id( + async_id_fields_ref_[AsyncHooks::kExecutionAsyncId]); } inline Environment::AsyncCallbackScope::AsyncCallbackScope(Environment* env) @@ -305,7 +307,7 @@ inline Environment::Environment(IsolateData* isolate_data, AssignToContext(context); - destroy_ids_list_.reserve(512); + destroy_async_id_list_.reserve(512); performance_state_ = Calloc(1); performance_state_->milestones[ performance::NODE_PERFORMANCE_MILESTONE_ENVIRONMENT] = @@ -358,13 +360,13 @@ inline uv_idle_t* Environment::immediate_idle_handle() { return &immediate_idle_handle_; } -inline Environment* Environment::from_destroy_ids_timer_handle( +inline Environment* Environment::from_destroy_async_ids_timer_handle( uv_timer_t* handle) { - return ContainerOf(&Environment::destroy_ids_timer_handle_, handle); + return ContainerOf(&Environment::destroy_async_ids_timer_handle_, handle); } -inline uv_timer_t* Environment::destroy_ids_timer_handle() { - return &destroy_ids_timer_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, @@ -453,35 +455,35 @@ inline void Environment::set_abort_on_uncaught_exception(bool value) { abort_on_uncaught_exception_ = value; } -inline std::vector* Environment::destroy_ids_list() { - return &destroy_ids_list_; +inline std::vector* Environment::destroy_async_id_list() { + return &destroy_async_id_list_; } inline double Environment::new_async_id() { - async_hooks()->uid_fields()[AsyncHooks::kAsyncUidCntr] = - async_hooks()->uid_fields()[AsyncHooks::kAsyncUidCntr] + 1; - return async_hooks()->uid_fields()[AsyncHooks::kAsyncUidCntr]; + async_hooks()->async_id_fields()[AsyncHooks::kAsyncIdCounter] = + async_hooks()->async_id_fields()[AsyncHooks::kAsyncIdCounter] + 1; + return async_hooks()->async_id_fields()[AsyncHooks::kAsyncIdCounter]; } -inline double Environment::current_async_id() { - return async_hooks()->uid_fields()[AsyncHooks::kCurrentAsyncId]; +inline double Environment::execution_async_id() { + return async_hooks()->async_id_fields()[AsyncHooks::kExecutionAsyncId]; } -inline double Environment::trigger_id() { - return async_hooks()->uid_fields()[AsyncHooks::kCurrentTriggerId]; +inline double Environment::trigger_async_id() { + return async_hooks()->async_id_fields()[AsyncHooks::kTriggerAsyncId]; } -inline double Environment::get_init_trigger_id() { - AliasedBuffer& uid_fields = - async_hooks()->uid_fields(); - double tid = uid_fields[AsyncHooks::kInitTriggerId]; - uid_fields[AsyncHooks::kInitTriggerId] = 0; - if (tid <= 0) tid = current_async_id(); +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_id(const double id) { - async_hooks()->uid_fields()[AsyncHooks::kInitTriggerId] = id; +inline void Environment::set_init_trigger_async_id(const double id) { + async_hooks()->async_id_fields()[AsyncHooks::kInitTriggerAsyncId] = id; } inline double* Environment::heap_statistics_buffer() const { diff --git a/src/env.cc b/src/env.cc index 79c819708d..5a6a69704d 100644 --- a/src/env.cc +++ b/src/env.cc @@ -49,7 +49,7 @@ 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_ids_timer_handle()); + uv_timer_init(event_loop(), destroy_async_ids_timer_handle()); auto close_and_finish = [](Environment* env, uv_handle_t* handle, void* arg) { env->CloseHandle(handle, [](uv_handle_t* handle) {}); @@ -72,7 +72,7 @@ void Environment::Start(int argc, close_and_finish, nullptr); RegisterHandleCleanup( - reinterpret_cast(&destroy_ids_timer_handle_), + reinterpret_cast(&destroy_async_ids_timer_handle_), close_and_finish, nullptr); diff --git a/src/env.h b/src/env.h index 2be0eef453..2672ab7fb7 100644 --- a/src/env.h +++ b/src/env.h @@ -333,7 +333,7 @@ class Environment; struct node_async_ids { double async_id; - double trigger_id; + double trigger_async_id; }; class IsolateData { @@ -389,10 +389,10 @@ class Environment { }; enum UidFields { - kCurrentAsyncId, - kCurrentTriggerId, - kAsyncUidCntr, - kInitTriggerId, + kExecutionAsyncId, + kTriggerAsyncId, + kAsyncIdCounter, + kInitTriggerAsyncId, kUidFieldsCount, }; @@ -401,28 +401,28 @@ class Environment { inline AliasedBuffer& fields(); inline int fields_count() const; - inline AliasedBuffer& uid_fields(); - inline int uid_fields_count() const; + inline AliasedBuffer& async_id_fields(); + inline int async_id_fields_count() const; inline v8::Local provider_string(int idx); - inline void push_ids(double async_id, double trigger_id); - inline bool pop_ids(double async_id); + 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_id_stack(); // Used in fatal exceptions. + inline void clear_async_id_stack(); // Used in fatal exceptions. - // Used to propagate the trigger_id to the constructor of any newly created - // resources using RAII. Instead of needing to pass the trigger_id along - // with other constructor arguments. + // 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 { public: InitScope() = delete; - explicit InitScope(Environment* env, double init_trigger_id); + explicit InitScope(Environment* env, double init_trigger_async_id); ~InitScope(); private: Environment* env_; - AliasedBuffer uid_fields_ref_; + AliasedBuffer async_id_fields_ref_; DISALLOW_COPY_AND_ASSIGN(InitScope); }; @@ -435,12 +435,12 @@ class Environment { // Used by provider_string(). v8::Isolate* isolate_; // Stores the ids of the current execution context stack. - std::stack ids_stack_; + std::stack 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 uid_fields_; + AliasedBuffer async_id_fields_; DISALLOW_COPY_AND_ASSIGN(AsyncHooks); }; @@ -550,10 +550,11 @@ class Environment { inline uint32_t watched_providers() const; static inline Environment* from_immediate_check_handle(uv_check_t* handle); - static inline Environment* from_destroy_ids_timer_handle(uv_timer_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_ids_timer_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, @@ -587,13 +588,13 @@ class Environment { // The necessary API for async_hooks. inline double new_async_id(); - inline double current_async_id(); - inline double trigger_id(); - inline double get_init_trigger_id(); - inline void set_init_trigger_id(const double 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); // List of id's that have been destroyed and need the destroy() cb called. - inline std::vector* destroy_ids_list(); + inline std::vector* destroy_async_id_list(); std::unordered_multimap module_map; @@ -699,7 +700,7 @@ class Environment { IsolateData* const isolate_data_; uv_check_t immediate_check_handle_; uv_idle_t immediate_idle_handle_; - uv_timer_t destroy_ids_timer_handle_; + uv_timer_t destroy_async_ids_timer_handle_; uv_prepare_t idle_prepare_handle_; uv_check_t idle_check_handle_; @@ -713,7 +714,7 @@ class Environment { bool abort_on_uncaught_exception_; bool emit_napi_warning_; size_t makecallback_cntr_; - std::vector destroy_ids_list_; + std::vector destroy_async_id_list_; bool can_call_into_js_ = true; diff --git a/src/node.cc b/src/node.cc index e2e4cec7b9..80b6e37a88 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1402,7 +1402,7 @@ InternalCallbackScope::InternalCallbackScope(Environment* env, AsyncWrap::EmitBefore(env, asyncContext.async_id); } - env->async_hooks()->push_ids(async_context_.async_id, + env->async_hooks()->push_async_ids(async_context_.async_id, async_context_.trigger_async_id); pushed_ids_ = true; } @@ -1417,7 +1417,7 @@ void InternalCallbackScope::Close() { HandleScope handle_scope(env_->isolate()); if (pushed_ids_) - env_->async_hooks()->pop_ids(async_context_.async_id); + env_->async_hooks()->pop_async_id(async_context_.async_id); if (failed_) return; @@ -1442,8 +1442,8 @@ void InternalCallbackScope::Close() { // Make sure the stack unwound properly. If there are nested MakeCallback's // then it should return early and not reach this code. - CHECK_EQ(env_->current_async_id(), 0); - CHECK_EQ(env_->trigger_id(), 0); + CHECK_EQ(env_->execution_async_id(), 0); + CHECK_EQ(env_->trigger_async_id(), 0); Local process = env_->process_object(); @@ -1452,8 +1452,8 @@ void InternalCallbackScope::Close() { return; } - CHECK_EQ(env_->current_async_id(), 0); - CHECK_EQ(env_->trigger_id(), 0); + CHECK_EQ(env_->execution_async_id(), 0); + CHECK_EQ(env_->trigger_async_id(), 0); if (!env_->can_call_into_js()) return; @@ -4730,9 +4730,9 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data, { Environment::AsyncCallbackScope callback_scope(&env); - env.async_hooks()->push_ids(1, 0); + env.async_hooks()->push_async_ids(1, 0); LoadEnvironment(&env); - env.async_hooks()->pop_ids(1); + env.async_hooks()->pop_async_id(1); } env.set_trace_sync_io(trace_sync_io); diff --git a/src/node.h b/src/node.h index 368d0aa8a5..1ec5dd8ee9 100644 --- a/src/node.h +++ b/src/node.h @@ -569,7 +569,7 @@ NODE_EXTERN async_id AsyncHooksGetTriggerAsyncId(v8::Isolate* isolate); /* If the native API doesn't inherit from the helper class then the callbacks * must be triggered manually. This triggers the init() callback. The return - * value is the uid assigned to the resource. + * value is the async id assigned to the resource. * * The `trigger_async_id` parameter should correspond to the resource which is * creating the new resource, which will usually be the return value of diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index dfe0028147..11d0aa42b2 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -479,7 +479,7 @@ class Parser : public AsyncWrap { ASSIGN_OR_RETURN_UNWRAP(&parser, args.Holder()); // Should always be called from the same context. CHECK_EQ(env, parser->env()); - // The parser is being reused. Reset the uid and call init() callbacks. + // The parser is being reused. Reset the async id and call init() callbacks. parser->AsyncReset(); parser->Init(type); } diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc index a5642dc6fb..11fd830ced 100644 --- a/src/pipe_wrap.cc +++ b/src/pipe_wrap.cc @@ -52,7 +52,7 @@ using AsyncHooks = Environment::AsyncHooks; Local PipeWrap::Instantiate(Environment* env, AsyncWrap* parent) { EscapableHandleScope handle_scope(env->isolate()); - AsyncHooks::InitScope init_scope(env, parent->get_id()); + 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()); diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h index 8b5b154207..25293d2d06 100644 --- a/src/stream_base-inl.h +++ b/src/stream_base-inl.h @@ -136,7 +136,7 @@ void StreamBase::JSMethod(const FunctionCallbackInfo& args) { if (!wrap->IsAlive()) return args.GetReturnValue().Set(UV_EINVAL); - AsyncHooks::InitScope init_scope(handle->env(), handle->get_id()); + AsyncHooks::InitScope init_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 51bad94a4f..d753dea318 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -55,7 +55,7 @@ int StreamBase::Shutdown(const FunctionCallbackInfo& args) { AsyncWrap* wrap = GetAsyncWrap(); CHECK_NE(wrap, nullptr); - env->set_init_trigger_id(wrap->get_id()); + env->set_init_trigger_async_id(wrap->get_async_id()); ShutdownWrap* req_wrap = new ShutdownWrap(env, req_wrap_obj, this, @@ -160,7 +160,7 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { wrap = GetAsyncWrap(); CHECK_NE(wrap, nullptr); - env->set_init_trigger_id(wrap->get_id()); + env->set_init_trigger_async_id(wrap->get_async_id()); req_wrap = WriteWrap::New(env, req_wrap_obj, this, AfterWrite, storage_size); offset = 0; @@ -249,7 +249,7 @@ int StreamBase::WriteBuffer(const FunctionCallbackInfo& args) { wrap = GetAsyncWrap(); if (wrap != nullptr) - env->set_init_trigger_id(wrap->get_id()); + env->set_init_trigger_async_id(wrap->get_async_id()); // Allocate, or write rest req_wrap = WriteWrap::New(env, req_wrap_obj, this, AfterWrite); @@ -334,7 +334,7 @@ int StreamBase::WriteString(const FunctionCallbackInfo& args) { wrap = GetAsyncWrap(); if (wrap != nullptr) - env->set_init_trigger_id(wrap->get_id()); + env->set_init_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 5824c85247..7bda85f622 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -55,7 +55,7 @@ using AsyncHooks = Environment::AsyncHooks; Local TCPWrap::Instantiate(Environment* env, AsyncWrap* parent) { EscapableHandleScope handle_scope(env->isolate()); - AsyncHooks::InitScope init_scope(env, parent->get_id()); + 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); @@ -268,7 +268,7 @@ void TCPWrap::Connect(const FunctionCallbackInfo& args) { int err = uv_ip4_addr(*ip_address, port, &addr); if (err == 0) { - env->set_init_trigger_id(wrap->get_id()); + env->set_init_trigger_async_id(wrap->get_async_id()); ConnectWrap* req_wrap = new ConnectWrap(env, req_wrap_obj, AsyncWrap::PROVIDER_TCPCONNECTWRAP); err = req_wrap->Dispatch(uv_tcp_connect, @@ -303,7 +303,7 @@ void TCPWrap::Connect6(const FunctionCallbackInfo& args) { int err = uv_ip6_addr(*ip_address, port, &addr); if (err == 0) { - env->set_init_trigger_id(wrap->get_id()); + env->set_init_trigger_async_id(wrap->get_async_id()); ConnectWrap* req_wrap = new ConnectWrap(env, req_wrap_obj, AsyncWrap::PROVIDER_TCPCONNECTWRAP); err = req_wrap->Dispatch(uv_tcp_connect, diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index d0276ca79a..efe6059084 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -350,7 +350,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_id(wrap->get_id()); + env->set_init_trigger_async_id(wrap->get_async_id()); SendWrap* req_wrap = new SendWrap(env, req_wrap_obj, have_callback); size_t msg_size = 0; @@ -497,7 +497,7 @@ void UDPWrap::OnRecv(uv_udp_t* handle, Local UDPWrap::Instantiate(Environment* env, AsyncWrap* parent) { EscapableHandleScope scope(env->isolate()); - AsyncHooks::InitScope init_scope(env, parent->get_id()); + AsyncHooks::InitScope init_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() diff --git a/test/common/index.js b/test/common/index.js index 4ef6f330c0..278f9f6a78 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -74,15 +74,15 @@ if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) { util.inspect(initHandles[k]); }); - const _addIdToDestroyList = async_wrap.addIdToDestroyList; - async_wrap.addIdToDestroyList = function addIdToDestroyList(id) { + const _queueDestroyAsyncId = async_wrap.queueDestroyAsyncId; + async_wrap.queueDestroyAsyncId = function queueDestroyAsyncId(id) { if (destroyListList[id] !== undefined) { process._rawDebug(destroyListList[id]); process._rawDebug(); throw new Error(`same id added to destroy list twice (${id})`); } destroyListList[id] = new Error().stack; - _addIdToDestroyList(id); + _queueDestroyAsyncId(id); }; require('async_hooks').createHook({ diff --git a/test/parallel/test-async-wrap-destroyid.js b/test/parallel/test-async-wrap-destroyid.js index 75f8ed9e66..56cc74871b 100644 --- a/test/parallel/test-async-wrap-destroyid.js +++ b/test/parallel/test-async-wrap-destroyid.js @@ -32,6 +32,6 @@ hooks = async_hooks.createHook({ if (n <= 0) return; test_id = (Math.random() * 1e9) >>> 0; - async_wrap.addIdToDestroyList(test_id); + async_wrap.queueDestroyAsyncId(test_id); setImmediate(common.mustCall(runner), n - 1); })(RUNS); From 8f6867d8cc0de63f465093a895d7bbf7d830ee4c Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Wed, 14 Jun 2017 01:27:02 -0600 Subject: [PATCH 2/3] async_wrap: allow user to pass execution_async_id Allow the user to pass in an execution_async_id instead of always generating one. This way the JS API can be used to pre-allocate the execution_async_id when the JS object is instantiated, before the native resource is created. Also allow the new execution_async_id to be passed via asyncReset(). PR-URL: https://github.com/nodejs/node/pull/14208 Reviewed-By: James M Snell Reviewed-By: Refael Ackermann --- src/async-wrap.cc | 15 +++++++++------ src/async-wrap.h | 5 +++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/async-wrap.cc b/src/async-wrap.cc index 7ca3f6c604..913ff9e7c8 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -455,7 +455,8 @@ void AsyncWrap::ClearAsyncIdStack(const FunctionCallbackInfo& args) { void AsyncWrap::AsyncReset(const FunctionCallbackInfo& args) { AsyncWrap* wrap; ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder()); - wrap->AsyncReset(); + double execution_async_id = args[0]->IsNumber() ? args[0]->NumberValue() : -1; + wrap->AsyncReset(execution_async_id); } @@ -577,7 +578,8 @@ void LoadAsyncWrapperInfo(Environment* env) { AsyncWrap::AsyncWrap(Environment* env, Local object, - ProviderType provider) + ProviderType provider, + double execution_async_id) : BaseObject(env, object), provider_type_(provider) { CHECK_NE(provider, PROVIDER_NONE); @@ -587,7 +589,7 @@ AsyncWrap::AsyncWrap(Environment* env, persistent().SetWrapperClassId(NODE_ASYNC_ID_OFFSET + provider); // Use AsyncReset() call to execute the init() callbacks. - AsyncReset(); + AsyncReset(execution_async_id); } @@ -603,7 +605,7 @@ AsyncWrap::AsyncWrap(Environment* env, persistent().SetWrapperClassId(NODE_ASYNC_ID_OFFSET + provider_type_); // Use AsyncReset() call to execute the init() callbacks. - AsyncReset(silent); + AsyncReset(-1, silent); } @@ -615,8 +617,9 @@ AsyncWrap::~AsyncWrap() { // Generalized call for both the constructor and for handles that are pooled // and reused over their lifetime. This way a new uid can be assigned when // the resource is pulled out of the pool and put back into use. -void AsyncWrap::AsyncReset(bool silent) { - async_id_ = env()->new_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(); if (silent) return; diff --git a/src/async-wrap.h b/src/async-wrap.h index 0e0415da17..d24eb0d46d 100644 --- a/src/async-wrap.h +++ b/src/async-wrap.h @@ -94,7 +94,8 @@ class AsyncWrap : public BaseObject { AsyncWrap(Environment* env, v8::Local object, - ProviderType provider); + ProviderType provider, + double execution_async_id = -1); virtual ~AsyncWrap(); @@ -132,7 +133,7 @@ class AsyncWrap : public BaseObject { inline double get_trigger_async_id() const; - void AsyncReset(bool silent = false); + void AsyncReset(double execution_async_id = -1, bool silent = false); // Only call these within a valid HandleScope. v8::MaybeLocal MakeCallback(const v8::Local cb, From e82107f31e1a1938159c640ef07303be893dbfa4 Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Fri, 22 Sep 2017 01:49:49 -0600 Subject: [PATCH 3/3] src: clear async id stack if bootstrap throws If bootstrap throws and if ids are added to the async id stack and if the exception wasn't handled by the fatal exception handler then the AsyncCallbackScope destructor will cause the AsyncHooks::pop_ids() stack check to fail. Causing the application to crash. So clear the async id stack manually. This is only possible if the user: 1) manually calls MakeCallback() or 2) uses async await in the top level. Which will cause _tickCallback() to fire before bootstrap finishes executing. The following example shows how the application can fail due to exceeding the maximum call stack while using async await: async function fn() { fn(); throw new Error(); } (async function() { await fn(); })(); If this occurs during bootstrap then the application will pring the following warning a number of times then exit with a status of 0: (node:*) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: *): Error Here's the same recursive call done after enabling a new AsyncHook() the following will print instead of the above warning and exit with a non-zero code (currently it's 7 because of how node::FatalException assigns error codes based on where the failure happened): script.js:25 async function fn() { ^ RangeError: Maximum call stack size exceeded at at fn (script.js:25:18) at fn (script.js:26:3) .... This has to do with how Promises lazily enable PromiseHook if an AsyncHook() is enabled. Whether these need to be made uniform is outside the scope of this commit Fixes: https://github.com/nodejs/node/issues/15448 PR-URL: https://github.com/nodejs/node/pull/15553 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Refael Ackermann --- src/node.cc | 11 +++++++++- .../test-async-wrap-pop-id-during-load.js | 21 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-async-wrap-pop-id-during-load.js diff --git a/src/node.cc b/src/node.cc index 80b6e37a88..d52dbcb7e1 100644 --- a/src/node.cc +++ b/src/node.cc @@ -3787,7 +3787,16 @@ void LoadEnvironment(Environment* env) { // who do not like how bootstrap_node.js sets up the module system but do // like Node's I/O bindings may want to replace 'f' with their own function. Local arg = env->process_object(); - f->Call(Null(env->isolate()), 1, &arg); + auto ret = f->Call(env->context(), Null(env->isolate()), 1, &arg); + // If there was an error during bootstrap then it was either handled by the + // FatalException handler or it's unrecoverable (e.g. max call stack + // exceeded). Either way, clear the stack so that the AsyncCallbackScope + // destructor doesn't fail on the id check. + // There are only two ways to have a stack size > 1: 1) the user manually + // called MakeCallback or 2) user awaited during bootstrap, which triggered + // _tickCallback(). + if (ret.IsEmpty()) + env->async_hooks()->clear_async_id_stack(); } static void PrintHelp() { diff --git a/test/parallel/test-async-wrap-pop-id-during-load.js b/test/parallel/test-async-wrap-pop-id-during-load.js new file mode 100644 index 0000000000..1017fc02a7 --- /dev/null +++ b/test/parallel/test-async-wrap-pop-id-during-load.js @@ -0,0 +1,21 @@ +'use strict'; + +require('../common'); + +if (process.argv[2] === 'async') { + async function fn() { + fn(); + throw new Error(); + } + (async function() { await fn(); })(); + // While the above should error, just in case it dosn't the script shouldn't + // fork itself indefinitely so return early. + return; +} + +const assert = require('assert'); +const { spawnSync } = require('child_process'); + +const ret = spawnSync(process.execPath, [__filename, 'async']); +assert.strictEqual(ret.status, 0); +assert.ok(!/async.*hook/i.test(ret.stderr.toString('utf8', 0, 1024)));