Skip to content

Commit c4199a7

Browse files
Flarnaaduh95
authored andcommitted
async_hooks: enabledHooksExist shall return if hooks are enabled
Correct the implementaton of enabledHooksExist to return true if there are enabled hooks. Adapt callsites which used getHooksArrays() as workaround. PR-URL: #61054 Fixes: #61019 Refs: #59873 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
1 parent 0b0d77a commit c4199a7

File tree

5 files changed

+25
-9
lines changed

5 files changed

+25
-9
lines changed

lib/async_hooks.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ const {
5555
emitBefore,
5656
emitAfter,
5757
emitDestroy,
58-
enabledHooksExist,
5958
initHooksExist,
6059
destroyHooksExist,
6160
} = internal_async_hooks;
@@ -203,7 +202,7 @@ class AsyncResource {
203202
this[trigger_async_id_symbol] = triggerAsyncId;
204203

205204
if (initHooksExist()) {
206-
if (enabledHooksExist() && type.length === 0) {
205+
if (type.length === 0) {
207206
throw new ERR_ASYNC_TYPE(type);
208207
}
209208

lib/internal/async_hooks.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,7 @@ function hasHooks(key) {
480480
}
481481

482482
function enabledHooksExist() {
483-
return hasHooks(kCheck);
483+
return active_hooks.array.length > 0;
484484
}
485485

486486
function initHooksExist() {
@@ -562,7 +562,7 @@ function popAsyncContext(asyncId) {
562562
const stackLength = async_hook_fields[kStackLength];
563563
if (stackLength === 0) return false;
564564

565-
if (enabledHooksExist() && async_id_fields[kExecutionAsyncId] !== asyncId) {
565+
if (async_hook_fields[kCheck] > 0 && async_id_fields[kExecutionAsyncId] !== asyncId) {
566566
// Do the same thing as the native code (i.e. crash hard).
567567
return popAsyncContext_(asyncId);
568568
}

lib/internal/process/task_queues.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ const {
2525

2626
const {
2727
getDefaultTriggerAsyncId,
28-
getHookArrays,
28+
enabledHooksExist,
2929
newAsyncId,
3030
initHooksExist,
3131
emitInit,
@@ -160,7 +160,7 @@ function queueMicrotask(callback) {
160160
validateFunction(callback, 'callback');
161161

162162
const contextFrame = AsyncContextFrame.current();
163-
if (contextFrame || getHookArrays()[0].length > 0) {
163+
if (contextFrame || enabledHooksExist()) {
164164
const asyncResource = new AsyncResource(
165165
'Microtask',
166166
defaultMicrotaskResourceOpts,

src/env.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,7 @@ void AsyncHooks::push_async_context(double async_id,
124124
double trigger_async_id,
125125
Local<Object>* resource) {
126126
CHECK_IMPLIES(resource != nullptr, !resource->IsEmpty());
127-
// Since async_hooks is experimental, do only perform the check
128-
// when async_hooks is enabled.
127+
129128
if (fields_[kCheck] > 0) {
130129
CHECK_GE(async_id, -1);
131130
CHECK_GE(trigger_async_id, -1);
@@ -1736,7 +1735,7 @@ AsyncHooks::AsyncHooks(Isolate* isolate, const SerializeInfo* info)
17361735
clear_async_id_stack();
17371736

17381737
// Always perform async_hooks checks, not just when async_hooks is enabled.
1739-
// TODO(AndreasMadsen): Consider removing this for LTS releases.
1738+
// Can be disabled via CLI option --no-force-async-hooks-checks
17401739
// See discussion in https://github.com/nodejs/node/pull/15454
17411740
// When removing this, do it by reverting the commit. Otherwise the test
17421741
// and flag changes won't be included.
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Flags: --expose-internals
2+
'use strict';
3+
4+
require('../common');
5+
const assert = require('assert');
6+
const { createHook } = require('async_hooks');
7+
const { enabledHooksExist } = require('internal/async_hooks');
8+
9+
assert.strictEqual(enabledHooksExist(), false);
10+
11+
const ah = createHook({});
12+
assert.strictEqual(enabledHooksExist(), false);
13+
14+
ah.enable();
15+
assert.strictEqual(enabledHooksExist(), true);
16+
17+
ah.disable();
18+
assert.strictEqual(enabledHooksExist(), false);

0 commit comments

Comments
 (0)