Permalink
Browse files

async_wrap: fix Promises with later enabled hooks

Assign a `PromiseWrap` instance to Promises that do not have one
yet when the PromiseHook is being called.

Fixes: #13237
PR-URL: #13242
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Matthew Loring <mattloring@google.com>
  • Loading branch information...
addaleax authored and jasnell committed May 26, 2017
1 parent 4a8ea63 commit a6023ee0b514cf1c3f3b3bab8bb11886d79d2111
Showing with 49 additions and 10 deletions.
  1. +12 −8 src/async-wrap.cc
  2. +3 −2 src/async-wrap.h
  3. +34 −0 test/parallel/test-async-wrap-promise-after-enabled.js
View
@@ -283,8 +283,8 @@ bool AsyncWrap::EmitAfter(Environment* env, double async_id) {
class PromiseWrap : public AsyncWrap {
public:
PromiseWrap(Environment* env, Local<Object> object)
: AsyncWrap(env, object, PROVIDER_PROMISE) {}
PromiseWrap(Environment* env, Local<Object> object, bool silent)
: AsyncWrap(env, object, PROVIDER_PROMISE, silent) {}
size_t self_size() const override { return sizeof(*this); }
};
@@ -293,13 +293,14 @@ static void PromiseHook(PromiseHookType type, Local<Promise> promise,
Local<Value> parent, void* arg) {
Local<Context> context = promise->CreationContext();
Environment* env = Environment::GetCurrent(context);
if (type == PromiseHookType::kInit) {
PromiseWrap* wrap = new PromiseWrap(env, promise);
PromiseWrap* wrap = Unwrap<PromiseWrap>(promise);
if (type == PromiseHookType::kInit || wrap == nullptr) {
bool silent = type != PromiseHookType::kInit;
wrap = new PromiseWrap(env, promise, silent);
wrap->MakeWeak(wrap);
} else if (type == PromiseHookType::kResolve) {
// TODO(matthewloring): need to expose this through the async hooks api.
}
PromiseWrap* wrap = Unwrap<PromiseWrap>(promise);
CHECK_NE(wrap, nullptr);
if (type == PromiseHookType::kBefore) {
PreCallbackExecution(wrap, false);
@@ -491,7 +492,8 @@ void LoadAsyncWrapperInfo(Environment* env) {
AsyncWrap::AsyncWrap(Environment* env,
Local<Object> object,
ProviderType provider)
ProviderType provider,
bool silent)
: BaseObject(env, object),
provider_type_(provider) {
CHECK_NE(provider, PROVIDER_NONE);
@@ -501,7 +503,7 @@ AsyncWrap::AsyncWrap(Environment* env,
persistent().SetWrapperClassId(NODE_ASYNC_ID_OFFSET + provider);
// Use AsyncReset() call to execute the init() callbacks.
AsyncReset();
AsyncReset(silent);
}
@@ -513,10 +515,12 @@ 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() {
void AsyncWrap::AsyncReset(bool silent) {
async_id_ = env()->new_async_id();
trigger_id_ = env()->get_init_trigger_id();
if (silent) return;
EmitAsyncInit(env(), object(),
env()->async_hooks()->provider_string(provider_type()),
async_id_, trigger_id_);
View
@@ -86,7 +86,8 @@ class AsyncWrap : public BaseObject {
AsyncWrap(Environment* env,
v8::Local<v8::Object> object,
ProviderType provider);
ProviderType provider,
bool silent = false);
virtual ~AsyncWrap();
@@ -116,7 +117,7 @@ class AsyncWrap : public BaseObject {
inline double get_trigger_id() const;
void AsyncReset();
void AsyncReset(bool silent = false);
// Only call these within a valid HandleScope.
// TODO(trevnorris): These should return a MaybeLocal.
@@ -0,0 +1,34 @@
'use strict';
// Regression test for https://github.com/nodejs/node/issues/13237
const common = require('../common');
const assert = require('assert');
const async_hooks = require('async_hooks');
const seenEvents = [];
const p = new Promise((resolve) => resolve(1));
p.then(() => seenEvents.push('then'));
const hooks = async_hooks.createHook({
init: common.mustNotCall(),
before: common.mustCall((id) => {
assert.ok(id > 1);
seenEvents.push('before');
}),
after: common.mustCall((id) => {
assert.ok(id > 1);
seenEvents.push('after');
hooks.disable();
})
});
setImmediate(() => {
assert.deepStrictEqual(seenEvents, ['before', 'then', 'after']);
});
hooks.enable(); // After `setImmediate` in order to not catch its init event.

0 comments on commit a6023ee

Please sign in to comment.