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.

Author: Flarna

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

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

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'));

src/env.cc

@@ -127,8 +127,7 @@ void AsyncHooks::push_async_context(
     std::variant<Local<Object>*, Global<Object>*> 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.

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);

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');
   }));
 }));

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');
 }));
 

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