From 7e3a3c962f09233c53cee7ebe381341d7c8b7162 Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Thu, 9 Mar 2017 16:13:34 -0700 Subject: [PATCH] async_hooks: initial async_hooks implementation Fill this commit messsage with more details about the change once all changes are rebased. * Add lib/async_hooks.js * Add JS methods to AsyncWrap for handling the async id stack * Introduce AsyncReset() so that JS functions can reset the id and again trigger the init hooks, allow AsyncWrap::Reset() to be called from JS via asyncReset(). * Add env variable to test additional things in test/common.js PR-URL: https://github.com/nodejs/node/pull/12892 Ref: https://github.com/nodejs/node/pull/11883 Ref: https://github.com/nodejs/node/pull/8531 Reviewed-By: Andreas Madsen Reviewed-By: Anna Henningsen Reviewed-By: Sam Roberts Reviewed-By: Matteo Collina Reviewed-By: Refael Ackermann Reviewed-By: James M Snell Reviewed-By: Jeremiah Senkpiel --- lib/async_hooks.js | 488 ++++++++++++++++++++ lib/internal/module.js | 8 +- node.gyp | 1 + src/async-wrap.cc | 124 +++-- src/async-wrap.h | 9 +- src/node_http_parser.cc | 2 +- src/tcp_wrap.cc | 1 + test/common/index.js | 59 +++ test/parallel/test-async-wrap-destroyid.js | 37 ++ test/parallel/test-async-wrap-getasyncid.js | 21 +- 10 files changed, 699 insertions(+), 51 deletions(-) create mode 100644 lib/async_hooks.js create mode 100644 test/parallel/test-async-wrap-destroyid.js diff --git a/lib/async_hooks.js b/lib/async_hooks.js new file mode 100644 index 00000000000000..736b189097672c --- /dev/null +++ b/lib/async_hooks.js @@ -0,0 +1,488 @@ +'use strict'; + +const async_wrap = process.binding('async_wrap'); +/* Both these arrays are used to communicate between JS and C++ with as little + * overhead as possible. + * + * async_hook_fields is a Uint32Array() that communicates the number of each + * type of active hooks of each type and wraps the uin32_t array of + * node::Environment::AsyncHooks::fields_. + * + * async_uid_fields is a Float64Array() that contains the async/trigger ids for + * several operations. These fields are as follows: + * kCurrentAsyncId: The async id of the current execution stack. + * kCurrentTriggerId: The trigger id of the current execution stack. + * kAsyncUidCntr: Counter that tracks the unique ids given to new resources. + * kInitTriggerId: Written to just before creating a new resource, so the + * constructor knows what other resource is responsible for its init(). + * Used this way so the trigger id doesn't need to be passed to every + * resource's constructor. + */ +const { async_hook_fields, async_uid_fields } = async_wrap; +// Used to change the state of the async id stack. +const { pushAsyncIds, popAsyncIds } = async_wrap; +// Array of all AsyncHooks that will be iterated whenever an async event fires. +// Using var instead of (preferably const) in order to assign +// tmp_active_hooks_array if a hook is enabled/disabled during hook execution. +var active_hooks_array = []; +// Track whether a hook callback is currently being processed. Used to make +// sure active_hooks_array isn't altered in mid execution if another hook is +// added or removed. +var processing_hook = false; +// Use to temporarily store and updated active_hooks_array if the user enables +// or disables a hook while hooks are being processed. +var tmp_active_hooks_array = null; +// Keep track of the field counds held in tmp_active_hooks_array. +var tmp_async_hook_fields = null; + +// Each constant tracks how many callbacks there are for any given step of +// async execution. These are tracked so if the user didn't include callbacks +// for a given step, that step can bail out early. +const { kInit, kBefore, kAfter, kDestroy, kCurrentAsyncId, kCurrentTriggerId, + kAsyncUidCntr, kInitTriggerId } = async_wrap.constants; + +// Used in AsyncHook and AsyncEvent. +const async_id_symbol = Symbol('_asyncId'); +const trigger_id_symbol = Symbol('_triggerId'); +const init_symbol = Symbol('init'); +const before_symbol = Symbol('before'); +const after_symbol = Symbol('after'); +const destroy_symbol = Symbol('destroy'); + +// Setup the callbacks that node::AsyncWrap will call when there are hooks to +// process. They use the same functions as the JS embedder API. +async_wrap.setupHooks({ init, + before: emitBeforeN, + after: emitAfterN, + destroy: emitDestroyN }); + +// Used to fatally abort the process if a callback throws. +function fatalError(e) { + if (typeof e.stack === 'string') { + process._rawDebug(e.stack); + } else { + const o = { message: e }; + Error.captureStackTrace(o, fatalError); + process._rawDebug(o.stack); + } + if (process.execArgv.some( + (e) => /^--abort[_-]on[_-]uncaught[_-]exception$/.test(e))) { + process.abort(); + } + process.exit(1); +} + + +// Public API // + +class AsyncHook { + constructor({ init, before, after, destroy }) { + if (init && typeof init !== 'function') + throw new TypeError('init must be a function'); + if (before && typeof before !== 'function') + throw new TypeError('before must be a function'); + if (after && typeof after !== 'function') + throw new TypeError('after must be a function'); + if (destroy && typeof destroy !== 'function') + throw new TypeError('destroy must be a function'); + + this[init_symbol] = init; + this[before_symbol] = before; + this[after_symbol] = after; + this[destroy_symbol] = destroy; + } + + enable() { + // The set of callbacks for a hook should be the same regardless of whether + // enable()/disable() are run during their execution. The following + // references are reassigned to the tmp arrays if a hook is currently being + // processed. + const [hooks_array, hook_fields] = getHookArrays(); + + // Each hook is only allowed to be added once. + if (hooks_array.includes(this)) + return; + + // createHook() has already enforced that the callbacks are all functions, + // so here simply increment the count of whether each callbacks exists or + // not. + hook_fields[kInit] += +!!this[init_symbol]; + hook_fields[kBefore] += +!!this[before_symbol]; + hook_fields[kAfter] += +!!this[after_symbol]; + hook_fields[kDestroy] += +!!this[destroy_symbol]; + hooks_array.push(this); + return this; + } + + disable() { + const [hooks_array, hook_fields] = getHookArrays(); + + const index = hooks_array.indexOf(this); + if (index === -1) + return; + + hook_fields[kInit] -= +!!this[init_symbol]; + hook_fields[kBefore] -= +!!this[before_symbol]; + hook_fields[kAfter] -= +!!this[after_symbol]; + hook_fields[kDestroy] -= +!!this[destroy_symbol]; + hooks_array.splice(index, 1); + return this; + } +} + + +function getHookArrays() { + if (!processing_hook) + return [active_hooks_array, async_hook_fields]; + // If this hook is being enabled while in the middle of processing the array + // of currently active hooks then duplicate the current set of active hooks + // and store this there. This shouldn't fire until the next time hooks are + // processed. + if (tmp_active_hooks_array === null) + storeActiveHooks(); + return [tmp_active_hooks_array, tmp_async_hook_fields]; +} + + +function storeActiveHooks() { + tmp_active_hooks_array = active_hooks_array.slice(); + // Don't want to make the assumption that kInit to kDestroy are indexes 0 to + // 4. So do this the long way. + tmp_async_hook_fields = []; + tmp_async_hook_fields[kInit] = async_hook_fields[kInit]; + tmp_async_hook_fields[kBefore] = async_hook_fields[kBefore]; + tmp_async_hook_fields[kAfter] = async_hook_fields[kAfter]; + tmp_async_hook_fields[kDestroy] = async_hook_fields[kDestroy]; +} + + +// Then restore the correct hooks array in case any hooks were added/removed +// during hook callback execution. +function restoreTmpHooks() { + active_hooks_array = tmp_active_hooks_array; + async_hook_fields[kInit] = tmp_async_hook_fields[kInit]; + async_hook_fields[kBefore] = tmp_async_hook_fields[kBefore]; + async_hook_fields[kAfter] = tmp_async_hook_fields[kAfter]; + async_hook_fields[kDestroy] = tmp_async_hook_fields[kDestroy]; + + tmp_active_hooks_array = null; + tmp_async_hook_fields = null; +} + + +function createHook(fns) { + return new AsyncHook(fns); +} + + +function currentId() { + return async_uid_fields[kCurrentAsyncId]; +} + + +function triggerId() { + return async_uid_fields[kCurrentTriggerId]; +} + + +// Embedder API // + +class AsyncEvent { + constructor(type, triggerId) { + this[async_id_symbol] = ++async_uid_fields[kAsyncUidCntr]; + // Read and reset the current kInitTriggerId so that when the constructor + // finishes the kInitTriggerId field is always 0. + if (triggerId === undefined) { + triggerId = initTriggerId(); + // If a triggerId was passed, any kInitTriggerId still must be null'd. + } else { + async_uid_fields[kInitTriggerId] = 0; + } + this[trigger_id_symbol] = triggerId; + + // Return immediately if there's nothing to do. + if (async_hook_fields[kInit] === 0) + return; + + if (typeof type !== 'string' || type.length <= 0) + throw new TypeError('type must be a string with length > 0'); + if (!Number.isSafeInteger(triggerId) || triggerId < 0) + throw new RangeError('triggerId must be an unsigned integer'); + + processing_hook = true; + for (var i = 0; i < active_hooks_array.length; i++) { + if (typeof active_hooks_array[i][init_symbol] === 'function') { + runInitCallback(active_hooks_array[i][init_symbol], + this[async_id_symbol], + type, + triggerId, + this); + } + } + processing_hook = false; + } + + emitBefore() { + emitBeforeS(this[async_id_symbol], this[trigger_id_symbol]); + return this; + } + + emitAfter() { + emitAfterS(this[async_id_symbol]); + return this; + } + + emitDestroy() { + emitDestroyS(this[async_id_symbol]); + return this; + } + + asyncId() { + return this[async_id_symbol]; + } + + triggerId() { + return this[trigger_id_symbol]; + } +} + + +function runInAsyncIdScope(asyncId, cb) { + // Store the async id now to make sure the stack is still good when the ids + // are popped off the stack. + const prevId = currentId(); + pushAsyncIds(asyncId, prevId); + try { + cb(); + } finally { + popAsyncIds(asyncId); + } +} + + +// Sensitive Embedder API // + +// Increment the internal id counter and return the value. Important that the +// counter increment first. Since it's done the same way in +// Environment::new_async_uid() +function newUid() { + return ++async_uid_fields[kAsyncUidCntr]; +} + + +// Return the triggerId meant for the constructor calling it. It's up to the +// user to safeguard this call and make sure it's zero'd out when the +// constructor is complete. +function initTriggerId() { + var tId = async_uid_fields[kInitTriggerId]; + // 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; +} + + +function setInitTriggerId(triggerId) { + // CHECK(Number.isSafeInteger(triggerId)) + // CHECK(triggerId > 0) + async_uid_fields[kInitTriggerId] = triggerId; +} + + +function emitInitS(asyncId, type, triggerId, resource) { + // Short circuit all checks for the common case. Which is that no hooks have + // been set. Do this to remove performance impact for embedders (and core). + // Even though it bypasses all the argument checks. The performance savings + // here is critical. + if (async_hook_fields[kInit] === 0) + return; + + // This can run after the early return check b/c running this function + // manually means that the embedder must have used initTriggerId(). + if (!Number.isSafeInteger(triggerId)) { + if (triggerId !== undefined) + resource = triggerId; + triggerId = initTriggerId(); + } + + // I'd prefer allowing these checks to not exist, or only throw in a debug + // build, in order to improve performance. + if (!Number.isSafeInteger(asyncId) || asyncId < 0) + throw new RangeError('asyncId must be an unsigned integer'); + if (typeof type !== 'string' || type.length <= 0) + throw new TypeError('type must be a string with length > 0'); + if (!Number.isSafeInteger(triggerId) || triggerId < 0) + throw new RangeError('triggerId must be an unsigned integer'); + + processing_hook = true; + for (var i = 0; i < active_hooks_array.length; i++) { + if (typeof active_hooks_array[i][init_symbol] === 'function') { + runInitCallback( + active_hooks_array[i][init_symbol], asyncId, type, triggerId, resource); + } + } + processing_hook = false; + + // Isn't null if hooks were added/removed while the hooks were running. + if (tmp_active_hooks_array !== null) { + restoreTmpHooks(); + } +} + + +function emitBeforeN(asyncId) { + processing_hook = true; + for (var i = 0; i < active_hooks_array.length; i++) { + if (typeof active_hooks_array[i][before_symbol] === 'function') { + runCallback(active_hooks_array[i][before_symbol], asyncId); + } + } + processing_hook = false; + + if (tmp_active_hooks_array !== null) { + restoreTmpHooks(); + } +} + + +// Usage: emitBeforeS(asyncId[, triggerId]). If triggerId is omitted then +// asyncId will be used instead. +function emitBeforeS(asyncId, triggerId = asyncId) { + // CHECK(Number.isSafeInteger(asyncId) && asyncId > 0) + // CHECK(Number.isSafeInteger(triggerId) && triggerId > 0) + + // Validate the ids. + if (asyncId < 0 || triggerId < 0) { + fatalError('before(): asyncId or triggerId is less than zero ' + + `(asyncId: ${asyncId}, triggerId: ${triggerId})`); + } + + pushAsyncIds(asyncId, triggerId); + + if (async_hook_fields[kBefore] === 0) { + return; + } + + emitBeforeN(asyncId); +} + + +// Called from native. The asyncId stack handling is taken care of there before +// this is called. +function emitAfterN(asyncId) { + if (async_hook_fields[kAfter] > 0) { + processing_hook = true; + for (var i = 0; i < active_hooks_array.length; i++) { + if (typeof active_hooks_array[i][after_symbol] === 'function') { + runCallback(active_hooks_array[i][after_symbol], asyncId); + } + } + processing_hook = false; + } + + if (tmp_active_hooks_array !== null) { + restoreTmpHooks(); + } +} + + +// TODO(trevnorris): Calling emitBefore/emitAfter from native can't adjust the +// kIdStackIndex. But what happens if the user doesn't have both before and +// after callbacks. +function emitAfterS(asyncId) { + emitAfterN(asyncId); + popAsyncIds(asyncId); +} + + +function emitDestroyS(asyncId) { + // Return early if there are no destroy callbacks, or on attempt to emit + // destroy on the void. + if (async_hook_fields[kDestroy] === 0 || asyncId === 0) + return; + async_wrap.addIdToDestroyList(asyncId); +} + + +function emitDestroyN(asyncId) { + processing_hook = true; + for (var i = 0; i < active_hooks_array.length; i++) { + if (typeof active_hooks_array[i][destroy_symbol] === 'function') { + runCallback(active_hooks_array[i][destroy_symbol], asyncId); + } + } + processing_hook = false; + + if (tmp_active_hooks_array !== null) { + restoreTmpHooks(); + } +} + + +// Emit callbacks for native calls. Since some state can be setup directly from +// C++ there's no need to perform all the work here. + +// This should only be called if hooks_array has kInit > 0. There are no global +// values to setup. Though hooks_array will be cloned if C++ needed to call +// init(). +// TODO(trevnorris): Perhaps have MakeCallback call a single JS function that +// does the before/callback/after calls to remove two additional calls to JS. +function init(asyncId, type, resource, triggerId) { + processing_hook = true; + for (var i = 0; i < active_hooks_array.length; i++) { + if (typeof active_hooks_array[i][init_symbol] === 'function') { + runInitCallback( + active_hooks_array[i][init_symbol], asyncId, type, triggerId, resource); + } + } + processing_hook = false; +} + + +// Generalized callers for all callbacks that handles error handling. + +// If either runInitCallback() or runCallback() throw then force the +// application to shutdown if one of the callbacks throws. This may change in +// the future depending on whether it can be determined if there's a slim +// chance of the application remaining stable after handling one of these +// exceptions. + +function runInitCallback(cb, asyncId, type, triggerId, resource) { + try { + cb(asyncId, type, triggerId, resource); + } catch (e) { + fatalError(e); + } +} + + +function runCallback(cb, asyncId) { + try { + cb(asyncId); + } catch (e) { + fatalError(e); + } +} + + +// Placing all exports down here because the exported classes won't export +// otherwise. +module.exports = { + // Public API + createHook, + currentId, + triggerId, + // Embedder API + AsyncEvent, + runInAsyncIdScope, + // Sensitive Embedder API + newUid, + initTriggerId, + setInitTriggerId, + emitInit: emitInitS, + emitBefore: emitBeforeS, + emitAfter: emitAfterS, + emitDestroy: emitDestroyS, +}; diff --git a/lib/internal/module.js b/lib/internal/module.js index 49bd9b84eee9a1..08d8f770c8d873 100644 --- a/lib/internal/module.js +++ b/lib/internal/module.js @@ -77,10 +77,10 @@ function stripShebang(content) { } const builtinLibs = [ - 'assert', 'buffer', 'child_process', 'cluster', 'crypto', 'dgram', 'dns', - 'domain', 'events', 'fs', 'http', 'https', 'net', 'os', 'path', 'punycode', - 'querystring', 'readline', 'repl', 'stream', 'string_decoder', 'tls', 'tty', - 'url', 'util', 'v8', 'vm', 'zlib' + 'assert', 'async_hooks', 'buffer', 'child_process', 'cluster', 'crypto', + 'dgram', 'dns', 'domain', 'events', 'fs', 'http', 'https', 'net', 'os', + 'path', 'punycode', 'querystring', 'readline', 'repl', 'stream', + 'string_decoder', 'tls', 'tty', 'url', 'util', 'v8', 'vm', 'zlib' ]; function addBuiltinLibsToObject(object) { diff --git a/node.gyp b/node.gyp index c20b07a66c6ea4..d7a282134ee286 100644 --- a/node.gyp +++ b/node.gyp @@ -22,6 +22,7 @@ 'node_core_target_name%': 'node', 'library_files': [ 'lib/internal/bootstrap_node.js', + 'lib/async_hooks.js', 'lib/assert.js', 'lib/buffer.js', 'lib/child_process.js', diff --git a/src/async-wrap.cc b/src/async-wrap.cc index ab3bf5aa74912e..6ccccfcb65c201 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -134,6 +134,48 @@ RetainedObjectInfo* WrapperInfo(uint16_t class_id, Local wrapper) { // end RetainedAsyncInfo +static void DestroyIdsCb(uv_idle_t* handle) { + uv_idle_stop(handle); + + Environment* env = Environment::from_destroy_ids_idle_handle(handle); + + HandleScope handle_scope(env->isolate()); + Context::Scope context_scope(env->context()); + Local fn = env->async_hooks_destroy_function(); + + TryCatch try_catch(env->isolate()); + + std::vector destroy_ids_list; + destroy_ids_list.swap(*env->destroy_ids_list()); + for (auto current_id : destroy_ids_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); + MaybeLocal ret = fn->Call( + env->context(), Undefined(env->isolate()), 1, &argv); + + if (ret.IsEmpty()) { + ClearFatalExceptionHandlers(env); + FatalException(env->isolate(), try_catch); + } + } + + env->destroy_ids_list()->clear(); +} + + +static void PushBackDestroyId(Environment* env, double id) { + if (env->async_hooks()->fields()[AsyncHooks::kDestroy] == 0) + return; + + if (env->destroy_ids_list()->empty()) + uv_idle_start(env->destroy_ids_idle_handle(), DestroyIdsCb); + + env->destroy_ids_list()->push_back(id); +} + + static void SetupHooks(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -170,6 +212,42 @@ void AsyncWrap::GetAsyncId(const FunctionCallbackInfo& args) { } +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. + 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); +} + + +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)); +} + + +void AsyncWrap::ClearIdStack(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + env->async_hooks()->clear_id_stack(); +} + + +void AsyncWrap::AsyncReset(const FunctionCallbackInfo& args) { + AsyncWrap* wrap; + ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder()); + wrap->AsyncReset(); +} + + +void AsyncWrap::QueueDestroyId(const FunctionCallbackInfo& args) { + CHECK(args[0]->IsNumber()); + PushBackDestroyId(Environment::GetCurrent(args), args[0]->NumberValue()); +} + + void AsyncWrap::Initialize(Local target, Local unused, Local context) { @@ -178,6 +256,10 @@ void AsyncWrap::Initialize(Local target, HandleScope scope(isolate); env->SetMethod(target, "setupHooks", SetupHooks); + env->SetMethod(target, "pushAsyncIds", PushAsyncIds); + env->SetMethod(target, "popAsyncIds", PopAsyncIds); + env->SetMethod(target, "clearIdStack", ClearIdStack); + env->SetMethod(target, "addIdToDestroyList", QueueDestroyId); v8::PropertyAttribute ReadOnlyDontDelete = static_cast(v8::ReadOnly | v8::DontDelete); @@ -252,37 +334,6 @@ void AsyncWrap::Initialize(Local target, } -void AsyncWrap::DestroyIdsCb(uv_idle_t* handle) { - uv_idle_stop(handle); - - Environment* env = Environment::from_destroy_ids_idle_handle(handle); - - HandleScope handle_scope(env->isolate()); - Context::Scope context_scope(env->context()); - Local fn = env->async_hooks_destroy_function(); - - TryCatch try_catch(env->isolate()); - - std::vector destroy_ids_list; - destroy_ids_list.swap(*env->destroy_ids_list()); - for (auto current_id : destroy_ids_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); - MaybeLocal ret = fn->Call( - env->context(), Undefined(env->isolate()), 1, &argv); - - if (ret.IsEmpty()) { - ClearFatalExceptionHandlers(env); - FatalException(env->isolate(), try_catch); - } - } - - env->destroy_ids_list()->clear(); -} - - void LoadAsyncWrapperInfo(Environment* env) { HeapProfiler* heap_profiler = env->isolate()->GetHeapProfiler(); #define V(PROVIDER) \ @@ -310,21 +361,14 @@ AsyncWrap::AsyncWrap(Environment* env, AsyncWrap::~AsyncWrap() { - if (env()->async_hooks()->fields()[AsyncHooks::kDestroy] == 0) { - return; - } - - if (env()->destroy_ids_list()->empty()) - uv_idle_start(env()->destroy_ids_idle_handle(), DestroyIdsCb); - - env()->destroy_ids_list()->push_back(get_id()); + PushBackDestroyId(env(), get_id()); } // 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::Reset() { +void AsyncWrap::AsyncReset() { AsyncHooks* async_hooks = env()->async_hooks(); async_id_ = env()->new_async_id(); trigger_id_ = env()->get_init_trigger_id(); diff --git a/src/async-wrap.h b/src/async-wrap.h index 1fe0499468b485..8a93e838786297 100644 --- a/src/async-wrap.h +++ b/src/async-wrap.h @@ -85,8 +85,11 @@ class AsyncWrap : public BaseObject { v8::Local context); static void GetAsyncId(const v8::FunctionCallbackInfo& args); - - static void DestroyIdsCb(uv_idle_t* handle); + static void PushAsyncIds(const v8::FunctionCallbackInfo& args); + static void PopAsyncIds(const v8::FunctionCallbackInfo& args); + static void ClearIdStack(const v8::FunctionCallbackInfo& args); + static void AsyncReset(const v8::FunctionCallbackInfo& args); + static void QueueDestroyId(const v8::FunctionCallbackInfo& args); inline ProviderType provider_type() const; @@ -94,7 +97,7 @@ class AsyncWrap : public BaseObject { inline double get_trigger_id() const; - void Reset(); + void AsyncReset(); // Only call these within a valid HandleScope. // TODO(trevnorris): These should return a MaybeLocal. diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index 01e7f6daca0c99..40e106cd46fd3c 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -477,7 +477,7 @@ class Parser : public AsyncWrap { // 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. - parser->Reset(); + parser->AsyncReset(); parser->Init(type); } diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index d1bf4a952a1f7c..4967b407145c1c 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -86,6 +86,7 @@ void TCPWrap::Initialize(Local target, Null(env->isolate())); env->SetProtoMethod(t, "getAsyncId", AsyncWrap::GetAsyncId); + env->SetProtoMethod(t, "asyncReset", AsyncWrap::AsyncReset); env->SetProtoMethod(t, "close", HandleWrap::Close); diff --git a/test/common/index.js b/test/common/index.js index a10adc4d0d674b..482680bd2f6ff3 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -64,6 +64,50 @@ exports.enoughTestCpu = Array.isArray(cpus) && exports.rootDir = exports.isWindows ? 'c:\\' : '/'; exports.buildType = process.config.target_defaults.default_configuration; +// If env var is set then enable async_hook hooks for all tests. +if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) { + const destroydIdsList = {}; + const destroyListList = {}; + const initHandles = {}; + const async_wrap = process.binding('async_wrap'); + + process.on('exit', () => { + // itterate through handles to make sure nothing crashes + for (const k in initHandles) + util.inspect(initHandles[k]); + }); + + const _addIdToDestroyList = async_wrap.addIdToDestroyList; + async_wrap.addIdToDestroyList = function addIdToDestroyList(id) { + if (destroyListList[id] !== undefined) { + process._rawDebug(destroyListList[id]); + process._rawDebug(); + throw new Error(`same id added twice (${id})`); + } + destroyListList[id] = new Error().stack; + _addIdToDestroyList(id); + }; + + require('async_hooks').createHook({ + init(id, ty, tr, h) { + if (initHandles[id]) { + throw new Error(`init called twice for same id (${id})`); + } + initHandles[id] = h; + }, + before() { }, + after() { }, + destroy(id) { + if (destroydIdsList[id] !== undefined) { + process._rawDebug(destroydIdsList[id]); + process._rawDebug(); + throw new Error(`destroy called for same id (${id})`); + } + destroydIdsList[id] = new Error().stack; + }, + }).enable(); +} + function rimrafSync(p) { let st; try { @@ -684,3 +728,18 @@ exports.crashOnUnhandledRejection = function() { process.on('unhandledRejection', (err) => process.nextTick(() => { throw err; })); }; + +exports.getTTYfd = function getTTYfd() { + const tty = require('tty'); + let tty_fd = 0; + if (!tty.isatty(tty_fd)) tty_fd++; + else if (!tty.isatty(tty_fd)) tty_fd++; + else if (!tty.isatty(tty_fd)) tty_fd++; + else try { + tty_fd = require('fs').openSync('/dev/tty'); + } catch (e) { + // There aren't any tty fd's available to use. + return -1; + } + return tty_fd; +}; diff --git a/test/parallel/test-async-wrap-destroyid.js b/test/parallel/test-async-wrap-destroyid.js new file mode 100644 index 00000000000000..75f8ed9e661fe3 --- /dev/null +++ b/test/parallel/test-async-wrap-destroyid.js @@ -0,0 +1,37 @@ +'use strict'; + +const common = require('../common'); +const async_wrap = process.binding('async_wrap'); +const assert = require('assert'); +const async_hooks = require('async_hooks'); +const RUNS = 5; +let test_id = null; +let run_cntr = 0; +let hooks = null; + +process.on('beforeExit', common.mustCall(() => { + process.removeAllListeners('uncaughtException'); + hooks.disable(); + assert.strictEqual(test_id, null); + assert.strictEqual(run_cntr, RUNS); +})); + + +hooks = async_hooks.createHook({ + destroy(id) { + if (id === test_id) { + run_cntr++; + test_id = null; + } + }, +}).enable(); + + +(function runner(n) { + assert.strictEqual(test_id, null); + if (n <= 0) return; + + test_id = (Math.random() * 1e9) >>> 0; + async_wrap.addIdToDestroyList(test_id); + setImmediate(common.mustCall(runner), n - 1); +})(RUNS); diff --git a/test/parallel/test-async-wrap-getasyncid.js b/test/parallel/test-async-wrap-getasyncid.js index 5e2ce3a820381c..98aa3c857a8b18 100644 --- a/test/parallel/test-async-wrap-getasyncid.js +++ b/test/parallel/test-async-wrap-getasyncid.js @@ -218,9 +218,24 @@ if (common.hasCrypto) { { - const tty_wrap = process.binding('tty_wrap'); - if (tty_wrap.isTTY(0)) { - testInitialized(new tty_wrap.TTY(0, false), 'TTY'); + // Do our best to grab a tty fd. + const tty_fd = common.getTTYfd(); + if (tty_fd >= 0) { + const tty_wrap = process.binding('tty_wrap'); + // fd may still be invalid, so guard against it. + const handle = (() => { + try { + return new tty_wrap.TTY(tty_fd, false); + } catch (e) { + return null; + } + })(); + if (handle !== null) + testInitialized(handle, 'TTY'); + else + delete providers.TTYWRAP; + } else { + delete providers.TTYWRAP; } }