Skip to content

Commit

Permalink
async_hooks: improve comments and function names
Browse files Browse the repository at this point in the history 
* Reword several of the comments to be more descriptive.
* Rename functions to better indicate what they are doing.
* Change AsyncHooks::uid_fields_ to be a fixed array instead of a
  pointer.
* Define regex early so line ends before 80 columns.
* Remove obsolete comments.
* Rename AsyncHooks::InitScope::uid_fields_ to uid_fields_ptr_ because
  using the same name as AsyncHooks::uid_fields_ was confusing.
* Place variables that are used to store the new set of hooks if another
  hook is enabled/disabled during hook execution into an object to act
  as a namespace.

PR-URL: #14722
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
  • Loading branch information
@trevnorris @MylesBorins
trevnorris authored and MylesBorins committed Sep 12, 2017
1 parent fadccba commit 3e73ea8
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 97 deletions.
190 changes: 102 additions & 88 deletions lib/async_hooks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,39 +2,58 @@

const internalUtil = require('internal/util');
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 wrapping the uint32_t array of
* Environment::AsyncHooks::fields_[]. Each index tracks the number of active
* hooks for each type.
*
* 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.
* async_uid_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
* 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.
*/
const { async_hook_fields, async_uid_fields } = async_wrap;
// Used to change the state of the async id stack.
// Store the pair executionAsyncId and triggerAsyncId in a std::stack on
// Environment::AsyncHooks::ids_stack_ tracks the resource responsible for the
// current execution stack. This is unwound as each resource exits. In the case
// of a fatal exception this stack is emptied after calling each hook's after()
// callback.
const { pushAsyncIds, popAsyncIds } = async_wrap;
// 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 = [];
// Use a counter to 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. A counter is used to track nested calls.
var processing_hook = 0;
// 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 counts held in tmp_active_hooks_array.
var tmp_async_hook_fields = null;
// For performance reasons, only track Proimses when a hook is enabled.
const { enablePromiseHook, disablePromiseHook } = async_wrap;
// Properties in active_hooks are used to keep track of the set of hooks being
// executed in case another hook is enabled/disabled. The new set of hooks is
// then restored once the active set of hooks is finished executing.
const active_hooks = {
// Array of all AsyncHooks that will be iterated whenever an async event
// fires. Using var instead of (preferably const) in order to assign
// active_hooks.tmp_array if a hook is enabled/disabled during hook
// execution.
array: [],
// Use a counter to track nested calls of async hook callbacks and make sure
// the active_hooks.array isn't altered mid execution.
call_depth: 0,
// Use to temporarily store and updated active_hooks.array if the user
// enables or disables a hook while hooks are being processed. If a hook is
// enabled() or disabled() during hook execution then the current set of
// active hooks is duplicated and set equal to active_hooks.tmp_array. Any
// subsequent changes are on the duplicated array. When all hooks have
// completed executing active_hooks.tmp_array is assigned to
// active_hooks.array.
tmp_array: null,
// Keep track of the field counts held in active_hooks.tmp_array. Because the
// async_hook_fields can't be reassigned, store each uint32 in an array that
// is written back to async_hook_fields when active_hooks.array is restored.
tmp_fields: null
};


// Each constant tracks how many callbacks there are for any given step of
// async execution. These are tracked so if the user didn't include callbacks
Expand All @@ -43,6 +62,9 @@ const { kInit, kBefore, kAfter, kDestroy, kTotals, kCurrentAsyncId,
kCurrentTriggerId, kAsyncUidCntr,
kInitTriggerId } = 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;

// Used in AsyncHook and AsyncResource.
Expand All @@ -54,6 +76,9 @@ const emitBeforeNative = emitHookFactory(before_symbol, 'emitBeforeNative');
const emitAfterNative = emitHookFactory(after_symbol, 'emitAfterNative');
const emitDestroyNative = emitHookFactory(destroy_symbol, 'emitDestroyNative');

// TODO(refack): move to node-config.cc
const abort_regex = /^--abort[_-]on[_-]uncaught[_-]exception$/;

