src: keep ALS store in AsyncResource::MakeCallback

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<Function> fn;
   Global<Function> done;
+  node::AsyncResource* resource = nullptr;
+
+  ~State() { delete resource; }
 };
 
 static void OnTimer(uv_timer_t* handle) {
@@ -30,22 +37,38 @@ static void OnTimer(uv_timer_t* handle) {
   Local<Object> 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<Function> done = state->done.Get(isolate);
   (void)node::MakeCallback(isolate, recv, done, 0, nullptr, {0, 0});
   uv_close(reinterpret_cast<uv_handle_t*>(&state->timer),
            [](uv_handle_t* h) { delete static_cast<State*>(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<Value>& args) {
   Isolate* isolate = args.GetIsolate();
   State* state = new State;
   state->isolate = isolate;
   state->n = args[0]->IntegerValue(isolate->GetCurrentContext()).FromJust();
   state->fn.Reset(isolate, args[1].As<Function>());
   state->done.Reset(isolate, args[2].As<Function>());
+  state->type = static_cast<Type>(
+      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);

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

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<Value> AsyncResource::MakeCallback(Local<Function> callback,
                                               int argc,
                                               Local<Value>* 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<Value> AsyncResource::MakeCallback(const char* method,
                                               int argc,
                                               Local<Value>* 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<String> method_string;
+  if (!String::NewFromUtf8(env_->isolate(), method).ToLocal(&method_string)) {
+    return {};
+  }
+  return MakeCallback(method_string, argc, argv);
 }
 
 MaybeLocal<Value> AsyncResource::MakeCallback(Local<String> symbol,
                                               int argc,
                                               Local<Value>* 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<Value> callback;
+  if (!get_resource()
+           ->Get(isolate->GetCurrentContext(), symbol)
+           .ToLocal(&callback)) {
+    return {};
+  }
+  if (!callback->IsFunction()) return Undefined(isolate);
+  return MakeCallback(callback.As<Function>(), argc, argv);
 }
 
 Local<Object> AsyncResource::get_resource() {

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