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

report: fix missing section javascriptHeap on OOMError · nodejs/node@5355a46 · GitHub

/ node Public

Commit 5355a46

Browse files
legendecas
authored and
juanarbol
committed
report: fix missing section javascriptHeap on OOMError
`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: #44398 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
1 parent 2ba547a commit 5355a46

6 files changed

Lines changed: 55 additions & 38 deletions

‎src/node_report.cc‎

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -784,21 +784,8 @@ static void PrintRelease(JSONWriter* writer) {
784784

785785
} // namespace report
786786

787-
// External function to trigger a report, writing to file.
788787
std::string TriggerNodeReport(Isolate* isolate,
789-
const char* message,
790-
const char* trigger,
791-
const std::string& name,
792-
Local<Value> error) {
793-
Environment* env = nullptr;
794-
if (isolate != nullptr) {
795-
env = Environment::GetCurrent(isolate);
796-
}
797-
return TriggerNodeReport(env, message, trigger, name, error);
798-
}
799-
800-
// External function to trigger a report, writing to file.
801-
std::string TriggerNodeReport(Environment* env,
788+
Environment* env,
802789
const char* message,
803790
const char* trigger,
804791
const std::string& name,
@@ -868,10 +855,6 @@ std::string TriggerNodeReport(Environment* env,
868855
compact = per_process::cli_options->report_compact;
869856
}
870857

871-
Isolate* isolate = nullptr;
872-
if (env != nullptr) {
873-
isolate = env->isolate();
874-
}
875858
report::WriteNodeReport(
876859
isolate, env, message, trigger, filename, *outstream, error, compact);
877860

@@ -887,6 +870,33 @@ std::string TriggerNodeReport(Environment* env,
887870
return filename;
888871
}
889872

873+
// External function to trigger a report, writing to file.
874+
std::string TriggerNodeReport(Isolate* isolate,
875+
const char* message,
876+
const char* trigger,
877+
const std::string& name,
878+
Local<Value> error) {
879+
Environment* env = nullptr;
880+
if (isolate != nullptr) {
881+
env = Environment::GetCurrent(isolate);
882+
}
883+
return TriggerNodeReport(isolate, env, message, trigger, name, error);
884+
}
885+
886+
// External function to trigger a report, writing to file.
887+
std::string TriggerNodeReport(Environment* env,
888+
const char* message,
889+
const char* trigger,
890+
const std::string& name,
891+
Local<Value> error) {
892+
return TriggerNodeReport(env != nullptr ? env->isolate() : nullptr,
893+
env,
894+
message,
895+
trigger,
896+
name,
897+
error);
898+
}
899+
890900
// External function to trigger a report, writing to a supplied stream.
891901
void GetNodeReport(Isolate* isolate,
892902
const char* message,

‎test/report/test-report-fatalerror-oomerror-compact.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,13 @@ const fixtures = require('../common/fixtures');
1212

1313
// Common args that will cause an out-of-memory error for child process.
1414
const ARGS = [
15-
'--max-old-space-size=20',
15+
'--max-heap-size=20',
1616
fixtures.path('report-oom'),
1717
];
18+
const REPORT_FIELDS = [
19+
['header.trigger', 'OOMError'],
20+
['javascriptHeap.memoryLimit', 20 * 1024 * 1024 /* 20MB */],
21+
];
1822

1923
{
2024
// Verify that --report-compact is respected when set.
@@ -27,8 +31,8 @@ const ARGS = [
2731
assert.strictEqual(reports.length, 1);
2832

2933
const report = reports[0];
30-
helper.validate(report);
31-
assert.strictEqual(require(report).header.threadId, null);
34+
helper.validate(report, REPORT_FIELDS);
35+
3236
// Subtract 1 because "xx\n".split("\n") => [ 'xx', '' ].
3337
const lines = fs.readFileSync(report, 'utf8').split('\n').length - 1;
3438
assert.strictEqual(lines, 1);

‎test/report/test-report-fatalerror-oomerror-directory.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,13 @@ const fixtures = require('../common/fixtures');
1212

1313
// Common args that will cause an out-of-memory error for child process.
1414
const ARGS = [
15-
'--max-old-space-size=20',
15+
'--max-heap-size=20',
1616
fixtures.path('report-oom'),
1717
];
18+
const REPORT_FIELDS = [
19+
['header.trigger', 'OOMError'],
20+
['javascriptHeap.memoryLimit', 20 * 1024 * 1024 /* 20MB */],
21+
];
1822

1923
{
2024
// Verify that --report-directory is respected when set.
@@ -29,8 +33,8 @@ const ARGS = [
2933
assert.strictEqual(reports.length, 1);
3034

3135
const report = reports[0];
32-
helper.validate(report);
33-
assert.strictEqual(require(report).header.threadId, null);
36+
helper.validate(report, REPORT_FIELDS);
37+
3438
const lines = fs.readFileSync(report, 'utf8').split('\n').length - 1;
3539
assert(lines > 10);
3640
}

‎test/report/test-report-fatalerror-oomerror-filename.js‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,13 @@ const fixtures = require('../common/fixtures');
1111

1212
// Common args that will cause an out-of-memory error for child process.
1313
const ARGS = [
14-
'--max-old-space-size=20',
14+
'--max-heap-size=20',
1515
fixtures.path('report-oom'),
1616
];
17+
const REPORT_FIELDS = [
18+
['header.trigger', 'OOMError'],
19+
['javascriptHeap.memoryLimit', 20 * 1024 * 1024 /* 20MB */],
20+
];
1721

1822
{
1923
// Verify that --report-compact is respected when set.
@@ -34,7 +38,5 @@ const ARGS = [
3438
const lines = child.stderr.split('\n');
3539
// Skip over unavoidable free-form output and gc log from V8.
3640
const report = lines.find((i) => i.startsWith('{'));
37-
const json = JSON.parse(report);
38-
39-
assert.strictEqual(json.header.threadId, null);
41+
helper.validateContent(report, REPORT_FIELDS);
4042
}

‎test/report/test-report-fatalerror-oomerror-not-set.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ const fixtures = require('../common/fixtures');
1111

1212
// Common args that will cause an out-of-memory error for child process.
1313
const ARGS = [
14-
'--max-old-space-size=20',
14+
'--max-heap-size=20',
1515
fixtures.path('report-oom'),
1616
];
1717

‎test/report/test-report-fatalerror-oomerror-set.js‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,13 @@ const fixtures = require('../common/fixtures');
1111

1212
// Common args that will cause an out-of-memory error for child process.
1313
const ARGS = [
14-
'--max-old-space-size=20',
14+
'--max-heap-size=20',
1515
fixtures.path('report-oom'),
1616
];
17+
const REPORT_FIELDS = [
18+
['header.trigger', 'OOMError'],
19+
['javascriptHeap.memoryLimit', 20 * 1024 * 1024 /* 20MB */],
20+
];
1721

1822
{
1923
// Verify that --report-on-fatalerror is respected when set.
@@ -26,12 +30,5 @@ const ARGS = [
2630
assert.strictEqual(reports.length, 1);
2731

2832
const report = reports[0];
29-
helper.validate(report);
30-
31-
const content = require(report);
32-
// Errors occur in a context where env is not available, so thread ID is
33-
// unknown. Assert this, to verify that the underlying env-less situation is
34-
// actually reached.
35-
assert.strictEqual(content.header.threadId, null);
36-
assert.strictEqual(content.header.trigger, 'OOMError');
33+
helper.validate(report, REPORT_FIELDS);
3734
}

0 commit comments

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