From 3cab19c12a09c7297eafede2157884cfb8e15675 Mon Sep 17 00:00:00 2001 From: legendecas Date: Thu, 10 Jun 2021 23:55:54 +0800 Subject: [PATCH] report: generates report on threads with no isolates PR-URL: https://github.com/nodejs/node/pull/38994 Reviewed-By: Gireesh Punathil Reviewed-By: Anna Henningsen Reviewed-By: James M Snell --- src/node_errors.cc | 9 +-- src/node_report.cc | 20 ++--- test/common/report.js | 73 ++++++++++--------- .../test_fatal/test_threads_report.js | 36 +++++++++ 4 files changed, 89 insertions(+), 49 deletions(-) create mode 100644 test/node-api/test_fatal/test_threads_report.js diff --git a/src/node_errors.cc b/src/node_errors.cc index 786e4153f2c13f..c05bf1732bf48c 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -424,13 +424,10 @@ void OnFatalError(const char* location, const char* message) { } Isolate* isolate = Isolate::GetCurrent(); - // TODO(legendecas): investigate failures on triggering node-report with - // nullptr isolates. - if (isolate == nullptr) { - fflush(stderr); - ABORT(); + Environment* env = nullptr; + if (isolate != nullptr) { + env = Environment::GetCurrent(isolate); } - Environment* env = Environment::GetCurrent(isolate); bool report_on_fatalerror; { Mutex::ScopedLock lock(node::per_process::cli_options_mutex); diff --git a/src/node_report.cc b/src/node_report.cc index 13f87b1e52e5d6..a91d16d5f11e64 100644 --- a/src/node_report.cc +++ b/src/node_report.cc @@ -267,20 +267,22 @@ static void WriteNodeReport(Isolate* isolate, PrintVersionInformation(&writer); writer.json_objectend(); - writer.json_objectstart("javascriptStack"); - // Report summary JavaScript error stack backtrace - PrintJavaScriptErrorStack(&writer, isolate, error, trigger); + if (isolate != nullptr) { + writer.json_objectstart("javascriptStack"); + // Report summary JavaScript error stack backtrace + PrintJavaScriptErrorStack(&writer, isolate, error, trigger); - // Report summary JavaScript error properties backtrace - PrintJavaScriptErrorProperties(&writer, isolate, error); - writer.json_objectend(); // the end of 'javascriptStack' + // Report summary JavaScript error properties backtrace + PrintJavaScriptErrorProperties(&writer, isolate, error); + writer.json_objectend(); // the end of 'javascriptStack' + + // Report V8 Heap and Garbage Collector information + PrintGCStatistics(&writer, isolate); + } // Report native stack backtrace PrintNativeStack(&writer); - // Report V8 Heap and Garbage Collector information - PrintGCStatistics(&writer, isolate); - // Report OS and current thread resource usage PrintResourceUsage(&writer); diff --git a/test/common/report.js b/test/common/report.js index f49493b443edea..28c10181474f25 100644 --- a/test/common/report.js +++ b/test/common/report.js @@ -54,10 +54,10 @@ function validateContent(report, fields = []) { function _validateContent(report, fields = []) { const isWindows = process.platform === 'win32'; + const isJavaScriptThreadReport = report.javascriptStack != null; // Verify that all sections are present as own properties of the report. - const sections = ['header', 'javascriptStack', 'nativeStack', - 'javascriptHeap', 'libuv', 'environmentVariables', + const sections = ['header', 'nativeStack', 'libuv', 'environmentVariables', 'sharedObjects', 'resourceUsage', 'workers']; if (!isWindows) sections.push('userLimits'); @@ -65,6 +65,9 @@ function _validateContent(report, fields = []) { if (report.uvthreadResourceUsage) sections.push('uvthreadResourceUsage'); + if (isJavaScriptThreadReport) + sections.push('javascriptStack', 'javascriptHeap'); + checkForUnknownFields(report, sections); sections.forEach((section) => { assert(report.hasOwnProperty(section)); @@ -163,19 +166,6 @@ function _validateContent(report, fields = []) { }); assert.strictEqual(header.host, os.hostname()); - // Verify the format of the javascriptStack section. - checkForUnknownFields(report.javascriptStack, - ['message', 'stack', 'errorProperties']); - assert.strictEqual(typeof report.javascriptStack.errorProperties, - 'object'); - assert.strictEqual(typeof report.javascriptStack.message, 'string'); - if (report.javascriptStack.stack !== undefined) { - assert(Array.isArray(report.javascriptStack.stack)); - report.javascriptStack.stack.forEach((frame) => { - assert.strictEqual(typeof frame, 'string'); - }); - } - // Verify the format of the nativeStack section. assert(Array.isArray(report.nativeStack)); report.nativeStack.forEach((frame) => { @@ -186,26 +176,41 @@ function _validateContent(report, fields = []) { assert.strictEqual(typeof frame.symbol, 'string'); }); - // Verify the format of the javascriptHeap section. - const heap = report.javascriptHeap; - const jsHeapFields = ['totalMemory', 'totalCommittedMemory', 'usedMemory', - 'availableMemory', 'memoryLimit', 'heapSpaces']; - checkForUnknownFields(heap, jsHeapFields); - assert(Number.isSafeInteger(heap.totalMemory)); - assert(Number.isSafeInteger(heap.totalCommittedMemory)); - assert(Number.isSafeInteger(heap.usedMemory)); - assert(Number.isSafeInteger(heap.availableMemory)); - assert(Number.isSafeInteger(heap.memoryLimit)); - assert(typeof heap.heapSpaces === 'object' && heap.heapSpaces !== null); - const heapSpaceFields = ['memorySize', 'committedMemory', 'capacity', 'used', - 'available']; - Object.keys(heap.heapSpaces).forEach((spaceName) => { - const space = heap.heapSpaces[spaceName]; - checkForUnknownFields(space, heapSpaceFields); - heapSpaceFields.forEach((field) => { - assert(Number.isSafeInteger(space[field])); + if (isJavaScriptThreadReport) { + // Verify the format of the javascriptStack section. + checkForUnknownFields(report.javascriptStack, + ['message', 'stack', 'errorProperties']); + assert.strictEqual(typeof report.javascriptStack.errorProperties, + 'object'); + assert.strictEqual(typeof report.javascriptStack.message, 'string'); + if (report.javascriptStack.stack !== undefined) { + assert(Array.isArray(report.javascriptStack.stack)); + report.javascriptStack.stack.forEach((frame) => { + assert.strictEqual(typeof frame, 'string'); + }); + } + + // Verify the format of the javascriptHeap section. + const heap = report.javascriptHeap; + const jsHeapFields = ['totalMemory', 'totalCommittedMemory', 'usedMemory', + 'availableMemory', 'memoryLimit', 'heapSpaces']; + checkForUnknownFields(heap, jsHeapFields); + assert(Number.isSafeInteger(heap.totalMemory)); + assert(Number.isSafeInteger(heap.totalCommittedMemory)); + assert(Number.isSafeInteger(heap.usedMemory)); + assert(Number.isSafeInteger(heap.availableMemory)); + assert(Number.isSafeInteger(heap.memoryLimit)); + assert(typeof heap.heapSpaces === 'object' && heap.heapSpaces !== null); + const heapSpaceFields = ['memorySize', 'committedMemory', 'capacity', + 'used', 'available']; + Object.keys(heap.heapSpaces).forEach((spaceName) => { + const space = heap.heapSpaces[spaceName]; + checkForUnknownFields(space, heapSpaceFields); + heapSpaceFields.forEach((field) => { + assert(Number.isSafeInteger(space[field])); + }); }); - }); + } // Verify the format of the resourceUsage section. const usage = report.resourceUsage; diff --git a/test/node-api/test_fatal/test_threads_report.js b/test/node-api/test_fatal/test_threads_report.js new file mode 100644 index 00000000000000..83c6642bdc41eb --- /dev/null +++ b/test/node-api/test_fatal/test_threads_report.js @@ -0,0 +1,36 @@ +'use strict'; +const common = require('../../common'); +const helper = require('../../common/report.js'); +const tmpdir = require('../../common/tmpdir'); + +const assert = require('assert'); +const child_process = require('child_process'); +const test_fatal = require(`./build/${common.buildType}/test_fatal`); + +if (common.buildType === 'Debug') + common.skip('as this will currently fail with a Debug check ' + + 'in v8::Isolate::GetCurrent()'); + +// Test in a child process because the test code will trigger a fatal error +// that crashes the process. +if (process.argv[2] === 'child') { + test_fatal.TestThread(); + // Busy loop to allow the work thread to abort. + while (true) {} +} + +tmpdir.refresh(); +const p = child_process.spawnSync( + process.execPath, + [ '--report-on-fatalerror', __filename, 'child' ], + { cwd: tmpdir.path }); +assert.ifError(p.error); +assert.ok(p.stderr.toString().includes( + 'FATAL ERROR: work_thread foobar')); +assert.ok(p.status === 134 || p.signal === 'SIGABRT'); + +const reports = helper.findReports(p.pid, tmpdir.path); +assert.strictEqual(reports.length, 1); + +const report = reports[0]; +helper.validate(report);