Nodejs/node
1
0
Fork 0
mirror of https://github.com/nodejs/node.git synced 2024-11-21 10:59:27 +00:00
Code Issues Actions 25 Packages Projects Releases Wiki Activity

report: fix missing section javascriptHeap on OOMError

Browse Source 
`Environment::GetCurrent` may not available in the context of OOM.
Removes the cyclic `Environment::GetCurrent` and `env->isolate()`
calls to ensure both `isolate` and `env` is present if available.

However, this behavior is not guaranteed. As
`Environment::GetCurrent` didn't allocate new handles in the heap,
when a Context is entered it can still get the valid env pointer.
Removes the unstable assertion of the absence of env in the test.

PR-URL: https://github.com/nodejs/node/pull/44398
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
This commit is contained in:
legendecas 2022-08-26 00:20:07 +08:00
parent d0f73d383d
commit 7f496fefb6
6 changed files with 55 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
src/node_report.cc
View File

@ -784,21 +784,8 @@ static void PrintRelease(JSONWriter* writer) {
} // namespace report
// External function to trigger a report, writing to file.
std::string TriggerNodeReport(Isolate* isolate,
const char* message,
const char* trigger,
const std::string& name,
Local<Value> error) {
Environment* env = nullptr;
if (isolate != nullptr) {
env = Environment::GetCurrent(isolate);
}
return TriggerNodeReport(env, message, trigger, name, error);
}
// External function to trigger a report, writing to file.
std::string TriggerNodeReport(Environment* env,
Environment* env,
const char* message,
const char* trigger,
const std::string& name,
@ -868,10 +855,6 @@ std::string TriggerNodeReport(Environment* env,
compact = per_process::cli_options->report_compact;
}
Isolate* isolate = nullptr;
if (env != nullptr) {
isolate = env->isolate();
}
report::WriteNodeReport(
isolate, env, message, trigger, filename, *outstream, error, compact);
@ -887,6 +870,33 @@ std::string TriggerNodeReport(Environment* env,
return filename;
}
// External function to trigger a report, writing to file.
std::string TriggerNodeReport(Isolate* isolate,
const char* message,
const char* trigger,
const std::string& name,
Local<Value> error) {
Environment* env = nullptr;
if (isolate != nullptr) {
env = Environment::GetCurrent(isolate);
}
return TriggerNodeReport(isolate, env, message, trigger, name, error);
}
// External function to trigger a report, writing to file.
std::string TriggerNodeReport(Environment* env,
const char* message,
const char* trigger,
const std::string& name,
Local<Value> error) {
return TriggerNodeReport(env != nullptr ? env->isolate() : nullptr,
env,
message,
trigger,
name,
error);
}
// External function to trigger a report, writing to a supplied stream.
void GetNodeReport(Isolate* isolate,
const char* message,

10
test/report/test-report-fatalerror-oomerror-compact.js
View File

@ -12,9 +12,13 @@ const fixtures = require('../common/fixtures');
// Common args that will cause an out-of-memory error for child process.
const ARGS = [
'--max-old-space-size=20',
'--max-heap-size=20',
fixtures.path('report-oom'),
];
const REPORT_FIELDS = [
['header.trigger', 'OOMError'],
['javascriptHeap.memoryLimit', 20 * 1024 * 1024 /* 20MB */],
];
{
// Verify that --report-compact is respected when set.
@ -27,8 +31,8 @@ const ARGS = [
assert.strictEqual(reports.length, 1);
const report = reports[0];
helper.validate(report);
assert.strictEqual(require(report).header.threadId, null);
helper.validate(report, REPORT_FIELDS);
// Subtract 1 because "xx\n".split("\n") => [ 'xx', '' ].
const lines = fs.readFileSync(report, 'utf8').split('\n').length - 1;
assert.strictEqual(lines, 1);

10
test/report/test-report-fatalerror-oomerror-directory.js
View File

@ -12,9 +12,13 @@ const fixtures = require('../common/fixtures');
// Common args that will cause an out-of-memory error for child process.
const ARGS = [
'--max-old-space-size=20',
'--max-heap-size=20',
fixtures.path('report-oom'),
];
const REPORT_FIELDS = [
['header.trigger', 'OOMError'],
['javascriptHeap.memoryLimit', 20 * 1024 * 1024 /* 20MB */],
];
{
// Verify that --report-directory is respected when set.
@ -29,8 +33,8 @@ const ARGS = [
assert.strictEqual(reports.length, 1);
const report = reports[0];
helper.validate(report);
assert.strictEqual(require(report).header.threadId, null);
helper.validate(report, REPORT_FIELDS);
const lines = fs.readFileSync(report, 'utf8').split('\n').length - 1;
assert(lines > 10);
}

10
test/report/test-report-fatalerror-oomerror-filename.js
View File

@ -11,9 +11,13 @@ const fixtures = require('../common/fixtures');
// Common args that will cause an out-of-memory error for child process.
const ARGS = [
'--max-old-space-size=20',
'--max-heap-size=20',
fixtures.path('report-oom'),
];
const REPORT_FIELDS = [
['header.trigger', 'OOMError'],
['javascriptHeap.memoryLimit', 20 * 1024 * 1024 /* 20MB */],
];
{
// Verify that --report-compact is respected when set.
@ -34,7 +38,5 @@ const ARGS = [
const lines = child.stderr.split('\n');
// Skip over unavoidable free-form output and gc log from V8.
const report = lines.find((i) => i.startsWith('{'));
const json = JSON.parse(report);
assert.strictEqual(json.header.threadId, null);
helper.validateContent(report, REPORT_FIELDS);
}

2
test/report/test-report-fatalerror-oomerror-not-set.js
View File

@ -11,7 +11,7 @@ const fixtures = require('../common/fixtures');
// Common args that will cause an out-of-memory error for child process.
const ARGS = [
'--max-old-space-size=20',
'--max-heap-size=20',
fixtures.path('report-oom'),
];

15
test/report/test-report-fatalerror-oomerror-set.js
View File

@ -11,9 +11,13 @@ const fixtures = require('../common/fixtures');
// Common args that will cause an out-of-memory error for child process.
const ARGS = [
'--max-old-space-size=20',
'--max-heap-size=20',
fixtures.path('report-oom'),
];
const REPORT_FIELDS = [
['header.trigger', 'OOMError'],
['javascriptHeap.memoryLimit', 20 * 1024 * 1024 /* 20MB */],
];
{
// Verify that --report-on-fatalerror is respected when set.
@ -26,12 +30,5 @@ const ARGS = [
assert.strictEqual(reports.length, 1);
const report = reports[0];
helper.validate(report);
const content = require(report);
// Errors occur in a context where env is not available, so thread ID is
// unknown. Assert this, to verify that the underlying env-less situation is
// actually reached.
assert.strictEqual(content.header.threadId, null);
assert.strictEqual(content.header.trigger, 'OOMError');
helper.validate(report, REPORT_FIELDS);
}