async_hooks: skip runtime checks when disabled

PR-URL: https://github.com/nodejs/node/pull/15454
Ref: https://github.com/nodejs/node/pull/14387
Ref: https://github.com/nodejs/node/pull/14722
Ref: https://github.com/nodejs/node/issues/14717
Ref: https://github.com/nodejs/node/issues/15448
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

6 files changed, 101 insertions, 42 deletions

diff --git a/test/async-hooks/test-emit-init.js b/test/async-hooks/test-emit-init.js
index 631dcd75996..9c61f19dab7 100644
--- a/test/async-hooks/test-emit-init.js
+++ b/test/async-hooks/test-emit-init.js
@@ -2,13 +2,10 @@
 
 const common = require('../common');
 const assert = require('assert');
+const spawnSync = require('child_process').spawnSync;
 const async_hooks = require('async_hooks');
 const initHooks = require('./init-hooks');
 
-// Verify that if there is no registered hook, then those invalid parameters
-// won't be checked.
-assert.doesNotThrow(() => async_hooks.emitInit());
-
 const expectedId = async_hooks.newUid();
 const expectedTriggerId = async_hooks.newUid();
 const expectedType = 'test_emit_init_type';
@@ -25,24 +22,40 @@ const hooks1 = initHooks({
 
 hooks1.enable();
 
-assert.throws(() => {
-  async_hooks.emitInit();
-}, common.expectsError({
-  code: 'ERR_INVALID_ASYNC_ID',
-  type: RangeError,
-}));
-assert.throws(() => {
-  async_hooks.emitInit(expectedId);
-}, common.expectsError({
-  code: 'ERR_INVALID_ASYNC_ID',
-  type: RangeError,
-}));
-assert.throws(() => {
-  async_hooks.emitInit(expectedId, expectedType, -2);
-}, common.expectsError({
-  code: 'ERR_INVALID_ASYNC_ID',
-  type: RangeError,
-}));
+switch (process.argv[2]) {
+  case 'test_invalid_async_id':
+    async_hooks.emitInit();
+    return;
+  case 'test_invalid_trigger_id':
+    async_hooks.emitInit(expectedId);
+    return;
+  case 'test_invalid_trigger_id_negative':
+    async_hooks.emitInit(expectedId, expectedType, -2);
+    return;
+}
+assert.ok(!process.argv[2]);
+
+
+const c1 = spawnSync(process.execPath, [__filename, 'test_invalid_async_id']);
+assert.strictEqual(
+  c1.stderr.toString().split(/[\r\n]+/g)[0],
+  'RangeError [ERR_INVALID_ASYNC_ID]: Invalid asyncId value: undefined');
+assert.strictEqual(c1.status, 1);
+
+const c2 = spawnSync(process.execPath, [__filename, 'test_invalid_trigger_id']);
+assert.strictEqual(
+  c2.stderr.toString().split(/[\r\n]+/g)[0],
+  'RangeError [ERR_INVALID_ASYNC_ID]: Invalid triggerAsyncId value: undefined');
+assert.strictEqual(c2.status, 1);
+
+const c3 = spawnSync(process.execPath, [
+  __filename, 'test_invalid_trigger_id_negative'
+]);
+assert.strictEqual(
+  c3.stderr.toString().split(/[\r\n]+/g)[0],
+  'RangeError [ERR_INVALID_ASYNC_ID]: Invalid triggerAsyncId value: -2');
+assert.strictEqual(c3.status, 1);
+
 
 async_hooks.emitInit(expectedId, expectedType, expectedTriggerId,
                      expectedResource);
diff --git a/test/async-hooks/test-force-checks-flag.js b/test/async-hooks/test-force-checks-flag.js
new file mode 100644
index 00000000000..daafd56778b
--- /dev/null
+++ b/test/async-hooks/test-force-checks-flag.js
@@ -0,0 +1,25 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const async_hooks = require('async_hooks');
+const spawnSync = require('child_process').spawnSync;
+
+// disabling this way will just decrement the kCheck counter. It won't actually
+// disable the checks, because they were enabled with the flag.
+common.revert_force_async_hooks_checks();
+
+switch (process.argv[2]) {
+  case 'test_invalid_async_id':
+    async_hooks.emitInit();
+    return;
+}
+assert.ok(!process.argv[2]);
+
+
+const c1 = spawnSync(process.execPath, [
+  '--force-async-hooks-checks', __filename, 'test_invalid_async_id'
+]);
+assert.strictEqual(
+  c1.stderr.toString().split(/[\r\n]+/g)[0],
+  'RangeError [ERR_INVALID_ASYNC_ID]: Invalid asyncId value: undefined');
+assert.strictEqual(c1.status, 1);
diff --git a/test/async-hooks/test-internal-nexttick-default-trigger.js b/test/async-hooks/test-internal-nexttick-default-trigger.js
index cd30830f1c0..ad352a8c147 100644
--- a/test/async-hooks/test-internal-nexttick-default-trigger.js
+++ b/test/async-hooks/test-internal-nexttick-default-trigger.js
@@ -26,11 +26,6 @@ internal.nextTick(null, common.mustCall(function() {
   assert.strictEqual(async_hooks.triggerAsyncId(), rootAsyncId);
 }));
 
-// internal default
-internal.nextTick(undefined, common.mustCall(function() {
-  assert.strictEqual(async_hooks.triggerAsyncId(), rootAsyncId);
-}));
-
 // internal
 internal.nextTick(rootAsyncId + 1, common.mustCall(function() {
   assert.strictEqual(async_hooks.triggerAsyncId(), rootAsyncId + 1);
diff --git a/test/async-hooks/test-no-assert-when-disabled.js b/test/async-hooks/test-no-assert-when-disabled.js
new file mode 100644
index 00000000000..b3822dde3ae
--- /dev/null
+++ b/test/async-hooks/test-no-assert-when-disabled.js
@@ -0,0 +1,26 @@
+'use strict';
+// Flags: --expose-internals
+const common = require('../common');
+
+const async_hooks = require('async_hooks');
+const internal = require('internal/process/next_tick');
+
+// In tests async_hooks dynamic checks are enabled by default. To verify
+// that no checks are enabled ordinarily disable the checks in this test.
+common.revert_force_async_hooks_checks();
+
+// When async_hooks is diabled (or never enabled), the checks
+// should be disabled as well. This is important while async_hooks is
+// experimental and there are still critial bugs to fix.
+
+// Using undefined as the triggerAsyncId.
+// Ref: https://github.com/nodejs/node/issues/14386
+// Ref: https://github.com/nodejs/node/issues/14381
+// Ref: https://github.com/nodejs/node/issues/14368
+internal.nextTick(undefined, common.mustCall());
+
+// Negative asyncIds and invalid type name
+async_hooks.emitInit(-1, null, -1, {});
+async_hooks.emitBefore(-1, -1);
+async_hooks.emitAfter(-1);
+async_hooks.emitDestroy(-1);
diff --git a/test/common/index.js b/test/common/index.js
index 1a80da6d19d..a138b4d3da0 100644
--- a/test/common/index.js
+++ b/test/common/index.js
@@ -63,6 +63,17 @@ exports.projectDir = path.resolve(__dirname, '..', '..');
 
 exports.buildType = process.config.target_defaults.default_configuration;
 
+// Always enable async_hooks checks in tests
+{
+  const async_wrap = process.binding('async_wrap');
+  const { kCheck } = async_wrap.constants;
+  async_wrap.async_hook_fields[kCheck] += 1;
+
+  exports.revert_force_async_hooks_checks = function() {
+    async_wrap.async_hook_fields[kCheck] -= 1;
+  };
+}
+
 // If env var is set then enable async_hook hooks for all tests.
 if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) {
   const destroydIdsList = {};
diff --git a/test/sequential/test-inspector-async-hook-teardown-at-debug-end.js b/test/sequential/test-inspector-async-hook-teardown-at-debug-end.js
index 9084efdd412..c5577363fd6 100644
--- a/test/sequential/test-inspector-async-hook-teardown-at-debug-end.js
+++ b/test/sequential/test-inspector-async-hook-teardown-at-debug-end.js
@@ -7,23 +7,12 @@ const spawn = require('child_process').spawn;
 
 const script = `
 const assert = require('assert');
+const async_wrap = process.binding('async_wrap');
+const { kTotals } = async_wrap.constants;
 
-// Verify that inspector-async-hook is registered
-// by checking that emitInit with invalid arguments
-// throw an error.
-// See test/async-hooks/test-emit-init.js
-assert.throws(
-  () => async_hooks.emitInit(),
-  'inspector async hook should have been enabled initially');
-
+assert.strictEqual(async_wrap.async_hook_fields[kTotals], 4);
 process._debugEnd();
-
-// Verify that inspector-async-hook is no longer registered,
-// thus emitInit() ignores invalid arguments
-// See test/async-hooks/test-emit-init.js
-assert.doesNotThrow(
-  () => async_hooks.emitInit(),
-  'inspector async hook should have beend disabled by _debugEnd()');
+assert.strictEqual(async_wrap.async_hook_fields[kTotals], 0);
 `;
 
 const args = ['--inspect', '-e', script];