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