From 8344ac3a1748794edcc85993533ad44983a41edb Mon Sep 17 00:00:00 2001 From: Nigro Simone Date: Sun, 4 Oct 2026 16:54:28 +0200 Subject: [PATCH] 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: https://github.com/nodejs/performance/issues/24 Signed-off-by: Nigro Simone --- lib/internal/async_hooks.js | 18 +++++++- src/api/callback.cc | 45 +++++++++++++++++-- src/async_wrap.cc | 18 ++++++++ src/env.cc | 17 ++++++- src/env.h | 10 +++++ src/node_internals.h | 10 +++++ .../test-execution-async-resource.js | 31 +++++++++++++ 7 files changed, 144 insertions(+), 5 deletions(-) create mode 100755 test/addons/async-resource/test-execution-async-resource.js diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 8a60b9a6ce52..eb403824edd1 100644 --- a/lib/internal/async_hooks.js +++ b/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). diff --git a/src/api/callback.cc b/src/api/callback.cc index 15bba45d786b..b0ac92c913f7 100644 --- a/src/api/callback.cc +++ b/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 InternalCallbackScope::lazy_resource(Isolate* isolate) const { + if (std::holds_alternative*>(lazy_resource_)) { + return *std::get*>(lazy_resource_); + } + return std::get*>(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)); } diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 7b7930187bd5..2030599e0798 100644 --- a/src/async_wrap.cc +++ b/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& 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& args) { + Environment* env = Environment::GetCurrent(args); + uint32_t depth = args[0].As()->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& 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 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); diff --git a/src/env.cc b/src/env.cc index e96b6e6bb129..b9c829484745 100644 --- a/src/env.cc +++ b/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 ctx) { diff --git a/src/env.h b/src/env.h index 63c52524695a..c4b42b250649 100644 --- a/src/env.h +++ b/src/env.h @@ -368,6 +368,8 @@ extern std::shared_ptr 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, 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 { diff --git a/src/node_internals.h b/src/node_internals.h index 1c4da8e2d203..a84bb651034d 100644 --- a/src/node_internals.h +++ b/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 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::Global*> lazy_resource_; v8::Global prior_context_frame_; std::optional allow_js_; }; diff --git a/test/addons/async-resource/test-execution-async-resource.js b/test/addons/async-resource/test-execution-async-resource.js new file mode 100755 index 000000000000..51386bd3a6bb --- /dev/null +++ b/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);