Repository: nodejs/node
Pull request: src: add async context frame to AsyncResource

Add member to hold the async context frame to `AsyncResource` to avoid the need for the `async_resource_context_frames_` map in `env`.

Semver major because it changes ABI.

Continuation of #54879.
Seems to be a good time now as `AsyncContextFrame` is now default for ALS (see #55552).


Changed files:
filename: src/api/async_resource.cc
patch: @@ -17,28 +17,23 @@ AsyncResource::AsyncResource(Isolate* isolate,
                              const char* name,
                              async_id trigger_async_id)
     : env_(Environment::GetCurrent(isolate)),
-      resource_(isolate, resource) {
+      resource_(isolate, resource),
+      context_frame_(isolate, async_context_frame::current(isolate)) {
   CHECK_NOT_NULL(env_);
-  env_->SetAsyncResourceContextFrame(
-      reinterpret_cast<std::uintptr_t>(this),
-      {isolate, async_context_frame::current(isolate)});
   async_context_ = EmitAsyncInit(isolate, resource, name, trigger_async_id);
 }
 
 AsyncResource::~AsyncResource() {
   CHECK_NOT_NULL(env_);
   EmitAsyncDestroy(env_, async_context_);
-  env_->RemoveAsyncResourceContextFrame(reinterpret_cast<std::uintptr_t>(this));
 }
 
 MaybeLocal<Value> AsyncResource::MakeCallback(Local<Function> callback,
                                               int argc,
                                               Local<Value>* argv) {
   auto isolate = env_->isolate();
-  auto context_frame =
-      env_->GetAsyncResourceContextFrame(reinterpret_cast<std::uintptr_t>(this))
-          .Get(isolate);
-  async_context_frame::Scope async_context_frame_scope(isolate, context_frame);
+  async_context_frame::Scope async_context_frame_scope(
+      isolate, context_frame_.Get(isolate));
 
   return node::MakeCallback(
       isolate, get_resource(), callback, argc, argv, async_context_);
@@ -48,10 +43,8 @@ MaybeLocal<Value> AsyncResource::MakeCallback(const char* method,
                                               int argc,
                                               Local<Value>* argv) {
   auto isolate = env_->isolate();
-  auto context_frame =
-      env_->GetAsyncResourceContextFrame(reinterpret_cast<std::uintptr_t>(this))
-          .Get(isolate);
-  async_context_frame::Scope async_context_frame_scope(isolate, context_frame);
+  async_context_frame::Scope async_context_frame_scope(
+      isolate, context_frame_.Get(isolate));
 
   return node::MakeCallback(
       isolate, get_resource(), method, argc, argv, async_context_);
@@ -61,10 +54,8 @@ MaybeLocal<Value> AsyncResource::MakeCallback(Local<String> symbol,
                                               int argc,
                                               Local<Value>* argv) {
   auto isolate = env_->isolate();
-  auto context_frame =
-      env_->GetAsyncResourceContextFrame(reinterpret_cast<std::uintptr_t>(this))
-          .Get(isolate);
-  async_context_frame::Scope async_context_frame_scope(isolate, context_frame);
+  async_context_frame::Scope async_context_frame_scope(
+      isolate, context_frame_.Get(isolate));
 
   return node::MakeCallback(
       isolate, get_resource(), symbol, argc, argv, async_context_);

filename: src/env-inl.h
patch: @@ -908,26 +908,6 @@ inline void Environment::RemoveHeapSnapshotNearHeapLimitCallback(
                                         heap_limit);
 }
 
-inline void Environment::SetAsyncResourceContextFrame(
-    std::uintptr_t async_resource_handle,
-    v8::Global<v8::Value>&& context_frame) {
-  async_resource_context_frames_.emplace(
-      std::make_pair(async_resource_handle, std::move(context_frame)));
-}
-
-inline const v8::Global<v8::Value>& Environment::GetAsyncResourceContextFrame(
-    std::uintptr_t async_resource_handle) {
-  auto&& async_resource_context_frame =
-      async_resource_context_frames_.find(async_resource_handle);
-  CHECK_NE(async_resource_context_frame, async_resource_context_frames_.end());
-
-  return async_resource_context_frame->second;
-}
-
-inline void Environment::RemoveAsyncResourceContextFrame(
-    std::uintptr_t async_resource_handle) {
-  async_resource_context_frames_.erase(async_resource_handle);
-}
 }  // namespace node
 
 // These two files depend on each other. Including base_object-inl.h after this

filename: src/env.h
patch: @@ -1070,14 +1070,6 @@ class Environment final : public MemoryRetainer {
 
   v8::Global<v8::Module> temporary_required_module_facade_original;
 
-  void SetAsyncResourceContextFrame(std::uintptr_t async_resource_handle,
-                                    v8::Global<v8::Value>&&);
-
-  const v8::Global<v8::Value>& GetAsyncResourceContextFrame(
-      std::uintptr_t async_resource_handle);
-
-  void RemoveAsyncResourceContextFrame(std::uintptr_t async_resource_handle);
-
  private:
   inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>,
                                                      v8::Local<v8::Value>),
@@ -1252,9 +1244,6 @@ class Environment final : public MemoryRetainer {
   // track of the BackingStore for a given pointer.
   std::unordered_map<char*, std::unique_ptr<v8::BackingStore>>
       released_allocated_buffers_;
-
-  std::unordered_map<std::uintptr_t, v8::Global<v8::Value>>
-      async_resource_context_frames_;
 };
 
 }  // namespace node

filename: src/node.h
patch: @@ -1524,6 +1524,7 @@ class NODE_EXTERN AsyncResource {
  private:
   Environment* env_;
   v8::Global<v8::Object> resource_;
+  v8::Global<v8::Value> context_frame_;
   async_context async_context_;
 };