diff options
| -rw-r--r-- | doc/api/cli.md | 6 | ||||
| -rw-r--r-- | doc/node.1 | 6 | ||||
| -rw-r--r-- | src/env-inl.h | 13 | ||||
| -rw-r--r-- | src/env.h | 2 | ||||
| -rw-r--r-- | src/node.cc | 16 | ||||
| -rw-r--r-- | test/async-hooks/test-force-checks-flag.js | 25 | ||||
| -rw-r--r-- | test/async-hooks/test-no-assert-when-disabled.js | 10 | ||||
| -rw-r--r-- | test/common/index.js | 11 |
8 files changed, 26 insertions, 63 deletions
diff --git a/doc/api/cli.md b/doc/api/cli.md index b6d778f5ac..c0fd8f5761 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -208,13 +208,13 @@ added: v2.1.0 Prints a stack trace whenever synchronous I/O is detected after the first turn of the event loop. -### `--force-async-hooks-checks` +### `--no-force-async-hooks-checks` <!-- YAML added: REPLACEME --> -Enables runtime checks for `async_hooks`. These can also be enabled dynamically -by enabling one of the `async_hooks` hooks. +Disables runtime checks for `async_hooks`. These will still be enabled +dynamically when `async_hooks` is enabled. ### `--trace-events-enabled` <!-- YAML diff --git a/doc/node.1 b/doc/node.1 index 1d162b95de..0fad250c6d 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -153,9 +153,9 @@ Print a stack trace whenever synchronous I/O is detected after the first turn of the event loop. .TP -.BR \-\-force\-async\-hooks\-checks -Enables runtime checks for `async_hooks`. These can also be enabled dynamically -by enabling one of the `async_hooks` hooks. +.BR \-\-no\-force\-async\-hooks\-checks +Disables runtime checks for `async_hooks`. These will still be enabled +dynamically when `async_hooks` is enabled. .TP .BR \-\-trace\-events\-enabled diff --git a/src/env-inl.h b/src/env-inl.h index e6e189be3d..bd13881cb1 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -88,6 +88,13 @@ inline Environment::AsyncHooks::AsyncHooks(v8::Isolate* isolate) async_id_fields_(isolate, kUidFieldsCount) { v8::HandleScope handle_scope(isolate_); + // Always perform async_hooks checks, not just when async_hooks is enabled. + // TODO(AndreasMadsen): Consider removing this for LTS releases. + // 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. + fields_[kCheck] = 1; + // kAsyncIdCounter should start at 1 because that'll be the id the execution // context during bootstrap (code that runs before entering uv_run()). async_id_fields_[AsyncHooks::kAsyncIdCounter] = 1; @@ -129,9 +136,9 @@ inline v8::Local<v8::String> Environment::AsyncHooks::provider_string(int idx) { return providers_[idx].Get(isolate_); } -inline void Environment::AsyncHooks::force_checks() { - // fields_ does not have the += operator defined - fields_[kCheck] = fields_[kCheck] + 1; +inline void Environment::AsyncHooks::no_force_checks() { + // fields_ does not have the -= operator defined + fields_[kCheck] = fields_[kCheck] - 1; } inline void Environment::AsyncHooks::push_async_ids(double async_id, @@ -410,7 +410,7 @@ class Environment { inline v8::Local<v8::String> provider_string(int idx); - inline void force_checks(); + inline void no_force_checks(); inline void push_async_ids(double async_id, double trigger_async_id); inline bool pop_async_id(double async_id); diff --git a/src/node.cc b/src/node.cc index b780d52653..2039edfeb9 100644 --- a/src/node.cc +++ b/src/node.cc @@ -173,7 +173,7 @@ static bool syntax_check_only = false; static bool trace_deprecation = false; static bool throw_deprecation = false; static bool trace_sync_io = false; -static bool force_async_hooks_checks = false; +static bool no_force_async_hooks_checks = false; static bool track_heap_objects = false; static const char* eval_string = nullptr; static std::vector<std::string> preload_modules; @@ -3899,8 +3899,8 @@ static void PrintHelp() { " stderr\n" " --trace-sync-io show stack trace when use of sync IO\n" " is detected after the first tick\n" - " --force-async-hooks-checks\n" - " enables checks for async_hooks\n" + " --no-force-async-hooks-checks\n" + " disable checks for async_hooks\n" " --trace-events-enabled track trace events\n" " --trace-event-categories comma separated list of trace event\n" " categories to record\n" @@ -4026,7 +4026,7 @@ static void CheckIfAllowedInEnv(const char* exe, bool is_env, "--trace-warnings", "--redirect-warnings", "--trace-sync-io", - "--force-async-hooks-checks", + "--no-force-async-hooks-checks", "--trace-events-enabled", "--trace-events-categories", "--track-heap-objects", @@ -4165,8 +4165,8 @@ static void ParseArgs(int* argc, trace_deprecation = true; } else if (strcmp(arg, "--trace-sync-io") == 0) { trace_sync_io = true; - } else if (strcmp(arg, "--force-async-hooks-checks") == 0) { - force_async_hooks_checks = true; + } else if (strcmp(arg, "--no-force-async-hooks-checks") == 0) { + no_force_async_hooks_checks = true; } else if (strcmp(arg, "--trace-events-enabled") == 0) { trace_enabled = true; } else if (strcmp(arg, "--trace-event-categories") == 0) { @@ -4815,8 +4815,8 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data, env.set_abort_on_uncaught_exception(abort_on_uncaught_exception); - if (force_async_hooks_checks) { - env.async_hooks()->force_checks(); + if (no_force_async_hooks_checks) { + env.async_hooks()->no_force_checks(); } { diff --git a/test/async-hooks/test-force-checks-flag.js b/test/async-hooks/test-force-checks-flag.js deleted file mode 100644 index daafd56778..0000000000 --- a/test/async-hooks/test-force-checks-flag.js +++ /dev/null @@ -1,25 +0,0 @@ -'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-no-assert-when-disabled.js b/test/async-hooks/test-no-assert-when-disabled.js index b3822dde3a..47aafe52c3 100644 --- a/test/async-hooks/test-no-assert-when-disabled.js +++ b/test/async-hooks/test-no-assert-when-disabled.js @@ -1,18 +1,10 @@ 'use strict'; -// Flags: --expose-internals +// Flags: --no-force-async-hooks-checks --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 diff --git a/test/common/index.js b/test/common/index.js index 04f0c79b2f..838c7b60da 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -65,17 +65,6 @@ 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 = {}; |