// Setup the callbacks that node::AsyncWrap will call when there are hooks to
// process. They use the same functions as the JS embedder API. These callbacks
// are setup immediately to prevent async_wrap.setupHooks() from being hijacked
Expand All @@ -72,7 +97,7 @@ function fatalError(e) {
Error.captureStackTrace(o, fatalError);
process._rawDebug(o.stack);
}
if (process.execArgv.some((e) => /^--abort[_-]on[_-]uncaught[_-]exception$/.test(e))) {
if (process.execArgv.some((e) => abort_regex.test(e))) {
process.abort();
}
process.exit(1);
Expand Down Expand Up @@ -122,7 +147,7 @@ class AsyncHook {
hooks_array.push(this);

if (prev_kTotals === 0 && hook_fields[kTotals] > 0)
async_wrap.enablePromiseHook();
enablePromiseHook();

return this;
}
Expand All @@ -144,49 +169,49 @@ class AsyncHook {
hooks_array.splice(index, 1);

if (prev_kTotals > 0 && hook_fields[kTotals] === 0)
async_wrap.disablePromiseHook();
disablePromiseHook();

return this;
}
}


function getHookArrays() {
if (processing_hook === 0)
return [active_hooks_array, async_hook_fields];
if (active_hooks.call_depth === 0)
return [active_hooks.array, async_hook_fields];
// If this hook is being enabled while in the middle of processing the array
// of currently active hooks then duplicate the current set of active hooks
// and store this there. This shouldn't fire until the next time hooks are
// processed.
if (tmp_active_hooks_array === null)
if (active_hooks.tmp_array === null)
storeActiveHooks();
return [tmp_active_hooks_array, tmp_async_hook_fields];
return [active_hooks.tmp_array, active_hooks.tmp_fields];
}


function storeActiveHooks() {
tmp_active_hooks_array = active_hooks_array.slice();
active_hooks.tmp_array = active_hooks.array.slice();
// Don't want to make the assumption that kInit to kDestroy are indexes 0 to
// 4. So do this the long way.
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];
active_hooks.tmp_fields = [];
active_hooks.tmp_fields[kInit] = async_hook_fields[kInit];
active_hooks.tmp_fields[kBefore] = async_hook_fields[kBefore];
active_hooks.tmp_fields[kAfter] = async_hook_fields[kAfter];
active_hooks.tmp_fields[kDestroy] = async_hook_fields[kDestroy];
}


// 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 restoreActiveHooks() {
active_hooks.array = active_hooks.tmp_array;
async_hook_fields[kInit] = active_hooks.tmp_fields[kInit];
async_hook_fields[kBefore] = active_hooks.tmp_fields[kBefore];
async_hook_fields[kAfter] = active_hooks.tmp_fields[kAfter];
async_hook_fields[kDestroy] = active_hooks.tmp_fields[kDestroy];

active_hooks.tmp_array = null;
active_hooks.tmp_fields = null;
}


