async_hooks: remove destroyed symbol on Promises

Promises are never destroyed manually therefore it's not needed to attach an object to track if destroy hook was called already. PR-URL: #42402 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
nodejs · Apr 6, 2022 · bb71433 · bb71433
1 parent abbb236
commit bb71433
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 10 deletions.
diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js
@@ -326,14 +326,10 @@ function promiseInitHookWithDestroyTracking(promise, parent) {
   destroyTracking(promise, parent);
 }
 
-const destroyedSymbol = Symbol('destroyed');
-
 function destroyTracking(promise, parent) {
   trackPromise(promise, parent);
   const asyncId = promise[async_id_symbol];
-  const destroyed = { destroyed: false };
-  promise[destroyedSymbol] = destroyed;
-  registerDestroyHook(promise, asyncId, destroyed);
+  registerDestroyHook(promise, asyncId);
 }
 
 function promiseBeforeHook(promise) {

diff --git a/src/async_wrap.cc b/src/async_wrap.cc
@@ -214,12 +214,13 @@ void AsyncWrap::WeakCallback(const WeakCallbackInfo<DestroyParam>& info) {
 
   p->env->RemoveCleanupHook(DestroyParamCleanupHook, p.get());
 
-  if (!prop_bag->Get(p->env->context(), p->env->destroyed_string())
+  if (!prop_bag.IsEmpty() &&
+      !prop_bag->Get(p->env->context(), p->env->destroyed_string())
         .ToLocal(&val)) {
     return;
   }
 
-  if (val->IsFalse()) {
+  if (val.IsEmpty() || val->IsFalse()) {
     AsyncWrap::EmitDestroy(p->env, p->asyncId);
   }
   // unique_ptr goes out of scope here and pointer is deleted.
@@ -229,14 +230,16 @@ void AsyncWrap::WeakCallback(const WeakCallbackInfo<DestroyParam>& info) {
 static void RegisterDestroyHook(const FunctionCallbackInfo<Value>& args) {
   CHECK(args[0]->IsObject());
   CHECK(args[1]->IsNumber());
-  CHECK(args[2]->IsObject());
+  CHECK(args.Length() == 2 || args[2]->IsObject());
 
   Isolate* isolate = args.GetIsolate();
   DestroyParam* p = new DestroyParam();
   p->asyncId = args[1].As<Number>()->Value();
   p->env = Environment::GetCurrent(args);
   p->target.Reset(isolate, args[0].As<Object>());
-  p->propBag.Reset(isolate, args[2].As<Object>());
+  if (args.Length() > 2) {
+    p->propBag.Reset(isolate, args[2].As<Object>());
+  }
   p->target.SetWeak(p, AsyncWrap::WeakCallback, WeakCallbackType::kParameter);
   p->env->AddCleanupHook(DestroyParamCleanupHook, p);
 }

diff --git a/typings/internalBinding/async_wrap.d.ts b/typings/internalBinding/async_wrap.d.ts
@@ -107,7 +107,7 @@ declare function InternalBinding(binding: 'async_wrap'): {
     promiseAfterHook: InternalAsyncWrapBinding.PromiseHook | undefined,
     promiseResolveHook: InternalAsyncWrapBinding.PromiseHook | undefined
   ): void;
-  registerDestroyHook(promise: Promise<unknown>, asyncId: number, destroyed: { destroyed: boolean }): void;
+  registerDestroyHook(resource: object, asyncId: number, destroyed?: { destroyed: boolean }): void;
   async_hook_fields: Uint32Array;
   async_id_fields: Float64Array;
   async_ids_stack: Float64Array;