Skip to content

Commit

Permalink
async_hooks: refactor to avoid unsafe array iteration
Browse files Browse the repository at this point in the history
PR-URL: #37125
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
  • Loading branch information
aduh95 authored and targos committed Jun 11, 2021
1 parent d85b70f commit dd18def
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 10 deletions.
15 changes: 7 additions & 8 deletions lib/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class AsyncHook {
// enable()/disable() are run during their execution. The following
// references are reassigned to the tmp arrays if a hook is currently being
// processed.
const [hooks_array, hook_fields] = getHookArrays();
const { 0: hooks_array, 1: hook_fields } = getHookArrays();

// Each hook is only allowed to be added once.
if (ArrayPrototypeIncludes(hooks_array, this))
Expand Down Expand Up @@ -116,7 +116,7 @@ class AsyncHook {
}

disable() {
const [hooks_array, hook_fields] = getHookArrays();
const { 0: hooks_array, 1: hook_fields } = getHookArrays();

const index = ArrayPrototypeIndexOf(hooks_array, this);
if (index === -1)
Expand Down Expand Up @@ -193,8 +193,7 @@ class AsyncResource {
emitBefore(asyncId, this[trigger_async_id_symbol], this);

try {
const ret = thisArg === undefined ?
fn(...args) :
const ret =
ReflectApply(fn, thisArg, args);

return ret;
Expand Down Expand Up @@ -302,25 +301,25 @@ class AsyncLocalStorage {
run(store, callback, ...args) {
// Avoid creation of an AsyncResource if store is already active
if (ObjectIs(store, this.getStore())) {
return callback(...args);
return ReflectApply(callback, null, args);
}
const resource = new AsyncResource('AsyncLocalStorage',
defaultAlsResourceOpts);
// Calling emitDestroy before runInAsyncScope avoids a try/finally
// It is ok because emitDestroy only schedules calling the hook
return resource.emitDestroy().runInAsyncScope(() => {
this.enterWith(store);
return callback(...args);
return ReflectApply(callback, null, args);
});
}

exit(callback, ...args) {
if (!this.enabled) {
return callback(...args);
return ReflectApply(callback, null, args);
}
this.disable();
try {
return callback(...args);
return ReflectApply(callback, null, args);
} finally {
this._enable();
}
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -428,14 +428,14 @@ function clearDefaultTriggerAsyncId() {

function defaultTriggerAsyncIdScope(triggerAsyncId, block, ...args) {
if (triggerAsyncId === undefined)
return block(...args);
return ReflectApply(block, null, args);
// CHECK(NumberIsSafeInteger(triggerAsyncId))
// CHECK(triggerAsyncId > 0)
const oldDefaultTriggerAsyncId = async_id_fields[kDefaultTriggerAsyncId];
async_id_fields[kDefaultTriggerAsyncId] = triggerAsyncId;

try {
return block(...args);
return ReflectApply(block, null, args);
} finally {
async_id_fields[kDefaultTriggerAsyncId] = oldDefaultTriggerAsyncId;
}
Expand Down

0 comments on commit dd18def

Please sign in to comment.