Skip to content

Commit

Permalink
async_hooks: remove destroyed symbol on Promises
Browse files Browse the repository at this point in the history
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 <rtrott@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
  • Loading branch information
Flarna authored and danielleadams committed Jun 26, 2022
1 parent b0bf132 commit 9bd9b61
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 10 deletions.
6 changes: 1 addition & 5 deletions lib/internal/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
11 changes: 7 additions & 4 deletions src/async_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -217,12 +217,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.
Expand All @@ -232,14 +233,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);
}
Expand Down
2 changes: 1 addition & 1 deletion typings/internalBinding/async_wrap.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 9bd9b61

Please sign in to comment.