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 <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
  • Loading branch information
Flarna authored and juanarbol committed Apr 6, 2022
1 parent abbb236 commit bb71433
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 @@ -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.
Expand All @@ -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);
}
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 bb71433

Please sign in to comment.