Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

async_hooks: support promise resolve hook #15296

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 37 additions & 1 deletion doc/api/async_hooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ const eid = async_hooks.executionAsyncId();
const tid = async_hooks.triggerAsyncId();

// Create a new AsyncHook instance. All of these callbacks are optional.
const asyncHook = async_hooks.createHook({ init, before, after, destroy });
const asyncHook =
async_hooks.createHook({ init, before, after, destroy, promiseResolve });

// Allow callbacks of this AsyncHook instance to call. This is not an implicit
// action after running the constructor, and must be explicitly run to begin
Expand Down Expand Up @@ -65,6 +66,11 @@ function after(asyncId) { }

// destroy is called when an AsyncWrap instance is destroyed.
function destroy(asyncId) { }

// promiseResolve is called only for promise resources, when the
// `resolve` function passed to the `Promise` constructor is invoked
// (either directly or through other means of resolving a promise).
function promiseResolve(asyncId) { }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be called before or after the resolve function is invoked? The distinction may be important for measuring timing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasnell I’m pretty sure that’s actually irrelevant, since the resolve method doesn’t actually do any synchronously visible work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also likely include some clarification about the relationship (if any) this has to the before and after emits. An example that shows what kind of ordering of these events to expect when doing a new Promise((resolve) => resolve(true)).then((a) => {}) would be worthwhile.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@addaleax

I’m pretty sure that’s actually irrelevant, since the resolve method doesn’t actually do any synchronously visible work.

Then I would be explicit about that in the docs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve added a note about that and added an example

