Skip to content

Commit 32cd18e

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 a49d543 commit 32cd18e

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
@@ -52,7 +52,6 @@ const {
5252
emitBefore,
5353
emitAfter,
5454
emitDestroy,
55-
enabledHooksExist,
5655
initHooksExist,
5756
destroyHooksExist,
5857
} = internal_async_hooks;
@@ -188,7 +187,7 @@ class AsyncResource {
188187
this[trigger_async_id_symbol] = triggerAsyncId;
189188

190189
if (initHooksExist()) {
191-
if (enabledHooksExist() && type.length === 0) {
190+
if (type.length === 0) {
192191
throw new ERR_ASYNC_TYPE(type);
193192
}
194193

lib/internal/async_hooks.js

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

483483
function enabledHooksExist() {
484-
return hasHooks(kCheck);
484+
return active_hooks.array.length > 0;
485485
}
486486

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

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

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
@@ -127,8 +127,7 @@ void AsyncHooks::push_async_context(
127127
std::variant<Local<Object>*, Global<Object>*> resource) {
128128
std::visit([](auto* ptr) { CHECK_IMPLIES(ptr != nullptr, !ptr->IsEmpty()); },
129129
resource);
130-
// Since async_hooks is experimental, do only perform the check
131-
// when async_hooks is enabled.
130+
132131
if (fields_[kCheck] > 0) {
133132
CHECK_GE(async_id, -1);
134133
CHECK_GE(trigger_async_id, -1);
@@ -1748,7 +1747,7 @@ AsyncHooks::AsyncHooks(Isolate* isolate, const SerializeInfo* info)
17481747
clear_async_id_stack();
17491748

17501749
// Always perform async_hooks checks, not just when async_hooks is enabled.
1751-
// TODO(AndreasMadsen): Consider removing this for LTS releases.
1750+
// Can be disabled via CLI option --no-force-async-hooks-checks
17521751
// See discussion in https://github.com/nodejs/node/pull/15454
17531752
// When removing this, do it by reverting the commit. Otherwise the test
17541753
// 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)