async_hooks: fix id assignment in fast-path promise hook

Native side of fast-path promise hook was not calling JS fastPromiseHook
function when there were no async ids previously assigned to the promise.
Because of that already created promises could not get id assigned in
situations when an async hook without a before listener function is enabled
after their creation. As the result executionAsyncId could return wrong id
when called within promise's .then().

Refs: #34512

Author: puzpuzpuz

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<double> GetAssignedPromiseAsyncId(Environment* env,
+                                               Local<Promise> promise,
+                                               Local<Value> id_symbol) {
+  Local<Value> maybe_async_id;
+  if (!promise->Get(env->context(), id_symbol).ToLocal(&maybe_async_id)) {
+    return Nothing<double>();
+  }
+  return maybe_async_id->IsNumber()
+      ? maybe_async_id->NumberValue(env->context())
+      : Just(AsyncWrap::kInvalidAsyncId);
+}
+
 class PromiseWrap : public AsyncWrap {
  public:
   PromiseWrap(Environment* env, Local<Object> object, bool silent)
@@ -231,18 +249,17 @@ PromiseWrap* PromiseWrap::New(Environment* env,
 
   // Skip for init events
   if (silent) {
-    Local<Value> maybe_async_id = promise
-        ->Get(context, env->async_id_symbol())
-        .ToLocalChecked();
-
-    Local<Value> 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<Number>()->Value();
-      double trigger_async_id = maybe_trigger_async_id.As<Number>()->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> promise,
 
   if (type == PromiseHookType::kBefore &&
       env->async_hooks()->fields()[AsyncHooks::kBefore] == 0) {
-    Local<Value> maybe_async_id;
-    if (!promise->Get(context, env->async_id_symbol())
-          .ToLocal(&maybe_async_id)) {
-      return;
-    }
-
-    Local<Value> 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<Number>()->Value();
-      double trigger_async_id = maybe_trigger_async_id.As<Number>()->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<Value> 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<Number>()->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 &&

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);
+}));