Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

report: fix missing section javascriptHeap on OOMError #44398

Closed
wants to merge 3 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
report: get stack trace with across origin contexts
When a new context with a different security token is entered, or
when no context is entered, `StackTrace::CurrentStackTrace` need to
be explicitly set with flag `kExposeFramesAcrossSecurityOrigins` to
avoid crashing.
legendecas committed Aug 29, 2022
commit 6d06cf159280b216aa0864c031455e184b236518
6 changes: 5 additions & 1 deletion src/node_report.cc
Original file line number Diff line number Diff line change
@@ -470,8 +470,12 @@ static void PrintJavaScriptStack(JSONWriter* writer,
void* samples[MAX_FRAME_COUNT];
isolate->GetStackSample(state, samples, MAX_FRAME_COUNT, &info);

constexpr StackTrace::StackTraceOptions stack_trace_options =
static_cast<StackTrace::StackTraceOptions>(
StackTrace::kDetailed |
StackTrace::kExposeFramesAcrossSecurityOrigins);
Local<StackTrace> stack = StackTrace::CurrentStackTrace(
isolate, MAX_FRAME_COUNT, StackTrace::kDetailed);
isolate, MAX_FRAME_COUNT, stack_trace_options);

if (stack->GetFrameCount() == 0) {
PrintEmptyJavaScriptStack(writer);
27 changes: 27 additions & 0 deletions test/addons/report-api/binding.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include <node.h>
#include <v8.h>

using v8::Context;
using v8::FunctionCallbackInfo;
using v8::Isolate;
using v8::Local;
@@ -43,11 +44,37 @@ void TriggerReportNoEnv(const FunctionCallbackInfo<Value>& args) {
Local<Value>());
}

void TriggerReportNoContext(const FunctionCallbackInfo<Value>& args) {
Isolate* isolate = args.GetIsolate();
Local<Context> context = isolate->GetCurrentContext();
context->Exit();

if (isolate->GetCurrentContext().IsEmpty()) {
node::TriggerNodeReport(
isolate, "FooMessage", "BarTrigger", std::string(), Local<Value>());
}

// Restore current context to avoid crashing in Context::Scope in
// SpinEventLoop.
context->Enter();
}

void TriggerReportNewContext(const FunctionCallbackInfo<Value>& args) {
Isolate* isolate = args.GetIsolate();
Local<Context> context = Context::New(isolate);
Context::Scope context_scope(context);

node::TriggerNodeReport(
isolate, "FooMessage", "BarTrigger", std::string(), Local<Value>());
}

void init(Local<Object> exports) {
NODE_SET_METHOD(exports, "triggerReport", TriggerReport);
NODE_SET_METHOD(exports, "triggerReportNoIsolate", TriggerReportNoIsolate);
NODE_SET_METHOD(exports, "triggerReportEnv", TriggerReportEnv);
NODE_SET_METHOD(exports, "triggerReportNoEnv", TriggerReportNoEnv);
NODE_SET_METHOD(exports, "triggerReportNoContext", TriggerReportNoContext);
NODE_SET_METHOD(exports, "triggerReportNewContext", TriggerReportNewContext);
}

NODE_MODULE(NODE_GYP_MODULE_NAME, init)
31 changes: 20 additions & 11 deletions test/addons/report-api/test.js
Original file line number Diff line number Diff line change
@@ -9,7 +9,7 @@ const tmpdir = require('../../common/tmpdir');
const binding = path.resolve(__dirname, `./build/${common.buildType}/binding`);
const addon = require(binding);

function myAddonMain(method, hasJavaScriptFrames) {
function myAddonMain(method, { hasIsolate, hasEnv }) {
tmpdir.refresh();
process.report.directory = tmpdir.path;

@@ -19,26 +19,35 @@ function myAddonMain(method, hasJavaScriptFrames) {
assert.strictEqual(reports.length, 1);

const report = reports[0];
helper.validate(report);
helper.validate(report, [
['header.event', 'FooMessage'],
['header.trigger', 'BarTrigger'],
]);

const content = require(report);
assert.strictEqual(content.header.event, 'FooMessage');
assert.strictEqual(content.header.trigger, 'BarTrigger');

// Check that the javascript stack is present.
if (hasJavaScriptFrames) {
if (hasIsolate) {
assert.strictEqual(content.javascriptStack.stack.findIndex((frame) => frame.match('myAddonMain')), 0);
} else {
assert.strictEqual(content.javascriptStack, undefined);
}

if (hasEnv) {
assert.strictEqual(content.header.threadId, 0);
} else {
assert.strictEqual(content.header.threadId, null);
}
}

const methods = [
['triggerReport', true],
['triggerReportNoIsolate', false],
['triggerReportEnv', true],
['triggerReportNoEnv', false],
['triggerReport', true, true],
['triggerReportNoIsolate', false, false],
['triggerReportEnv', true, true],
['triggerReportNoEnv', false, false],
['triggerReportNoContext', true, false],
['triggerReportNewContext', true, false],
];
for (const [method, hasJavaScriptFrames] of methods) {
myAddonMain(method, hasJavaScriptFrames);
for (const [method, hasIsolate, hasEnv] of methods) {
myAddonMain(method, { hasIsolate, hasEnv });
}