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/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, 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, ); }));