report: generates report on threads with no isolates

PR-URL: #38994 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>

Author: legendecas

src/node_errors.cc

@@ -425,13 +425,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);

src/node_report.cc

@@ -272,20 +272,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);
 

test/common/report.js

@@ -54,17 +54,20 @@ function validateContent(report, fields = []){
 
 function_validateContent(report,fields=[]){
 constisWindows=process.platform==='win32';
+constisJavaScriptThreadReport=report.javascriptStack!=null;
 
 // Verify that all sections are present as own properties of the report.
-constsections=['header','javascriptStack','nativeStack',
-'javascriptHeap','libuv','environmentVariables',
+constsections=['header','nativeStack','libuv','environmentVariables',
 'sharedObjects','resourceUsage','workers'];
 if(!isWindows)
 sections.push('userLimits');
 
 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(typeofreport.javascriptStack.errorProperties,
-'object');
-assert.strictEqual(typeofreport.javascriptStack.message,'string');
-if(report.javascriptStack.stack!==undefined){
-assert(Array.isArray(report.javascriptStack.stack));
-report.javascriptStack.stack.forEach((frame)=>{
-assert.strictEqual(typeofframe,'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(typeofframe.symbol,'string');
 });
 
-// Verify the format of the javascriptHeap section.
-constheap=report.javascriptHeap;
-constjsHeapFields=['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(typeofheap.heapSpaces==='object'&&heap.heapSpaces!==null);
-constheapSpaceFields=['memorySize','committedMemory','capacity','used',
-'available'];
-Object.keys(heap.heapSpaces).forEach((spaceName)=>{
-constspace=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(typeofreport.javascriptStack.errorProperties,
+'object');
+assert.strictEqual(typeofreport.javascriptStack.message,'string');
+if(report.javascriptStack.stack!==undefined){
+assert(Array.isArray(report.javascriptStack.stack));
+report.javascriptStack.stack.forEach((frame)=>{
+assert.strictEqual(typeofframe,'string');
+});
+}
+
+// Verify the format of the javascriptHeap section.
+constheap=report.javascriptHeap;
+constjsHeapFields=['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(typeofheap.heapSpaces==='object'&&heap.heapSpaces!==null);
+constheapSpaceFields=['memorySize','committedMemory','capacity',
+'used','available'];
+Object.keys(heap.heapSpaces).forEach((spaceName)=>{
+constspace=heap.heapSpaces[spaceName];
+checkForUnknownFields(space,heapSpaceFields);
+heapSpaceFields.forEach((field)=>{
+assert(Number.isSafeInteger(space[field]));
+});
 });
-});
+}
 
 // Verify the format of the resourceUsage section.
 constusage=report.resourceUsage;

test/node-api/test_fatal/test_threads_report.js

@@ -0,0 +1,36 @@
+'use strict';
+constcommon=require('../../common');
+consthelper=require('../../common/report.js');
+consttmpdir=require('../../common/tmpdir');
+
+constassert=require('assert');
+constchild_process=require('child_process');
+consttest_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();
+constp=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');
+
+constreports=helper.findReports(p.pid,tmpdir.path);
+assert.strictEqual(reports.length,1);
+
+constreport=reports[0];
+helper.validate(report);