diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 26451f22b51a34..ece6e06b58e851 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -39,9 +39,12 @@ using v8::Global; using v8::HandleScope; using v8::Integer; using v8::Isolate; +using v8::Just; using v8::Local; +using v8::Maybe; using v8::MaybeLocal; using v8::Name; +using v8::Nothing; using v8::Number; using v8::Object; using v8::ObjectTemplate; @@ -189,6 +192,21 @@ void AsyncWrap::EmitAfter(Environment* env, double async_id) { env->async_hooks_after_function()); } +// TODO(addaleax): Remove once we're on C++17. +constexpr double AsyncWrap::kInvalidAsyncId; + +static Maybe GetAssignedPromiseAsyncId(Environment* env, + Local promise, + Local id_symbol) { + Local maybe_async_id; + if (!promise->Get(env->context(), id_symbol).ToLocal(&maybe_async_id)) { + return Nothing(); + } + return maybe_async_id->IsNumber() + ? maybe_async_id->NumberValue(env->context()) + : Just(AsyncWrap::kInvalidAsyncId); +} + class PromiseWrap : public AsyncWrap { public: PromiseWrap(Environment* env, Local object, bool silent) @@ -231,18 +249,17 @@ PromiseWrap* PromiseWrap::New(Environment* env, // Skip for init events if (silent) { - Local maybe_async_id = promise - ->Get(context, env->async_id_symbol()) - .ToLocalChecked(); - - Local maybe_trigger_async_id = promise - ->Get(context, env->trigger_async_id_symbol()) - .ToLocalChecked(); - - if (maybe_async_id->IsNumber() && maybe_trigger_async_id->IsNumber()) { - double async_id = maybe_async_id.As()->Value(); - double trigger_async_id = maybe_trigger_async_id.As()->Value(); - return new PromiseWrap(env, obj, async_id, trigger_async_id); + double async_id; + double trigger_async_id; + if (!GetAssignedPromiseAsyncId(env, promise, env->async_id_symbol()) + .To(&async_id)) return nullptr; + if (!GetAssignedPromiseAsyncId(env, promise, env->trigger_async_id_symbol()) + .To(&trigger_async_id)) return nullptr; + + if (async_id != AsyncWrap::kInvalidAsyncId && + trigger_async_id != AsyncWrap::kInvalidAsyncId) { + return new PromiseWrap( + env, obj, async_id, trigger_async_id); } } @@ -321,46 +338,35 @@ static void FastPromiseHook(PromiseHookType type, Local promise, if (type == PromiseHookType::kBefore && env->async_hooks()->fields()[AsyncHooks::kBefore] == 0) { - Local maybe_async_id; - if (!promise->Get(context, env->async_id_symbol()) - .ToLocal(&maybe_async_id)) { - return; - } - - Local maybe_trigger_async_id; - if (!promise->Get(context, env->trigger_async_id_symbol()) - .ToLocal(&maybe_trigger_async_id)) { - return; - } - - if (maybe_async_id->IsNumber() && maybe_trigger_async_id->IsNumber()) { - double async_id = maybe_async_id.As()->Value(); - double trigger_async_id = maybe_trigger_async_id.As()->Value(); + double async_id; + double trigger_async_id; + if (!GetAssignedPromiseAsyncId(env, promise, env->async_id_symbol()) + .To(&async_id)) return; + if (!GetAssignedPromiseAsyncId(env, promise, env->trigger_async_id_symbol()) + .To(&trigger_async_id)) return; + + if (async_id != AsyncWrap::kInvalidAsyncId && + trigger_async_id != AsyncWrap::kInvalidAsyncId) { env->async_hooks()->push_async_context( async_id, trigger_async_id, promise); + return; } - - return; } if (type == PromiseHookType::kAfter && env->async_hooks()->fields()[AsyncHooks::kAfter] == 0) { - Local maybe_async_id; - if (!promise->Get(context, env->async_id_symbol()) - .ToLocal(&maybe_async_id)) { - return; - } + double async_id; + if (!GetAssignedPromiseAsyncId(env, promise, env->async_id_symbol()) + .To(&async_id)) return; - if (maybe_async_id->IsNumber()) { - double async_id = maybe_async_id.As()->Value(); + if (async_id != AsyncWrap::kInvalidAsyncId) { if (env->execution_async_id() == async_id) { // This condition might not be true if async_hooks was enabled during // the promise callback execution. env->async_hooks()->pop_async_context(async_id); } + return; } - - return; } if (type == PromiseHookType::kResolve && diff --git a/test/parallel/test-async-hooks-enable-before-promise-resolve.js b/test/parallel/test-async-hooks-enable-before-promise-resolve.js new file mode 100644 index 00000000000000..c96c9e5dd96655 --- /dev/null +++ b/test/parallel/test-async-hooks-enable-before-promise-resolve.js @@ -0,0 +1,25 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const async_hooks = require('async_hooks'); + +// This test ensures that fast-path PromiseHook assigns async ids +// to already created promises when the native hook function is +// triggered on before event. + +let initialAsyncId; +const promise = new Promise((resolve) => { + setTimeout(() => { + initialAsyncId = async_hooks.executionAsyncId(); + async_hooks.createHook({ + after: common.mustCall(() => {}, 2) + }).enable(); + resolve(); + }, 0); +}); + +promise.then(common.mustCall(() => { + const id = async_hooks.executionAsyncId(); + assert.notStrictEqual(id, initialAsyncId); + assert.ok(id > 0); +}));