Skip to content

Commit

Permalink
async_wrap: fix Promises with later enabled hooks
Browse files Browse the repository at this point in the history
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 28, 2017
1 parent 4a8ea63 commit a6023ee
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 10 deletions.
20 changes: 12 additions & 8 deletions src/async-wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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); }
};

Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}


Expand All @@ -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_);
Expand Down
5 changes: 3 additions & 2 deletions src/async-wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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.
Expand Down
34 changes: 34 additions & 0 deletions test/parallel/test-async-wrap-promise-after-enabled.js
Original file line number Diff line number Diff line change
@@ -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.