```

#### `async_hooks.createHook(callbacks)`
Expand Down Expand Up @@ -397,6 +403,36 @@ the `resource` object passed to `init` it's possible that `destroy` is
never called, causing a memory leak in the application. Of course if
the resource doesn't depend on GC then this isn't an issue.

##### `promiseResolve(asyncId)`

* `asyncId` {number}

Called when the `resolve` function passed to the `Promise` constructor is
invoked (either directly or through other means of resolving a promise).

Note that `resolve()` does not do any observable synchronous work.

*Note:* This does not necessarily mean that the `Promise` is fulfilled or
rejected at this point, if the `Promise` was resolved by assuming the state
of another `Promise`.

For example:

```js
new Promise((resolve) => resolve(true)).then((a) => {});
```

calls the following callbacks:

```js
init for PROMISE with id 5, trigger id: 1
promise resolve 5 # corresponds to resolve(true)
init for PROMISE with id 6, trigger id: 5 # the Promise returned by then()
before 6 # the then() callback is entered
promise resolve 6 # the then() callback resolves the promise by returning
after 6
```

#### `async_hooks.executionAsyncId()`

* Returns {number} the `asyncId` of the current execution context. Useful to track
Expand Down
21 changes: 17 additions & 4 deletions lib/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ 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, kTotals, kCurrentAsyncId,
kCurrentTriggerId, kAsyncUidCntr,
const { kInit, kBefore, kAfter, kDestroy, kPromiseResolve, kTotals,
kCurrentAsyncId, kCurrentTriggerId, kAsyncUidCntr,
kInitTriggerId } = async_wrap.constants;

// Symbols used to store the respective ids on both AsyncResource instances and
Expand All @@ -72,9 +72,12 @@ const init_symbol = Symbol('init');
const before_symbol = Symbol('before');
const after_symbol = Symbol('after');
const destroy_symbol = Symbol('destroy');
const promise_resolve_symbol = Symbol('promiseResolve');
const emitBeforeNative = emitHookFactory(before_symbol, 'emitBeforeNative');
const emitAfterNative = emitHookFactory(after_symbol, 'emitAfterNative');
const emitDestroyNative = emitHookFactory(destroy_symbol, 'emitDestroyNative');
const emitPromiseResolveNative =
emitHookFactory(promise_resolve_symbol, 'emitPromiseResolveNative');

// TODO(refack): move to node-config.cc
const abort_regex = /^--abort[_-]on[_-]uncaught[_-]exception$/;
Expand All @@ -86,7 +89,8 @@ const abort_regex = /^--abort[_-]on[_-]uncaught[_-]exception$/;
async_wrap.setupHooks({ init: emitInitNative,
before: emitBeforeNative,
after: emitAfterNative,
destroy: emitDestroyNative });
destroy: emitDestroyNative,
promise_resolve: emitPromiseResolveNative });

// Used to fatally abort the process if a callback throws.
function fatalError(e) {
Expand All @@ -107,7 +111,7 @@ function fatalError(e) {
// Public API //

class AsyncHook {
constructor({ init, before, after, destroy }) {
constructor({ init, before, after, destroy, promiseResolve }) {
if (init !== undefined && typeof init !== 'function')
throw new errors.TypeError('ERR_ASYNC_CALLBACK', 'init');
if (before !== undefined && typeof before !== 'function')
Expand All @@ -116,11 +120,14 @@ class AsyncHook {
throw new errors.TypeError('ERR_ASYNC_CALLBACK', 'before');
if (destroy !== undefined && typeof destroy !== 'function')
throw new errors.TypeError('ERR_ASYNC_CALLBACK', 'before');
if (promiseResolve !== undefined && typeof promiseResolve !== 'function')
throw new errors.TypeError('ERR_ASYNC_CALLBACK', 'promiseResolve');

this[init_symbol] = init;
this[before_symbol] = before;
this[after_symbol] = after;
this[destroy_symbol] = destroy;
this[promise_resolve_symbol] = promiseResolve;
}

enable() {
Expand All @@ -144,6 +151,8 @@ class AsyncHook {
hook_fields[kTotals] += hook_fields[kBefore] += +!!this[before_symbol];
hook_fields[kTotals] += hook_fields[kAfter] += +!!this[after_symbol];
hook_fields[kTotals] += hook_fields[kDestroy] += +!!this[destroy_symbol];
hook_fields[kTotals] +=
hook_fields[kPromiseResolve] += +!!this[promise_resolve_symbol];
hooks_array.push(this);

if (prev_kTotals === 0 && hook_fields[kTotals] > 0)
Expand All @@ -166,6 +175,8 @@ class AsyncHook {
hook_fields[kTotals] += hook_fields[kBefore] -= +!!this[before_symbol];
hook_fields[kTotals] += hook_fields[kAfter] -= +!!this[after_symbol];
hook_fields[kTotals] += hook_fields[kDestroy] -= +!!this[destroy_symbol];
hook_fields[kTotals] +=
hook_fields[kPromiseResolve] -= +!!this[promise_resolve_symbol];
hooks_array.splice(index, 1);

if (prev_kTotals > 0 && hook_fields[kTotals] === 0)
Expand Down Expand Up @@ -198,6 +209,7 @@ function storeActiveHooks() {
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];
active_hooks.tmp_fields[kPromiseResolve] = async_hook_fields[kPromiseResolve];
}


Expand All @@ -209,6 +221,7 @@ function restoreActiveHooks() {
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];
async_hook_fields[kPromiseResolve] = active_hooks.tmp_fields[kPromiseResolve];

active_hooks.tmp_array = null;
active_hooks.tmp_fields = null;
Expand Down
26 changes: 24 additions & 2 deletions src/async-wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,25 @@ static void PushBackDestroyId(Environment* env, double id) {
}


void AsyncWrap::EmitPromiseResolve(Environment* env, double async_id) {
AsyncHooks* async_hooks = env->async_hooks();

if (async_hooks->fields()[AsyncHooks::kPromiseResolve] == 0)
return;

Local<Value> uid = Number::New(env->isolate(), async_id);
Local<Function> fn = env->async_hooks_promise_resolve_function();
TryCatch try_catch(env->isolate());
MaybeLocal<Value> ar = fn->Call(
env->context(), Undefined(env->isolate()), 1, &uid);
if (ar.IsEmpty()) {
ClearFatalExceptionHandlers(env);
FatalException(env->isolate(), try_catch);
UNREACHABLE();
}
}


void AsyncWrap::EmitBefore(Environment* env, double async_id) {
AsyncHooks* async_hooks = env->async_hooks();

Expand Down Expand Up @@ -303,8 +322,6 @@ static void PromiseHook(PromiseHookType type, Local<Promise> promise,
}

wrap = PromiseWrap::New(env, promise, parent_wrap, silent);
} else if (type == PromiseHookType::kResolve) {
// TODO(matthewloring): need to expose this through the async hooks api.
}

CHECK_NE(wrap, nullptr);
Expand All @@ -321,6 +338,8 @@ static void PromiseHook(PromiseHookType type, Local<Promise> promise,
// PromiseHookType::kBefore that was not witnessed by the PromiseHook.
env->async_hooks()->pop_ids(wrap->get_id());
}
} else if (type == PromiseHookType::kResolve) {
AsyncWrap::EmitPromiseResolve(wrap->env(), wrap->get_id());
}
}

Expand Down Expand Up @@ -349,6 +368,7 @@ static void SetupHooks(const FunctionCallbackInfo<Value>& args) {
SET_HOOK_FN(before);
SET_HOOK_FN(after);
SET_HOOK_FN(destroy);
SET_HOOK_FN(promise_resolve);
#undef SET_HOOK_FN

{
Expand Down Expand Up @@ -500,6 +520,7 @@ void AsyncWrap::Initialize(Local<Object> target,
SET_HOOKS_CONSTANT(kBefore);
SET_HOOKS_CONSTANT(kAfter);
SET_HOOKS_CONSTANT(kDestroy);
SET_HOOKS_CONSTANT(kPromiseResolve);
SET_HOOKS_CONSTANT(kTotals);
SET_HOOKS_CONSTANT(kCurrentAsyncId);
SET_HOOKS_CONSTANT(kCurrentTriggerId);
Expand Down Expand Up @@ -533,6 +554,7 @@ void AsyncWrap::Initialize(Local<Object> target,
env->set_async_hooks_before_function(Local<Function>());
env->set_async_hooks_after_function(Local<Function>());
env->set_async_hooks_destroy_function(Local<Function>());
env->set_async_hooks_promise_resolve_function(Local<Function>());
}


Expand Down
1 change: 1 addition & 0 deletions src/async-wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ class AsyncWrap : public BaseObject {

static void EmitBefore(Environment* env, double id);
static void EmitAfter(Environment* env, double id);
static void EmitPromiseResolve(Environment* env, double id);

inline ProviderType provider_type() const;

Expand Down
2 changes: 2 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ struct performance_state;
V(async_hooks_init_function, v8::Function) \
V(async_hooks_before_function, v8::Function) \
V(async_hooks_after_function, v8::Function) \
V(async_hooks_promise_resolve_function, v8::Function) \
V(binding_cache_object, v8::Object) \
V(buffer_prototype_object, v8::Object) \
V(context, v8::Context) \
Expand Down Expand Up @@ -378,6 +379,7 @@ class Environment {
kBefore,
kAfter,
kDestroy,
kPromiseResolve,
kTotals,
kFieldsCount,
};
Expand Down
5 changes: 5 additions & 0 deletions test/parallel/test-async-hooks-promise.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,16 @@ const assert = require('assert');
const async_hooks = require('async_hooks');

const initCalls = [];
const resolveCalls = [];

async_hooks.createHook({
init: common.mustCall((id, type, triggerId, resource) => {
assert.strictEqual(type, 'PROMISE');
initCalls.push({ id, triggerId, resource });
}, 2),
promiseResolve: common.mustCall((id) => {
assert.strictEqual(initCalls[resolveCalls.length].id, id);
resolveCalls.push(id);
}, 2)
}).enable();

Expand Down