Expand Down Expand Up @@ -334,25 +359,30 @@ function emitHookFactory(symbol, name) {
// before this is called.
// eslint-disable-next-line func-style
const fn = function(asyncId) {
processing_hook += 1;
active_hooks.call_depth += 1;
// Use a single try/catch for all hook to avoid setting up one per
// iteration.
try {
for (var i = 0; i < active_hooks_array.length; i++) {
if (typeof active_hooks_array[i][symbol] === 'function') {
active_hooks_array[i][symbol](asyncId);
for (var i = 0; i < active_hooks.array.length; i++) {
if (typeof active_hooks.array[i][symbol] === 'function') {
active_hooks.array[i][symbol](asyncId);
}
}
} catch (e) {
fatalError(e);
} finally {
processing_hook -= 1;
active_hooks.call_depth -= 1;
}

if (processing_hook === 0 && tmp_active_hooks_array !== null) {
restoreTmpHooks();
// Hooks can only be restored if there have been no recursive hook calls.
// Also the active hooks do not need to be restored if enable()/disable()
// weren't called during hook execution, in which case
// active_hooks.tmp_array will be null.
if (active_hooks.call_depth === 0 && active_hooks.tmp_array !== null) {
restoreActiveHooks();
}
};

// Set the name property of the anonymous function as it looks good in the
// stack trace.
Object.defineProperty(fn, 'name', {
Expand All @@ -379,9 +409,6 @@ function emitBeforeScript(asyncId, triggerAsyncId) {
}


// 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 emitAfterScript(asyncId) {
if (async_hook_fields[kAfter] > 0)
emitAfterNative(asyncId);
Expand All @@ -399,26 +426,16 @@ function emitDestroyScript(asyncId) {
}


// 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.

// 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.
// Used by C++ to call all init() callbacks. Because some state can be setup
// from C++ there's no need to perform all the same operations as in
// emitInitScript.
function emitInitNative(asyncId, type, triggerAsyncId, resource) {
processing_hook += 1;
active_hooks.call_depth += 1;
// Use a single try/catch for all hook to avoid setting up one per iteration.
try {
for (var i = 0; i < active_hooks_array.length; i++) {
if (typeof active_hooks_array[i][init_symbol] === 'function') {
active_hooks_array[i][init_symbol](
for (var i = 0; i < active_hooks.array.length; i++) {
if (typeof active_hooks.array[i][init_symbol] === 'function') {
active_hooks.array[i][init_symbol](
asyncId, type, triggerAsyncId,
resource
);
Expand All @@ -427,18 +444,15 @@ function emitInitNative(asyncId, type, triggerAsyncId, resource) {
} catch (e) {
fatalError(e);
} finally {
processing_hook -= 1;
active_hooks.call_depth -= 1;
}

// * `tmp_active_hooks_array` is null if no hooks were added/removed while
// the hooks were running. In that case no restoration is needed.
// * In the case where another hook was added/removed while the hooks were
// running and a handle was created causing the `init` hooks to fire again,
// then `restoreTmpHooks` should not be called for the nested `hooks`.
// Otherwise `active_hooks_array` can change during execution of the
// `hooks`.
if (processing_hook === 0 && tmp_active_hooks_array !== null) {
restoreTmpHooks();
// Hooks can only be restored if there have been no recursive hook calls.
// Also the active hooks do not need to be restored if enable()/disable()
// weren't called during hook execution, in which case active_hooks.tmp_array
// will be null.
if (active_hooks.call_depth === 0 && active_hooks.tmp_array !== null) {
restoreActiveHooks();
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/env-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,13 +180,13 @@ inline void Environment::AsyncHooks::clear_id_stack() {
inline Environment::AsyncHooks::InitScope::InitScope(
Environment* env, double init_trigger_id)
: env_(env),
uid_fields_(env->async_hooks()->uid_fields()) {
env->async_hooks()->push_ids(uid_fields_[AsyncHooks::kCurrentAsyncId],
uid_fields_ref_(env->async_hooks()->uid_fields()) {
env->async_hooks()->push_ids(uid_fields_ref_[AsyncHooks::kCurrentAsyncId],
init_trigger_id);
}

inline Environment::AsyncHooks::InitScope::~InitScope() {
env_->async_hooks()->pop_ids(uid_fields_[AsyncHooks::kCurrentAsyncId]);
env_->async_hooks()->pop_ids(uid_fields_ref_[AsyncHooks::kCurrentAsyncId]);
}

inline Environment::AsyncHooks::ExecScope::ExecScope(
Expand Down
10 changes: 4 additions & 6 deletions src/env.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ class Environment {

private:
Environment* env_;
double* uid_fields_;
double* uid_fields_ref_;

DISALLOW_COPY_AND_ASSIGN(InitScope);
};
Expand Down Expand Up @@ -443,12 +443,10 @@ class Environment {
v8::Isolate* isolate_;
// Stores the ids of the current execution context stack.
std::stack<struct node_async_ids> ids_stack_;
// Used to communicate state between C++ and JS cheaply. Is placed in an
// Uint32Array() and attached to the async_wrap object.
// Attached to a Uint32Array that tracks the number of active hooks for
// each type.
uint32_t fields_[kFieldsCount];
// Used to communicate ids between C++ and JS cheaply. Placed in a
// Float64Array and attached to the async_wrap object. Using a double only
// gives us 2^53-1 unique ids, but that should be sufficient.
// Attached to a Float64Array that tracks the state of async resources.
double uid_fields_[kUidFieldsCount];

DISALLOW_COPY_AND_ASSIGN(AsyncHooks);
Expand Down

0 comments on commit 3e73ea8

Please sign in to comment.