From 52bbd35905f88d4b8973bfde562d47518ca29a36 Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Wed, 21 Jun 2023 21:58:15 +0800 Subject: [PATCH] report: disable js stack when no context is entered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are no guarantees that the JS stack can be generated when no context is entered. PR-URL: https://github.com/nodejs/node/pull/48495 Fixes: https://github.com/nodejs/node-v8/issues/250 Refs: https://chromium-review.googlesource.com/c/v8/v8/+/4582948 Reviewed-By: Gireesh Punathil Reviewed-By: Colin Ihrig Reviewed-By: Joyee Cheung Reviewed-By: James M Snell Reviewed-By: Gerhard Stöbich --- src/node_report.cc | 10 ++++++++-- test/addons/report-api/test.js | 12 ++++++------ test/common/report.js | 8 ++++---- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/node_report.cc b/src/node_report.cc index 94f31663d2c736..7c1c9e66d29833 100644 --- a/src/node_report.cc +++ b/src/node_report.cc @@ -66,6 +66,7 @@ static void PrintJavaScriptErrorStack(JSONWriter* writer, Isolate* isolate, Local error, const char* trigger); +static void PrintEmptyJavaScriptStack(JSONWriter* writer); static void PrintJavaScriptStack(JSONWriter* writer, Isolate* isolate, const char* trigger); @@ -184,6 +185,10 @@ static void WriteNodeReport(Isolate* isolate, // Report V8 Heap and Garbage Collector information PrintGCStatistics(&writer, isolate); + } else { + writer.json_objectstart("javascriptStack"); + PrintEmptyJavaScriptStack(&writer); + writer.json_objectend(); // the end of 'javascriptStack' } // Report native stack backtrace @@ -452,8 +457,9 @@ static void PrintEmptyJavaScriptStack(JSONWriter* writer) { 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")) { + // Can not capture the stacktrace when the isolate is in a OOM state or no + // context is entered. + if (!strcmp(trigger, "OOMError") || !isolate->InContext()) { PrintEmptyJavaScriptStack(writer); return; } diff --git a/test/addons/report-api/test.js b/test/addons/report-api/test.js index 67c53404688840..41cf19e75a365d 100644 --- a/test/addons/report-api/test.js +++ b/test/addons/report-api/test.js @@ -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, { hasIsolate, hasEnv }) { +function myAddonMain(method, { hasContext, hasEnv }) { tmpdir.refresh(); process.report.directory = tmpdir.path; @@ -27,10 +27,10 @@ function myAddonMain(method, { hasIsolate, hasEnv }) { const content = require(report); // Check that the javascript stack is present. - if (hasIsolate) { + if (hasContext) { assert.strictEqual(content.javascriptStack.stack.findIndex((frame) => frame.match('myAddonMain')), 0); } else { - assert.strictEqual(content.javascriptStack, undefined); + assert.strictEqual(content.javascriptStack.message, 'No stack.'); } if (hasEnv) { @@ -45,9 +45,9 @@ const methods = [ ['triggerReportNoIsolate', false, false], ['triggerReportEnv', true, true], ['triggerReportNoEnv', false, false], - ['triggerReportNoContext', true, false], + ['triggerReportNoContext', false, false], ['triggerReportNewContext', true, false], ]; -for (const [method, hasIsolate, hasEnv] of methods) { - myAddonMain(method, { hasIsolate, hasEnv }); +for (const [method, hasContext, hasEnv] of methods) { + myAddonMain(method, { hasContext, hasEnv }); } diff --git a/test/common/report.js b/test/common/report.js index f30c4d2a75477a..6e41561186570d 100644 --- a/test/common/report.js +++ b/test/common/report.js @@ -55,11 +55,11 @@ function validateContent(report, fields = []) { function _validateContent(report, fields = []) { const isWindows = process.platform === 'win32'; - const isJavaScriptThreadReport = report.javascriptStack != null; + const isJavaScriptThreadReport = report.javascriptHeap != null; // Verify that all sections are present as own properties of the report. - const sections = ['header', 'nativeStack', 'libuv', 'environmentVariables', - 'sharedObjects', 'resourceUsage', 'workers']; + const sections = ['header', 'nativeStack', 'javascriptStack', 'libuv', + 'environmentVariables', 'sharedObjects', 'resourceUsage', 'workers']; if (!isWindows) sections.push('userLimits'); @@ -67,7 +67,7 @@ function _validateContent(report, fields = []) { sections.push('uvthreadResourceUsage'); if (isJavaScriptThreadReport) - sections.push('javascriptStack', 'javascriptHeap'); + sections.push('javascriptHeap'); checkForUnknownFields(report, sections); sections.forEach((section) => {