FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

report: handle on-fatalerror better · nodejs/node@28e298f · GitHub

/ node Public

Commit 28e298f

Browse files
HarshithaKP
authored and
MylesBorins
committed
report: handle on-fatalerror better
--report-on-fatalerror was not honored properly, as there was no way to check the value which was stored in the Environment pointer which can be inaccessible under certain fatal error situations. Move the flag out of Environment pointer so that this is doable. Co-authored-by: Shobhit Chittora schittora@paypal.com PR-URL: #32207 Fixes: #31576 Refs: #29881 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
1 parent b7a8878 commit 28e298f

5 files changed

Lines changed: 31 additions & 22 deletions

File tree

‎src/node_errors.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ void OnFatalError(const char* location, const char* message) {
406406

407407
Isolate* isolate = Isolate::GetCurrent();
408408
Environment* env = Environment::GetCurrent(isolate);
409-
if (env == nullptr || env->isolate_data()->options()->report_on_fatalerror) {
409+
if (per_process::cli_options->report_on_fatalerror) {
410410
report::TriggerNodeReport(
411411
isolate, env, message, "FatalError", "", Local<String>());
412412
}

‎src/node_options.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -590,10 +590,6 @@ PerIsolateOptionsParser::PerIsolateOptionsParser(
590590
"generate diagnostic report upon receiving signals",
591591
&PerIsolateOptions::report_on_signal,
592592
kAllowedInEnvironment);
593-
AddOption("--report-on-fatalerror",
594-
"generate diagnostic report on fatal (internal) errors",
595-
&PerIsolateOptions::report_on_fatalerror,
596-
kAllowedInEnvironment);
597593
AddOption("--report-signal",
598594
"causes diagnostic report to be produced on provided signal,"
599595
" unsupported in Windows. (default: SIGUSR2)",
@@ -667,6 +663,10 @@ PerProcessOptionsParser::PerProcessOptionsParser(
667663
AddOption("--v8-options",
668664
"print V8 command line options",
669665
&PerProcessOptions::print_v8_help);
666+
AddOption("--report-on-fatalerror",
667+
"generate diagnostic report on fatal (internal) errors",
668+
&PerProcessOptions::report_on_fatalerror,
669+
kAllowedInEnvironment);
670670

671671
#ifdef NODE_HAVE_I18N_SUPPORT
672672
AddOption("--icu-data-dir",

‎src/node_options.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,6 @@ class PerIsolateOptions : public Options {
186186
bool no_node_snapshot = false;
187187
bool report_uncaught_exception = false;
188188
bool report_on_signal = false;
189-
bool report_on_fatalerror = false;
190189
bool report_compact = false;
191190
std::string report_signal = "SIGUSR2";
192191
std::string report_filename;
@@ -236,6 +235,7 @@ class PerProcessOptions : public Options {
236235
std::string use_largepages = "off";
237236
bool trace_sigint = false;
238237
std::vector<std::string> cmdline;
238+
bool report_on_fatalerror = false;
239239

240240
inline PerIsolateOptions* get_per_isolate_options();
241241
void CheckOptions(std::vector<std::string>* errors) override;

‎src/node_report_module.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,13 +131,13 @@ static void SetSignal(const FunctionCallbackInfo<Value>& info) {
131131
static void ShouldReportOnFatalError(const FunctionCallbackInfo<Value>& info) {
132132
Environment* env = Environment::GetCurrent(info);
133133
info.GetReturnValue().Set(
134-
env->isolate_data()->options()->report_on_fatalerror);
134+
node::per_process::cli_options->report_on_fatalerror);
135135
}
136136

137137
static void SetReportOnFatalError(const FunctionCallbackInfo<Value>& info) {
138138
Environment* env = Environment::GetCurrent(info);
139139
CHECK(info[0]->IsBoolean());
140-
env->isolate_data()->options()->report_on_fatalerror = info[0]->IsTrue();
140+
node::per_process::cli_options->report_on_fatalerror = info[0]->IsTrue();
141141
}
142142

143143
static void ShouldReportOnSignal(const FunctionCallbackInfo<Value>& info) {

‎test/report/test-report-fatal-error.js‎

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use strict';
22

3-
const common = require('../common');
3+
require('../common');
44
const assert = require('assert');
55
// Testcase to produce report on fatal error (javascript heap OOM)
66
if (process.argv[2] === 'child') {
@@ -20,17 +20,26 @@ if (process.argv[2] === 'child') {
2020
const helper = require('../common/report.js');
2121
const tmpdir = require('../common/tmpdir');
2222
tmpdir.refresh();
23-
const spawn = require('child_process').spawn;
24-
const args = ['--report-on-fatalerror',
25-
'--max-old-space-size=20',
26-
__filename,
27-
'child'];
28-
const child = spawn(process.execPath, args, { cwd: tmpdir.path });
29-
child.on('exit', common.mustCall((code) => {
30-
assert.notStrictEqual(code, 0, 'Process exited unexpectedly');
31-
const reports = helper.findReports(child.pid, tmpdir.path);
32-
assert.strictEqual(reports.length, 1);
33-
const report = reports[0];
34-
helper.validate(report);
35-
}));
23+
const spawnSync = require('child_process').spawnSync;
24+
let args = ['--report-on-fatalerror',
25+
'--max-old-space-size=20',
26+
__filename,
27+
'child'];
28+
29+
let child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
30+
31+
assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
32+
let reports = helper.findReports(child.pid, tmpdir.path);
33+
assert.strictEqual(reports.length, 1);
34+
const report = reports[0];
35+
helper.validate(report);
36+
// Verify that reports are not created on fatal error by default.
37+
args = ['--max-old-space-size=20',
38+
__filename,
39+
'child'];
40+
41+
child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
42+
assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
43+
reports = helper.findReports(child.pid, tmpdir.path);
44+
assert.strictEqual(reports.length, 0);
3645
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL