From 154c15a9c01abaffc164420ad5c4bd69a2595622 Mon Sep 17 00:00:00 2001 From: Nigro Simone Date: Sat, 26 Sep 2026 19:51:02 +0200 Subject: [PATCH 1/2] src: keep ALS store in AsyncResource::MakeCallback AsyncResource saves the async context frame when it is created, and MakeCallback() enters it with async_context_frame::Scope. Then node::MakeCallback() opens the callback scope with an undefined frame, so the callback never runs in the saved one. Since AsyncContextFrame is the default, AsyncLocalStorage loses its store in these callbacks. Pass the saved frame to InternalMakeCallback(), as the Node-API AsyncContext already does. This also removes the Scope from every call, with its two Environment lookups and its global handle. Refs: https://github.com/nodejs/node/pull/66316 Refs: https://github.com/nodejs/node/issues/43038 Refs: https://github.com/nodejs/performance/issues/24 Signed-off-by: Nigro Simone --- src/api/async_resource.cc | 41 +++++++++++-------- .../test-async-local-storage.js | 22 ++++++++++ 2 files changed, 47 insertions(+), 16 deletions(-) create mode 100644 test/addons/async-resource/test-async-local-storage.js diff --git a/src/api/async_resource.cc b/src/api/async_resource.cc index 621fca4ba4d1..54ed0382f896 100644 --- a/src/api/async_resource.cc +++ b/src/api/async_resource.cc @@ -1,6 +1,7 @@ #include "async_context_frame.h" #include "env-inl.h" #include "node.h" +#include "node_internals.h" namespace node { @@ -10,6 +11,7 @@ using v8::Local; using v8::MaybeLocal; using v8::Object; using v8::String; +using v8::Undefined; using v8::Value; AsyncResource::AsyncResource(Isolate* isolate, @@ -39,33 +41,40 @@ MaybeLocal AsyncResource::MakeCallback(Local callback, int argc, Local* argv) { auto isolate = env_->isolate(); - async_context_frame::Scope async_context_frame_scope( - isolate, context_frame_.Get(isolate)); - - return node::MakeCallback( - isolate, get_resource(), callback, argc, argv, async_context_); + // As in Node-API: node::MakeCallback() would run it with no frame. + return InternalMakeCallback(isolate, + get_resource(), + callback, + argc, + argv, + async_context_, + context_frame_.Get(isolate)); } MaybeLocal AsyncResource::MakeCallback(const char* method, int argc, Local* argv) { - auto isolate = env_->isolate(); - async_context_frame::Scope async_context_frame_scope( - isolate, context_frame_.Get(isolate)); - - return node::MakeCallback( - isolate, get_resource(), method, argc, argv, async_context_); + Local method_string; + if (!String::NewFromUtf8(env_->isolate(), method).ToLocal(&method_string)) { + return {}; + } + return MakeCallback(method_string, argc, argv); } MaybeLocal AsyncResource::MakeCallback(Local symbol, int argc, Local* argv) { auto isolate = env_->isolate(); - async_context_frame::Scope async_context_frame_scope( - isolate, context_frame_.Get(isolate)); - - return node::MakeCallback( - isolate, get_resource(), symbol, argc, argv, async_context_); + // Check can_call_into_js() first because calling Get() might do so. + if (!env_->can_call_into_js()) return {}; + Local callback; + if (!get_resource() + ->Get(isolate->GetCurrentContext(), symbol) + .ToLocal(&callback)) { + return {}; + } + if (!callback->IsFunction()) return Undefined(isolate); + return MakeCallback(callback.As(), argc, argv); } Local AsyncResource::get_resource() { diff --git a/test/addons/async-resource/test-async-local-storage.js b/test/addons/async-resource/test-async-local-storage.js new file mode 100644 index 000000000000..f475de221710 --- /dev/null +++ b/test/addons/async-resource/test-async-local-storage.js @@ -0,0 +1,22 @@ +'use strict'; + +const common = require('../../common'); +const assert = require('assert'); +const { AsyncLocalStorage } = require('async_hooks'); +const binding = require(`./build/${common.buildType}/binding`); + +// AsyncResource::MakeCallback() must run the callback in the async context +// frame that was active when the resource was created. + +const als = new AsyncLocalStorage(); +const object = { + 'methöd': common.mustCall(() => { + assert.strictEqual(als.getStore(), 'store'); + }, 3), +}; +const resource = als.run('store', () => binding.createAsyncResource(object)); + +binding.callViaFunction(resource); +binding.callViaString(resource); +binding.callViaUtf8Name(resource); +binding.destroyAsyncResource(resource); From 1212c0d0d9a6ef963598bdb553748776be63f6b8 Mon Sep 17 00:00:00 2001 From: Nigro Simone Date: Sat, 26 Sep 2026 20:04:28 +0200 Subject: [PATCH 2/2] benchmark: add AsyncResource and Call cases type=AsyncResource calls node::AsyncResource::MakeCallback() and type=Call a plain v8::Function::Call, from the same libuv timer as type=MakeCallback. Call is what the call costs without Node. Signed-off-by: Nigro Simone --- benchmark/napi/make_callback/binding.cc | 27 +++++++++++++++++++++++-- benchmark/napi/make_callback/index.js | 12 +++++++---- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/benchmark/napi/make_callback/binding.cc b/benchmark/napi/make_callback/binding.cc index 85d335170fb0..3a793b6c1902 100644 --- a/benchmark/napi/make_callback/binding.cc +++ b/benchmark/napi/make_callback/binding.cc @@ -12,12 +12,19 @@ using v8::Local; using v8::Object; using v8::Value; +// Same order as the types in index.js. +enum Type { kMakeCallback, kAsyncResource, kCall }; + struct State { uv_timer_t timer; Isolate* isolate; int64_t n; + Type type; Global fn; Global done; + node::AsyncResource* resource = nullptr; + + ~State() { delete resource; } }; static void OnTimer(uv_timer_t* handle) { @@ -30,7 +37,17 @@ static void OnTimer(uv_timer_t* handle) { Local recv = context->Global(); for (int64_t i = 0; i < state->n; i++) { HandleScope inner_scope(isolate); - (void)node::MakeCallback(isolate, recv, fn, 0, nullptr, {0, 0}); + switch (state->type) { + case kMakeCallback: + (void)node::MakeCallback(isolate, recv, fn, 0, nullptr, {0, 0}); + break; + case kAsyncResource: + (void)state->resource->MakeCallback(fn, 0, nullptr); + break; + case kCall: + (void)fn->Call(context, recv, 0, nullptr); + break; + } } Local done = state->done.Get(isolate); (void)node::MakeCallback(isolate, recv, done, 0, nullptr, {0, 0}); @@ -38,7 +55,7 @@ static void OnTimer(uv_timer_t* handle) { [](uv_handle_t* h) { delete static_cast(h->data); }); } -// run(n, fn, done): calls fn n times from a timer, then calls done. +// run(n, fn, done, type): calls fn n times from a timer, then calls done. static void Run(const FunctionCallbackInfo& args) { Isolate* isolate = args.GetIsolate(); State* state = new State; @@ -46,6 +63,12 @@ static void Run(const FunctionCallbackInfo& args) { state->n = args[0]->IntegerValue(isolate->GetCurrentContext()).FromJust(); state->fn.Reset(isolate, args[1].As()); state->done.Reset(isolate, args[2].As()); + state->type = static_cast( + args[3]->Int32Value(isolate->GetCurrentContext()).FromJust()); + if (state->type == kAsyncResource) { + state->resource = + new node::AsyncResource(isolate, Object::New(isolate), "Benchmark"); + } state->timer.data = state; uv_timer_init(node::GetCurrentEventLoop(isolate), &state->timer); uv_timer_start(&state->timer, OnTimer, 0, 0); diff --git a/benchmark/napi/make_callback/index.js b/benchmark/napi/make_callback/index.js index 8c3dba383437..37e76788be70 100644 --- a/benchmark/napi/make_callback/index.js +++ b/benchmark/napi/make_callback/index.js @@ -2,8 +2,9 @@ const common = require('../../common.js'); -// The addon calls into JS with node::MakeCallback from a libuv timer, so -// every call opens a top-level callback scope, like an I/O callback does. +// The addon calls into JS from a libuv timer, so every call opens a top-level +// callback scope, like an I/O callback does. type=Call is a plain +// v8::Function::Call, what the call costs without Node. let binding; try { @@ -13,11 +14,14 @@ try { process.exit(0); } +const types = ['MakeCallback', 'AsyncResource', 'Call']; + const bench = common.createBenchmark(main, { + type: types, n: [1e6, 1e7], }); -function main({ n }) { +function main({ type, n }) { bench.start(); - binding.run(n, () => {}, () => bench.end(n)); + binding.run(n, () => {}, () => bench.end(n), types.indexOf(type)); }