Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 32cd18e

Browse filesBrowse 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
Copy full SHA for 32cd18e

5 files changed

+25-9Lines changed: 25 additions & 9 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎lib/async_hooks.js‎

Copy file name to clipboardExpand all lines: lib/async_hooks.js
+1-2Lines 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

Collapse file

‎lib/internal/async_hooks.js‎

Copy file name to clipboardExpand all lines: lib/internal/async_hooks.js
+2-2Lines 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
}
Collapse file

‎lib/internal/process/task_queues.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/task_queues.js
+2-2Lines 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,
Collapse file

‎src/env.cc‎

Copy file name to clipboardExpand all lines: src/env.cc
+2-3Lines 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.
Collapse file
+18Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.