async_hooks: enabledHooksExist shall return if hooks are enabled · nodejs/node@32cd18e · GitHub

nodejs / node Public

• Notifications You must be signed in to change notification settings
• Fork 36.6k
• Star 120k

• Code
• Issues 633
• Pull requests 750
• Actions
• Projects
• Security and quality
• Insights

Additional navigation options

Commit 32cd18e

Browse filesBrowse the repository at this point in the historyBrowse files

Flarna

authored and

aduh95

committed

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

• lib
  • async_hooks.js
  • internal
    • async_hooks.js
    • process
      • task_queues.js
• src
  • env.cc
• test/parallel
  • test-async-hooks-enabledhooksexits.js

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 number	Diff line number	Diff line change
@@ -52,7 +52,6 @@ const {
52	52	emitBefore,
53	53	emitAfter,
54	54	emitDestroy,
55		- enabledHooksExist,
56	55	initHooksExist,
57	56	destroyHooksExist,
58	57	} = internal_async_hooks;
@@ -188,7 +187,7 @@ class AsyncResource {
188	187	this[trigger_async_id_symbol] = triggerAsyncId;
189	188
190	189	if (initHooksExist()) {
191		- if (enabledHooksExist() && type.length === 0) {
	190	+ if (type.length === 0) {
192	191	throw new ERR_ASYNC_TYPE(type);
193	192	}
194	193

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 number	Diff line number	Diff line change
@@ -481,7 +481,7 @@ function hasHooks(key) {
481	481	}
482	482
483	483	function enabledHooksExist() {
484		- return hasHooks(kCheck);
	484	+ return active_hooks.array.length > 0;
485	485	}
486	486
487	487	function initHooksExist() {
@@ -563,7 +563,7 @@ function popAsyncContext(asyncId) {
563	563	const stackLength = async_hook_fields[kStackLength];
564	564	if (stackLength === 0) return false;
565	565
566		- if (enabledHooksExist() && async_id_fields[kExecutionAsyncId] !== asyncId) {
	566	+ if (async_hook_fields[kCheck] > 0 && async_id_fields[kExecutionAsyncId] !== asyncId) {
567	567	// Do the same thing as the native code (i.e. crash hard).
568	568	return popAsyncContext_(asyncId);
569	569	}

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 number	Diff line number	Diff line change
@@ -25,7 +25,7 @@ const {
25	25
26	26	const {
27	27	getDefaultTriggerAsyncId,
28		- getHookArrays,
	28	+ enabledHooksExist,
29	29	newAsyncId,
30	30	initHooksExist,
31	31	emitInit,
@@ -160,7 +160,7 @@ function queueMicrotask(callback) {
160	160	validateFunction(callback, 'callback');
161	161
162	162	const contextFrame = AsyncContextFrame.current();
163		- if (contextFrame || getHookArrays()[0].length > 0) {
	163	+ if (contextFrame || enabledHooksExist()) {
164	164	const asyncResource = new AsyncResource(
165	165	'Microtask',
166	166	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 number	Diff line number	Diff line change
@@ -127,8 +127,7 @@ void AsyncHooks::push_async_context(
127	127	std::variant<Local<Object>*, Global<Object>*> resource) {
128	128	std::visit([](auto* ptr) { CHECK_IMPLIES(ptr != nullptr, !ptr->IsEmpty()); },
129	129	resource);
130		- // Since async_hooks is experimental, do only perform the check
131		- // when async_hooks is enabled.
	130	+
132	131	if (fields_[kCheck] > 0) {
133	132	CHECK_GE(async_id, -1);
134	133	CHECK_GE(trigger_async_id, -1);
@@ -1748,7 +1747,7 @@ AsyncHooks::AsyncHooks(Isolate* isolate, const SerializeInfo* info)
1748	1747	clear_async_id_stack();
1749	1748
1750	1749	// 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
1752	1751	// See discussion in https://github.com/nodejs/node/pull/15454
1753	1752	// When removing this, do it by reverting the commit. Otherwise the test
1754	1753	// and flag changes won't be included.

Collapse file

‎test/parallel/test-async-hooks-enabledhooksexits.js‎

Copy file name to clipboard

+18Lines changed: 18 additions & 0 deletions

Original file line number	Diff line number	Diff 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)