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: fix Promises with later enabled hooks #13242

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
async_hooks: 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
  • Loading branch information
addaleax committed May 27, 2017
commit 029b68fd5fcae5b6df97773d131c60542d0dff9b
7 changes: 4 additions & 3 deletions src/async-wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
wrap = new PromiseWrap(env, promise);
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
30 changes: 30 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,30 @@
'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({
before: common.mustCall((id) => {
assert.ok(id > 1);
seenEvents.push('before');
}),

after: common.mustCall((id) => {
assert.ok(id > 1);
seenEvents.push('after');
hooks.disable();
})
}).enable();

setImmediate(() => {
assert.deepStrictEqual(seenEvents, ['before', 'then', 'after']);
});