Skip to content

Commit

Permalink
report: print javascript stack on fatal error
Browse files Browse the repository at this point in the history
Try to print JavaScript stack on fatal error. OOMError needs to be
distinguished from fatal error since no new handle can be created at
that time.

PR-URL: #44242
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
  • Loading branch information
legendecas authored and ruyadorno committed Aug 22, 2022
1 parent 7a56787 commit 8cf6499
Show file tree
Hide file tree
Showing 8 changed files with 196 additions and 8 deletions.
1 change: 1 addition & 0 deletions src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ void SetIsolateErrorHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
auto* fatal_error_cb = s.fatal_error_callback ?
s.fatal_error_callback : OnFatalError;
isolate->SetFatalErrorHandler(fatal_error_cb);
isolate->SetOOMErrorHandler(OOMErrorHandler);

if ((s.flags & SHOULD_NOT_SET_PREPARE_STACK_TRACE_CALLBACK) == 0) {
auto* prepare_stack_trace_cb = s.prepare_stack_trace_callback ?
Expand Down
30 changes: 30 additions & 0 deletions src/node_errors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,36 @@ void OnFatalError(const char* location, const char* message) {
ABORT();
}

void OOMErrorHandler(const char* location, bool is_heap_oom) {
const char* message =
is_heap_oom ? "Allocation failed - JavaScript heap out of memory"
: "Allocation failed - process out of memory";
if (location) {
FPrintF(stderr, "FATAL ERROR: %s %s\n", location, message);
} else {
FPrintF(stderr, "FATAL ERROR: %s\n", message);
}

Isolate* isolate = Isolate::TryGetCurrent();
Environment* env = nullptr;
if (isolate != nullptr) {
env = Environment::GetCurrent(isolate);
}
bool report_on_fatalerror;
{
Mutex::ScopedLock lock(node::per_process::cli_options_mutex);
report_on_fatalerror = per_process::cli_options->report_on_fatalerror;
}

if (report_on_fatalerror) {
report::TriggerNodeReport(
isolate, env, message, "OOMError", "", Local<Object>());
}

fflush(stderr);
ABORT();
}

v8::ModifyCodeGenerationFromStringsResult ModifyCodeGenerationFromStrings(
v8::Local<v8::Context> context,
v8::Local<v8::Value> source,
Expand Down
1 change: 1 addition & 0 deletions src/node_errors.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ void AppendExceptionLine(Environment* env,

[[noreturn]] void FatalError(const char* location, const char* message);
void OnFatalError(const char* location, const char* message);
void OOMErrorHandler(const char* location, bool is_heap_oom);

// Helpers to construct errors similar to the ones provided by
// lib/internal/errors.js.
Expand Down
84 changes: 77 additions & 7 deletions src/node_report.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
constexpr int NODE_REPORT_VERSION = 2;
constexpr int NANOS_PER_SEC = 1000 * 1000 * 1000;
constexpr double SEC_PER_MICROS = 1e-6;
constexpr int MAX_FRAME_COUNT = 10;

namespace node {
namespace report {
Expand All @@ -43,6 +44,10 @@ using v8::Maybe;
using v8::MaybeLocal;
using v8::Nothing;
using v8::Object;
using v8::RegisterState;
using v8::SampleInfo;
using v8::StackFrame;
using v8::StackTrace;
using v8::String;
using v8::TryCatch;
using v8::V8;
Expand All @@ -62,6 +67,9 @@ static void PrintJavaScriptErrorStack(JSONWriter* writer,
Isolate* isolate,
Local<Value> error,
const char* trigger);
static void PrintJavaScriptStack(JSONWriter* writer,
Isolate* isolate,
const char* trigger);
static void PrintJavaScriptErrorProperties(JSONWriter* writer,
Isolate* isolate,
Local<Value> error);
Expand Down Expand Up @@ -269,8 +277,6 @@ static void WriteNodeReport(Isolate* isolate,
// 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 V8 Heap and Garbage Collector information
Expand Down Expand Up @@ -317,7 +323,7 @@ static void WriteNodeReport(Isolate* isolate,
env,
"Worker thread subreport",
trigger,
Local<Object>(),
Local<Value>(),
os);

Mutex::ScopedLock lock(workers_mutex);
Expand Down Expand Up @@ -534,19 +540,80 @@ static Maybe<std::string> ErrorToString(Isolate* isolate,
return Just<>(std::string(*sv, sv.length()));
}

static void PrintEmptyJavaScriptStack(JSONWriter* writer) {
writer->json_keyvalue("message", "No stack.");
writer->json_arraystart("stack");
writer->json_element("Unavailable.");
writer->json_arrayend();

writer->json_objectstart("errorProperties");
writer->json_objectend();
}

// Do our best to report the JavaScript stack without calling into JavaScript.
static void PrintJavaScriptStack(JSONWriter* writer,
Isolate* isolate,
const char* trigger) {
// Can not capture the stacktrace when the isolate is in a OOM state.
if (!strcmp(trigger, "OOMError")) {
PrintEmptyJavaScriptStack(writer);
return;
}

HandleScope scope(isolate);
RegisterState state;
state.pc = nullptr;
state.fp = &state;
state.sp = &state;

// in-out params
SampleInfo info;
void* samples[MAX_FRAME_COUNT];
isolate->GetStackSample(state, samples, MAX_FRAME_COUNT, &info);

Local<StackTrace> stack = StackTrace::CurrentStackTrace(
isolate, MAX_FRAME_COUNT, StackTrace::kDetailed);

if (stack->GetFrameCount() == 0) {
PrintEmptyJavaScriptStack(writer);
return;
}

writer->json_keyvalue("message", trigger);
writer->json_arraystart("stack");
for (int i = 0; i < stack->GetFrameCount(); i++) {
Local<StackFrame> frame = stack->GetFrame(isolate, i);

Utf8Value function_name(isolate, frame->GetFunctionName());
Utf8Value script_name(isolate, frame->GetScriptName());
const int line_number = frame->GetLineNumber();
const int column = frame->GetColumn();

std::string stack_line = SPrintF(
"at %s (%s:%d:%d)", *function_name, *script_name, line_number, column);
writer->json_element(stack_line);
}
writer->json_arrayend();
writer->json_objectstart("errorProperties");
writer->json_objectend();
}

// Report the JavaScript stack.
static void PrintJavaScriptErrorStack(JSONWriter* writer,
Isolate* isolate,
Local<Value> error,
const char* trigger) {
if (error.IsEmpty()) {
return PrintJavaScriptStack(writer, isolate, trigger);
}

TryCatch try_catch(isolate);
HandleScope scope(isolate);
Local<Context> context = isolate->GetCurrentContext();
std::string ss = "";
if ((!strcmp(trigger, "FatalError")) ||
(!strcmp(trigger, "Signal")) ||
(!ErrorToString(isolate, context, error).To(&ss))) {
ss = "No stack.\nUnavailable.\n";
if (!ErrorToString(isolate, context, error).To(&ss)) {
PrintEmptyJavaScriptStack(writer);
return;
}

int line = ss.find('\n');
Expand All @@ -569,6 +636,9 @@ static void PrintJavaScriptErrorStack(JSONWriter* writer,
}
writer->json_arrayend();
}

// Report summary JavaScript error properties backtrace
PrintJavaScriptErrorProperties(writer, isolate, error);
}

// Report a native stack backtrace
Expand Down
23 changes: 23 additions & 0 deletions test/addons/report-fatalerror/binding.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#include <node.h>
#include <v8.h>

using v8::FunctionCallbackInfo;
using v8::Isolate;
using v8::Local;
using v8::MaybeLocal;
using v8::Object;
using v8::Value;

void TriggerFatalError(const FunctionCallbackInfo<Value>& args) {
Isolate* isolate = args.GetIsolate();

// Trigger a v8 ApiCheck failure.
MaybeLocal<Value> value;
value.ToLocalChecked();
}

void init(Local<Object> exports) {
NODE_SET_METHOD(exports, "triggerFatalError", TriggerFatalError);
}

NODE_MODULE(NODE_GYP_MODULE_NAME, init)
9 changes: 9 additions & 0 deletions test/addons/report-fatalerror/binding.gyp
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
'targets': [
{
'target_name': 'binding',
'sources': [ 'binding.cc' ],
'includes': ['../common.gypi'],
}
]
}
52 changes: 52 additions & 0 deletions test/addons/report-fatalerror/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
'use strict';

const common = require('../../common');
const assert = require('assert');
const path = require('path');
const spawnSync = require('child_process').spawnSync;
const helper = require('../../common/report.js');
const tmpdir = require('../../common/tmpdir');

const binding = path.resolve(__dirname, `./build/${common.buildType}/binding`);

if (process.argv[2] === 'child') {
(function childMain() {
const addon = require(binding);
addon.triggerFatalError();
})();
return;
}

const ARGS = [
__filename,
'child',
];

{
// Verify that --report-on-fatalerror is respected when set.
tmpdir.refresh();
const args = ['--report-on-fatalerror', ...ARGS];
const child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');

const reports = helper.findReports(child.pid, tmpdir.path);
assert.strictEqual(reports.length, 1);

const report = reports[0];
helper.validate(report);

const content = require(report);
assert.strictEqual(content.header.trigger, 'FatalError');

// Check that the javascript stack is present.
assert.strictEqual(content.javascriptStack.stack.findIndex((frame) => frame.match('childMain')), 0);
}

{
// Verify that --report-on-fatalerror is respected when not set.
const args = ARGS;
const child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
const reports = helper.findReports(child.pid, tmpdir.path);
assert.strictEqual(reports.length, 0);
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,12 @@ const ARGS = [
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(require(report).header.threadId, null);
assert.strictEqual(content.header.threadId, null);
assert.strictEqual(content.header.trigger, 'OOMError');
}

{
Expand Down

0 comments on commit 8cf6499

Please sign in to comment.