From e51b8c38e0fb83a1e574e0921738b071273ee392 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerhard=20St=C3=B6bich?= <18708370+Flarna@users.noreply.github.com> Date: Sun, 14 Dec 2025 00:35:54 +0100 Subject: [PATCH 1/2] 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. --- lib/async_hooks.js | 3 +-- lib/internal/async_hooks.js | 4 ++-- lib/internal/streams/end-of-stream.js | 5 ++--- src/env.cc | 5 ++--- .../test-async-hooks-enabledhooksexits.js | 18 ++++++++++++++++++ ...test-stream-finished-async-local-storage.js | 5 ++--- ...t-stream-finished-bindAsyncResource-path.js | 4 ++-- .../test-stream-finished-default-path.js | 6 +++--- 8 files changed, 32 insertions(+), 18 deletions(-) create mode 100644 test/parallel/test-async-hooks-enabledhooksexits.js diff --git a/lib/async_hooks.js b/lib/async_hooks.js index dcb046e13e3388..3e3982a7ac61e5 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -52,7 +52,6 @@ const { emitBefore, emitAfter, emitDestroy, - enabledHooksExist, initHooksExist, destroyHooksExist, } = internal_async_hooks; @@ -188,7 +187,7 @@ class AsyncResource { this[trigger_async_id_symbol] = triggerAsyncId; if (initHooksExist()) { - if (enabledHooksExist() && type.length === 0) { + if (type.length === 0) { throw new ERR_ASYNC_TYPE(type); } diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index d698831a1425c7..71f6bc06414e7d 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -481,7 +481,7 @@ function hasHooks(key) { } function enabledHooksExist() { - return hasHooks(kCheck); + return active_hooks.array.length > 0; } function initHooksExist() { @@ -563,7 +563,7 @@ function popAsyncContext(asyncId) { const stackLength = async_hook_fields[kStackLength]; if (stackLength === 0) return false; - if (enabledHooksExist() && async_id_fields[kExecutionAsyncId] !== asyncId) { + if (async_hook_fields[kCheck] > 0 && async_id_fields[kExecutionAsyncId] !== asyncId) { // Do the same thing as the native code (i.e. crash hard). return popAsyncContext_(asyncId); } diff --git a/lib/internal/streams/end-of-stream.js b/lib/internal/streams/end-of-stream.js index 2d520cee367120..db854aa54f043c 100644 --- a/lib/internal/streams/end-of-stream.js +++ b/lib/internal/streams/end-of-stream.js @@ -45,7 +45,7 @@ const { kIsClosedPromise, } = require('internal/streams/utils'); -const { getHookArrays } = require('internal/async_hooks'); +const { enabledHooksExist } = require('internal/async_hooks'); const AsyncContextFrame = require('internal/async_context_frame'); // Lazy load @@ -78,8 +78,7 @@ function eos(stream, options, callback) { validateFunction(callback, 'callback'); validateAbortSignal(options.signal, 'options.signal'); - if (AsyncContextFrame.current() || - getHookArrays()[0].length > 0) { + if (AsyncContextFrame.current() || enabledHooksExist()) { // Avoid AsyncResource.bind() because it calls ObjectDefineProperties which // is a bottleneck here. callback = once(bindAsyncResource(callback, 'STREAM_END_OF_STREAM')); diff --git a/src/env.cc b/src/env.cc index b89a3811f2dc8f..6df324e04316c5 100644 --- a/src/env.cc +++ b/src/env.cc @@ -127,8 +127,7 @@ void AsyncHooks::push_async_context( std::variant*, Global*> resource) { std::visit([](auto* ptr) { CHECK_IMPLIES(ptr != nullptr, !ptr->IsEmpty()); }, resource); - // Since async_hooks is experimental, do only perform the check - // when async_hooks is enabled. + if (fields_[kCheck] > 0) { CHECK_GE(async_id, -1); CHECK_GE(trigger_async_id, -1); @@ -1756,7 +1755,7 @@ AsyncHooks::AsyncHooks(Isolate* isolate, const SerializeInfo* info) clear_async_id_stack(); // Always perform async_hooks checks, not just when async_hooks is enabled. - // TODO(AndreasMadsen): Consider removing this for LTS releases. + // Can be disabled via CLI option --no-force-async-hooks-checks // See discussion in https://github.com/nodejs/node/pull/15454 // When removing this, do it by reverting the commit. Otherwise the test // and flag changes won't be included. diff --git a/test/parallel/test-async-hooks-enabledhooksexits.js b/test/parallel/test-async-hooks-enabledhooksexits.js new file mode 100644 index 00000000000000..11737d65485b3d --- /dev/null +++ b/test/parallel/test-async-hooks-enabledhooksexits.js @@ -0,0 +1,18 @@ +// Flags: --expose-internals +'use strict'; + +require('../common'); +const assert = require('assert'); +const { createHook } = require('async_hooks'); +const { enabledHooksExist } = require('internal/async_hooks'); + +assert.strictEqual(enabledHooksExist(), false); + +const ah = createHook({}); +assert.strictEqual(enabledHooksExist(), false); + +ah.enable(); +assert.strictEqual(enabledHooksExist(), true); + +ah.disable(); +assert.strictEqual(enabledHooksExist(), false); diff --git a/test/parallel/test-stream-finished-async-local-storage.js b/test/parallel/test-stream-finished-async-local-storage.js index 9ae81aa75325b7..b6d8f6722b05c3 100644 --- a/test/parallel/test-stream-finished-async-local-storage.js +++ b/test/parallel/test-stream-finished-async-local-storage.js @@ -6,7 +6,7 @@ const { Readable, finished } = require('stream'); const { AsyncLocalStorage } = require('async_hooks'); const assert = require('assert'); const AsyncContextFrame = require('internal/async_context_frame'); -const internalAsyncHooks = require('internal/async_hooks'); +const { enabledHooksExist } = require('internal/async_hooks'); // This test verifies that ALS context is preserved when using stream.finished() @@ -15,8 +15,7 @@ const readable = new Readable(); als.run('test-context-1', common.mustCall(() => { finished(readable, common.mustCall(() => { - assert.strictEqual(AsyncContextFrame.enabled || internalAsyncHooks.getHookArrays()[0].length > 0, - true); + assert.strictEqual(AsyncContextFrame.enabled || enabledHooksExist(), true); assert.strictEqual(als.getStore(), 'test-context-1'); })); })); diff --git a/test/parallel/test-stream-finished-bindAsyncResource-path.js b/test/parallel/test-stream-finished-bindAsyncResource-path.js index 98ef2bb3b62ef1..82de2947c29b91 100644 --- a/test/parallel/test-stream-finished-bindAsyncResource-path.js +++ b/test/parallel/test-stream-finished-bindAsyncResource-path.js @@ -5,7 +5,7 @@ const common = require('../common'); const { Readable, finished } = require('stream'); const { createHook, executionAsyncId } = require('async_hooks'); const assert = require('assert'); -const internalAsyncHooks = require('internal/async_hooks'); +const { enabledHooksExist } = require('internal/async_hooks'); // This test verifies that when there are active async hooks, stream.finished() uses // the bindAsyncResource path @@ -27,7 +27,7 @@ const readable = new Readable(); finished(readable, common.mustCall(() => { const currentAsyncId = executionAsyncId(); const ctx = contextMap.get(currentAsyncId); - assert.strictEqual(internalAsyncHooks.getHookArrays()[0].length > 0, true); + assert.ok(enabledHooksExist()); assert.strictEqual(ctx, 'abc-123'); })); diff --git a/test/parallel/test-stream-finished-default-path.js b/test/parallel/test-stream-finished-default-path.js index 7beffc887b2bee..5d992ab657a4aa 100644 --- a/test/parallel/test-stream-finished-default-path.js +++ b/test/parallel/test-stream-finished-default-path.js @@ -5,16 +5,16 @@ const common = require('../common'); const { Readable, finished } = require('stream'); const assert = require('assert'); const AsyncContextFrame = require('internal/async_context_frame'); -const internalAsyncHooks = require('internal/async_hooks'); +const { enabledHooksExist } = require('internal/async_hooks'); // This test verifies that when there are no active async hooks, stream.finished() uses the default callback path const readable = new Readable(); finished(readable, common.mustCall(() => { - assert.strictEqual(internalAsyncHooks.getHookArrays()[0].length === 0, true); + assert.strictEqual(enabledHooksExist(), false); assert.strictEqual( - AsyncContextFrame.current() || internalAsyncHooks.getHookArrays()[0].length > 0, + AsyncContextFrame.current() || enabledHooksExist(), false, ); })); From 474e67ee32a761fc3b5cd32b9f555e6d852b5826 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerhard=20St=C3=B6bich?= <18708370+Flarna@users.noreply.github.com> Date: Mon, 22 Dec 2025 17:08:32 +0100 Subject: [PATCH 2/2] use enabledHooksExist in task_queues --- lib/internal/process/task_queues.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/process/task_queues.js b/lib/internal/process/task_queues.js index 518aea930789e0..84f406cd5e8f54 100644 --- a/lib/internal/process/task_queues.js +++ b/lib/internal/process/task_queues.js @@ -25,7 +25,7 @@ const { const { getDefaultTriggerAsyncId, - getHookArrays, + enabledHooksExist, newAsyncId, initHooksExist, emitInit, @@ -160,7 +160,7 @@ function queueMicrotask(callback) { validateFunction(callback, 'callback'); const contextFrame = AsyncContextFrame.current(); - if (contextFrame || getHookArrays()[0].length > 0) { + if (contextFrame || enabledHooksExist()) { const asyncResource = new AsyncResource( 'Microtask', defaultMicrotaskResourceOpts,