src: skip the async id stack when nobody sees it

With no async hook and no executionAsyncResource() user, every
InternalCallbackScope pushed and popped the async ids on the stack, only
for executionAsyncId() and triggerAsyncId() to read the top. Swap the
two ids in place instead: they are all that those two and
process.nextTick() read. executionAsyncResource() is the only reader of
the stack: the scopes that skip it form a chain in AsyncHooks, counted
in a new field, kLazyScopes, and JS asks C++ for the innermost one only
while that count is not zero. The checks for a corrupted stack stay.

Refs: nodejs/performance#24
Signed-off-by: Nigro Simone <nigro.simone@gmail.com>

Author: nigrosimone

lib/internal/async_hooks.js

@@ -52,6 +52,7 @@ const {
   pushAsyncContext: pushAsyncContext_,
   popAsyncContext: popAsyncContext_,
   executionAsyncResource: executionAsyncResource_,
+  lazyExecutionAsyncResource,
   clearAsyncIdStack,
 } = async_wrap;
 // Properties in active_hooks are used to keep track of the set of hooks being
@@ -91,6 +92,7 @@ const {
   kInit, kBefore, kAfter, kDestroy, kTotals, kPromiseResolve,
   kCheck, kExecutionAsyncId, kAsyncIdCounter, kTriggerAsyncId,
   kDefaultTriggerAsyncId, kStackLength, kUsesExecutionAsyncResource,
+  kLazyScopes,
 } = async_wrap.constants;
 
 const { async_id_symbol,
@@ -136,6 +138,12 @@ function executionAsyncResource() {
   async_hook_fields[kUsesExecutionAsyncResource] = 1;
 
   const index = async_hook_fields[kStackLength] - 1;
+  // A native callback that started before anybody used this function did not
+  // push its resource on the stack.
+  if (async_hook_fields[kLazyScopes] !== 0) {
+    const lazy = lazyExecutionAsyncResource(index + 1);
+    if (lazy !== undefined) return lookupPublicResource(lazy);
+  }
   if (index === -1) return topLevelResource;
   const resource = execution_async_resources[index] ||
     executionAsyncResource_(index);
@@ -550,7 +558,15 @@ function pushAsyncContext(asyncId, triggerAsyncId, resource) {
 // This is the equivalent of the native pop_async_ids() call.
 function popAsyncContext(asyncId) {
   const stackLength = async_hook_fields[kStackLength];
-  if (stackLength === 0) return false;
+  if (stackLength === 0) {
+    // A native callback that skipped the stack still has its id checked.
+    if (async_hook_fields[kLazyScopes] !== 0 &&
+        async_hook_fields[kCheck] > 0 &&
+        async_id_fields[kExecutionAsyncId] !== asyncId) {
+      return popAsyncContext_(asyncId);
+    }
+    return false;
+  }
 
   if (async_hook_fields[kCheck] > 0 && async_id_fields[kExecutionAsyncId] !== asyncId) {
     // Do the same thing as the native code (i.e. crash hard).

src/api/callback.cc

@@ -118,8 +118,27 @@ InternalCallbackScope::InternalCallbackScope(
     prior_context_frame_.Reset(isolate, prior_context_frame);
   }
 
-  env->async_hooks()->push_async_context(
-      async_context_.async_id, async_context_.trigger_async_id, object);
+  AsyncHooks* hooks = env->async_hooks();
+  if (hooks->fields()[AsyncHooks::kTotals] == 0 &&
+      hooks->fields()[AsyncHooks::kUsesExecutionAsyncResource] == 0)
+      [[likely]] {
+    // Nobody can see the id stack: swap the ids in place, they are all that
+    // executionAsyncId() and triggerAsyncId() read.
+    AliasedFloat64Array& ids = hooks->async_id_fields();
+    prior_async_id_ = ids[AsyncHooks::kExecutionAsyncId];
+    prior_trigger_async_id_ = ids[AsyncHooks::kTriggerAsyncId];
+    ids[AsyncHooks::kExecutionAsyncId] = async_context_.async_id;
+    ids[AsyncHooks::kTriggerAsyncId] = async_context_.trigger_async_id;
+    lazy_depth_ = hooks->fields()[AsyncHooks::kStackLength];
+    lazy_resource_ = object;
+    lazy_prev_ = hooks->lazy_top_;
+    hooks->lazy_top_ = this;
+    hooks->fields()[AsyncHooks::kLazyScopes] += 1;
+    lazy_ids_ = true;
+  } else {
+    hooks->push_async_context(
+        async_context_.async_id, async_context_.trigger_async_id, object);
+  }
 
   pushed_ids_ = true;
 
@@ -130,6 +149,13 @@ InternalCallbackScope::InternalCallbackScope(
   }
 }
 
+Local<Object> InternalCallbackScope::lazy_resource(Isolate* isolate) const {
+  if (std::holds_alternative<Local<Object>*>(lazy_resource_)) {
+    return *std::get<Local<Object>*>(lazy_resource_);
+  }
+  return std::get<Global<Object>*>(lazy_resource_)->Get(isolate);
+}
+
 InternalCallbackScope::~InternalCallbackScope() {
   Close();
   env_->PopAsyncCallbackScope();
@@ -160,7 +186,20 @@ void InternalCallbackScope::Close() {
   }
 
   if (pushed_ids_) {
-    env_->async_hooks()->pop_async_context(async_context_.async_id);
+    AsyncHooks* hooks = env_->async_hooks();
+    if (lazy_ids_) {
+      // clear_async_id_stack() may have dropped the chain already.
+      if (hooks->lazy_top_ == this) {
+        hooks->CheckLazyClose(async_context_.async_id, lazy_depth_);
+        hooks->lazy_top_ = lazy_prev_;
+        hooks->fields()[AsyncHooks::kLazyScopes] -= 1;
+      }
+      AliasedFloat64Array& ids = hooks->async_id_fields();
+      ids[AsyncHooks::kExecutionAsyncId] = prior_async_id_;
+      ids[AsyncHooks::kTriggerAsyncId] = prior_trigger_async_id_;
+    } else {
+      hooks->pop_async_context(async_context_.async_id);
+    }
 
     async_context_frame::set(env_, prior_context_frame_.Get(isolate));
   }

src/async_wrap.cc

@@ -25,6 +25,7 @@
 #include "env-inl.h"
 #include "node_errors.h"
 #include "node_external_reference.h"
+#include "node_internals.h"
 #include "tracing/traced_value.h"
 #include "util-inl.h"
 
@@ -290,6 +291,17 @@ void AsyncWrap::PopAsyncContext(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(env->async_hooks()->pop_async_context(async_id));
 }
 
+// The resource of the innermost scope that skipped the id stack, if nothing
+// was pushed on the stack after it (depth is the stack length JS sees).
+static void LazyExecutionAsyncResource(
+    const v8::FunctionCallbackInfo<v8::Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  uint32_t depth = args[0].As<v8::Uint32>()->Value();
+  InternalCallbackScope* scope = env->async_hooks()->lazy_top_;
+  if (scope != nullptr && scope->lazy_depth() == depth) {
+    args.GetReturnValue().Set(scope->lazy_resource(env->isolate()));
+  }
+}
 
 void AsyncWrap::ExecutionAsyncResource(
     const FunctionCallbackInfo<Value>& args) {
@@ -396,6 +408,10 @@ void AsyncWrap::CreatePerIsolateProperties(IsolateData* isolate_data,
   SetMethod(isolate, target, "pushAsyncContext", PushAsyncContext);
   SetMethod(isolate, target, "popAsyncContext", PopAsyncContext);
   SetMethod(isolate, target, "executionAsyncResource", ExecutionAsyncResource);
+  SetMethod(isolate,
+            target,
+            "lazyExecutionAsyncResource",
+            LazyExecutionAsyncResource);
   SetMethod(isolate, target, "clearAsyncIdStack", ClearAsyncIdStack);
   SetMethod(isolate, target, "queueDestroyAsyncId", QueueDestroyAsyncId);
   SetMethod(isolate, target, "setPromiseHooks", SetPromiseHooks);
@@ -470,6 +486,7 @@ void AsyncWrap::CreatePerContextProperties(Local<Object> target,
   SET_HOOKS_CONSTANT(kDefaultTriggerAsyncId);
   SET_HOOKS_CONSTANT(kUsesExecutionAsyncResource);
   SET_HOOKS_CONSTANT(kStackLength);
+  SET_HOOKS_CONSTANT(kLazyScopes);
 #undef SET_HOOKS_CONSTANT
   FORCE_SET_TARGET_FIELD(target, "constants", constants);
 
@@ -502,6 +519,7 @@ void AsyncWrap::RegisterExternalReferences(
   registry->Register(PushAsyncContext);
   registry->Register(PopAsyncContext);
   registry->Register(ExecutionAsyncResource);
+  registry->Register(LazyExecutionAsyncResource);
   registry->Register(ClearAsyncIdStack);
   registry->Register(QueueDestroyAsyncId);
   registry->Register(SetPromiseHooks);

src/env.cc

@@ -167,12 +167,25 @@ void AsyncHooks::push_async_context(
   }
 }
 
+void AsyncHooks::CheckLazyClose(double async_id, uint32_t depth) {
+  if (fields_[kCheck] > 0 && (async_id_fields_[kExecutionAsyncId] != async_id ||
+                              fields_[kStackLength] != depth)) [[unlikely]] {
+    FailWithCorruptedAsyncStack(async_id);
+  }
+}
+
 // Remember to keep this code aligned with popAsyncContext() in JS.
 bool AsyncHooks::pop_async_context(double async_id) {
   // In case of an exception then this may have already been reset, if the
   // stack was multiple MakeCallback()'s deep.
-  if (fields_[kStackLength] == 0) [[unlikely]]
+  if (fields_[kStackLength] == 0) [[unlikely]] {
+    // A scope that skipped the stack still has its id checked.
+    if (fields_[kLazyScopes] > 0 && fields_[kCheck] > 0 &&
+        async_id_fields_[kExecutionAsyncId] != async_id) {
+      FailWithCorruptedAsyncStack(async_id);
+    }
     return false;
+  }
 
   // Ask for the async_id to be restored as a check that the stack
   // hasn't been corrupted.
@@ -227,6 +240,8 @@ void AsyncHooks::clear_async_id_stack() {
   async_id_fields_[kExecutionAsyncId] = 0;
   async_id_fields_[kTriggerAsyncId] = 0;
   fields_[kStackLength] = 0;
+  lazy_top_ = nullptr;
+  fields_[kLazyScopes] = 0;
 }
 
 void AsyncHooks::InstallPromiseHooks(Local<Context> ctx) {

src/env.h

@@ -368,6 +368,8 @@ extern std::shared_ptr<KVStore> system_environment;
 
 struct EnvSerializeInfo;
 
+class InternalCallbackScope;
+
 class AsyncHooks : public MemoryRetainer {
  public:
   SET_MEMORY_INFO_NAME(AsyncHooks)
@@ -386,6 +388,7 @@ class AsyncHooks : public MemoryRetainer {
     kCheck,
     kStackLength,
     kUsesExecutionAsyncResource,
+    kLazyScopes,
     kFieldsCount,
   };
 
@@ -502,6 +505,13 @@ class AsyncHooks : public MemoryRetainer {
   const SerializeInfo* info_ = nullptr;
 
   std::array<v8::Global<v8::Function>, 4> js_promise_hooks_;
+
+ public:
+  // Innermost InternalCallbackScope that swapped the ids without the stack,
+  // for executionAsyncResource(). Counted in fields_[kLazyScopes].
+  InternalCallbackScope* lazy_top_ = nullptr;
+  // Close of a scope that skipped the stack: the same check as pop.
+  void CheckLazyClose(double async_id, uint32_t depth);
 };
 
 class ImmediateInfo : public MemoryRetainer {

src/node_internals.h

@@ -270,6 +270,10 @@ class InternalCallbackScope {
   inline bool Failed() const { return failed_; }
   inline void MarkAsFailed() { failed_ = true; }
 
+  // For executionAsyncResource() inside a scope that skipped the id stack.
+  inline uint32_t lazy_depth() const { return lazy_depth_; }
+  v8::Local<v8::Object> lazy_resource(v8::Isolate* isolate) const;
+
  private:
   Environment* env_;
   async_context async_context_;
@@ -279,6 +283,12 @@ class InternalCallbackScope {
   bool failed_ = false;
   bool pushed_ids_ = false;
   bool closed_ = false;
+  bool lazy_ids_ = false;
+  uint32_t lazy_depth_ = 0;
+  double prior_async_id_ = 0;
+  double prior_trigger_async_id_ = 0;
+  InternalCallbackScope* lazy_prev_ = nullptr;
+  std::variant<v8::Local<v8::Object>*, v8::Global<v8::Object>*> lazy_resource_;
   v8::Global<v8::Value> prior_context_frame_;
   std::optional<v8::Isolate::AllowJavascriptExecutionScope> allow_js_;
 };

test/addons/async-resource/test-execution-async-resource.js

@@ -0,0 +1,31 @@
+'use strict';
+
+const common = require('../../common');
+const assert = require('assert');
+const binding = require(`./build/${common.buildType}/binding`);
+const { executionAsyncId, executionAsyncResource } = require('async_hooks');
+
+// No hook is enabled and executionAsyncResource() was never called, so the
+// callback scope of AsyncResource::MakeCallback() skips the async id stack.
+// executionAsyncResource() must still find the resource, and executionAsyncId()
+// must still be the id of the callback, also around a nested callback.
+let calls = 0;
+const object = {
+  methöd: common.mustCall(function() {
+    assert.strictEqual(executionAsyncId(), uid);
+    assert.strictEqual(executionAsyncResource(), object);
+    if (calls++ === 0) {
+      assert.strictEqual(binding.callViaFunction(resource), 'baz');
+      assert.strictEqual(executionAsyncId(), uid);
+      assert.strictEqual(executionAsyncResource(), object);
+    }
+    return 'baz';
+  }, 2),
+};
+const resource = binding.createAsyncResource(object);
+const uid = binding.getAsyncId(resource);
+const outerId = executionAsyncId();
+
+assert.strictEqual(binding.callViaFunction(resource), 'baz');
+assert.strictEqual(executionAsyncId(), outerId);
+binding.destroyAsyncResource(resource);