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.
  • Loading branch information
Flarna committed Mar 19, 2022
1 parent 7fdb9d5 commit 50a82b7
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 50a82b7

Please sign in to comment.