From b229083235237dca999b50c4f306b657e64f924b Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 25 Sep 2024 14:36:52 +0200 Subject: [PATCH] src: parse --stack-trace-limit and use it in --trace-* flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, --trace-exit and --trace-sync-io doesn't take care of --stack-trace-limit and always print a stack trace with maximum size of 10. This patch parses --stack-trace-limit during initialization and use the value in --trace-* flags. PR-URL: https://github.com/nodejs/node/pull/55121 Fixes: https://github.com/nodejs/node/issues/55100 Reviewed-By: Michaël Zasso Reviewed-By: Chengzhong Wu --- src/env-inl.h | 4 ++ src/env.cc | 16 +++++--- src/env.h | 2 +- src/node_options-inl.h | 7 +++- src/node_options.cc | 10 ++++- src/node_options.h | 1 + test/fixtures/deep-exit.js | 15 +++++++ test/parallel/test-trace-exit-stack-limit.js | 42 ++++++++++++++++++++ 8 files changed, 88 insertions(+), 9 deletions(-) create mode 100644 test/fixtures/deep-exit.js create mode 100644 test/parallel/test-trace-exit-stack-limit.js diff --git a/src/env-inl.h b/src/env-inl.h index 3acc38020496fc..a9784eda52bc47 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -458,6 +458,10 @@ inline double Environment::get_default_trigger_async_id() { return default_trigger_async_id; } +inline int64_t Environment::stack_trace_limit() const { + return isolate_data_->options()->stack_trace_limit; +} + inline std::shared_ptr Environment::options() { return options_; } diff --git a/src/env.cc b/src/env.cc index 0cf593b8e057e9..1c09399a9c4dec 100644 --- a/src/env.cc +++ b/src/env.cc @@ -1252,9 +1252,11 @@ void Environment::PrintSyncTrace() const { fprintf( stderr, "(node:%d) WARNING: Detected use of sync API\n", uv_os_getpid()); - PrintStackTrace(isolate(), - StackTrace::CurrentStackTrace( - isolate(), stack_trace_limit(), StackTrace::kDetailed)); + PrintStackTrace( + isolate(), + StackTrace::CurrentStackTrace(isolate(), + static_cast(stack_trace_limit()), + StackTrace::kDetailed)); } MaybeLocal Environment::RunSnapshotSerializeCallback() const { @@ -1856,9 +1858,11 @@ void Environment::Exit(ExitCode exit_code) { fprintf(stderr, "WARNING: Exited the environment with code %d\n", static_cast(exit_code)); - PrintStackTrace(isolate(), - StackTrace::CurrentStackTrace( - isolate(), stack_trace_limit(), StackTrace::kDetailed)); + PrintStackTrace( + isolate(), + StackTrace::CurrentStackTrace(isolate(), + static_cast(stack_trace_limit()), + StackTrace::kDetailed)); } process_exit_handler_(this, exit_code); } diff --git a/src/env.h b/src/env.h index ea043744e1aea2..55124cd38e75ab 100644 --- a/src/env.h +++ b/src/env.h @@ -981,7 +981,7 @@ class Environment final : public MemoryRetainer { inline std::shared_ptr options(); inline std::shared_ptr> inspector_host_port(); - inline int32_t stack_trace_limit() const { return 10; } + inline int64_t stack_trace_limit() const; #if HAVE_INSPECTOR void set_coverage_connection( diff --git a/src/node_options-inl.h b/src/node_options-inl.h index fabf3be149be93..24954e0b583834 100644 --- a/src/node_options-inl.h +++ b/src/node_options-inl.h @@ -447,9 +447,14 @@ void OptionsParser::Parse( case kBoolean: *Lookup(info.field, options) = !is_negation; break; - case kInteger: + case kInteger: { + // Special case to pass --stack-trace-limit down to V8. + if (name == "--stack-trace-limit") { + v8_args->push_back(arg); + } *Lookup(info.field, options) = std::atoll(value.c_str()); break; + } case kUInteger: *Lookup(info.field, options) = std::strtoull(value.c_str(), nullptr, 10); diff --git a/src/node_options.cc b/src/node_options.cc index be8edf00f09d7b..d3b59690e917af 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -921,7 +921,10 @@ PerIsolateOptionsParser::PerIsolateOptionsParser( "--perf-basic-prof-only-functions", "", V8Option{}, kAllowedInEnvvar); AddOption("--perf-prof", "", V8Option{}, kAllowedInEnvvar); AddOption("--perf-prof-unwinding-info", "", V8Option{}, kAllowedInEnvvar); - AddOption("--stack-trace-limit", "", V8Option{}, kAllowedInEnvvar); + AddOption("--stack-trace-limit", + "", + &PerIsolateOptions::stack_trace_limit, + kAllowedInEnvvar); AddOption("--disallow-code-generation-from-strings", "disallow eval and friends", V8Option{}, @@ -1313,6 +1316,11 @@ void GetCLIOptionsValues(const FunctionCallbackInfo& args) { if (item.first == "--abort-on-uncaught-exception") { value = Boolean::New(isolate, s.original_per_env->abort_on_uncaught_exception); + } else if (item.first == "--stack-trace-limit") { + value = + Number::New(isolate, + static_cast( + *_ppop_instance.Lookup(field, opts))); } else { value = undefined_value; } diff --git a/src/node_options.h b/src/node_options.h index 0f9f72705ec633..fc7f898a6b9b60 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -273,6 +273,7 @@ class PerIsolateOptions : public Options { bool report_uncaught_exception = false; bool report_on_signal = false; bool experimental_shadow_realm = false; + int64_t stack_trace_limit = 10; std::string report_signal = "SIGUSR2"; bool build_snapshot = false; std::string build_snapshot_config; diff --git a/test/fixtures/deep-exit.js b/test/fixtures/deep-exit.js new file mode 100644 index 00000000000000..357137a279c556 --- /dev/null +++ b/test/fixtures/deep-exit.js @@ -0,0 +1,15 @@ +'use strict'; + +// This is meant to be run with --trace-exit. + +const depth = parseInt(process.env.STACK_DEPTH) || 30; +let counter = 1; +function recurse() { + if (counter++ < depth) { + recurse(); + } else { + process.exit(0); + } +} + +recurse(); diff --git a/test/parallel/test-trace-exit-stack-limit.js b/test/parallel/test-trace-exit-stack-limit.js new file mode 100644 index 00000000000000..c937ad828fc032 --- /dev/null +++ b/test/parallel/test-trace-exit-stack-limit.js @@ -0,0 +1,42 @@ +'use strict'; + +// This tests that --stack-trace-limit can be used to tweak the stack trace size of --trace-exit. +require('../common'); +const fixture = require('../common/fixtures'); +const { spawnSyncAndAssert } = require('../common/child_process'); +const assert = require('assert'); + +// When the stack trace limit is bigger than the stack trace size, it should output them all. +spawnSyncAndAssert( + process.execPath, + ['--trace-exit', '--stack-trace-limit=50', fixture.path('deep-exit.js')], + { + env: { + ...process.env, + STACK_DEPTH: 30 + } + }, + { + stderr(output) { + const matches = [...output.matchAll(/at recurse/g)]; + assert.strictEqual(matches.length, 30); + } + }); + +// When the stack trace limit is smaller than the stack trace size, it should truncate the stack size. +spawnSyncAndAssert( + process.execPath, + ['--trace-exit', '--stack-trace-limit=30', fixture.path('deep-exit.js')], + { + env: { + ...process.env, + STACK_DEPTH: 30 + } + }, + { + stderr(output) { + const matches = [...output.matchAll(/at recurse/g)]; + // The top frame is process.exit(), so one frame from recurse() is truncated. + assert.strictEqual(matches.length, 29); + } + });