fix: enable crashpad for ELECTRON_RUN_AS_NODE processes (#36460)
* wip: enable crashpad for node processes fix: add PID testing method wip: plumb fd into child_process in node * node::ProcessInitializationFlags::kNoDefaultSignalHandling * chore: clean up debug logging * chore: gate platform includes * test: clean up node process test * fix: pass pid in node_main * chore: cleanup impl * chore: fixup patch method definition * fix: expose bound methods to node_main * fix: remove bound methods * fix: crashpad connection for all ELECTRON_RUN_AS_NODE processes * chore: fix typo * chore: address review feedback * chore: delay crashpad initialization * chore: ensure options.env, code hygiene * chore: add argv test, check for process.env over {} * fix: fix test, return options.env immutability Co-authored-by: VerteDinde <keeleymhammond@gmail.com> Co-authored-by: Jeremy Rose <jeremya@chromium.org> Co-authored-by: VerteDinde <vertedinde@electronjs.org>
This commit is contained in:
parent
16a7bd7102
commit
2c723d7e84
10 changed files with 194 additions and 12 deletions
|
@ -530,6 +530,7 @@ filenames = {
|
||||||
"shell/browser/window_list_observer.h",
|
"shell/browser/window_list_observer.h",
|
||||||
"shell/browser/zoom_level_delegate.cc",
|
"shell/browser/zoom_level_delegate.cc",
|
||||||
"shell/browser/zoom_level_delegate.h",
|
"shell/browser/zoom_level_delegate.h",
|
||||||
|
"shell/common/api/crashpad_support.cc",
|
||||||
"shell/common/api/electron_api_asar.cc",
|
"shell/common/api/electron_api_asar.cc",
|
||||||
"shell/common/api/electron_api_clipboard.cc",
|
"shell/common/api/electron_api_clipboard.cc",
|
||||||
"shell/common/api/electron_api_clipboard.h",
|
"shell/common/api/electron_api_clipboard.h",
|
||||||
|
|
|
@ -51,3 +51,4 @@ src_iwyu_in_cleanup_queue_cc.patch
|
||||||
fix_expose_lookupandcompile_with_parameters.patch
|
fix_expose_lookupandcompile_with_parameters.patch
|
||||||
fix_prevent_changing_functiontemplateinfo_after_publish.patch
|
fix_prevent_changing_functiontemplateinfo_after_publish.patch
|
||||||
chore_add_missing_algorithm_include.patch
|
chore_add_missing_algorithm_include.patch
|
||||||
|
enable_crashpad_linux_node_processes.patch
|
||||||
|
|
51
patches/node/enable_crashpad_linux_node_processes.patch
Normal file
51
patches/node/enable_crashpad_linux_node_processes.patch
Normal file
|
@ -0,0 +1,51 @@
|
||||||
|
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||||
|
From: VerteDinde <keeleymhammond@gmail.com>
|
||||||
|
Date: Sun, 20 Nov 2022 21:45:20 -0800
|
||||||
|
Subject: fix: enable crashpad for ELECTRON_RUN_AS_NODE linux processes
|
||||||
|
|
||||||
|
Passes the crashpad handler PID and crashdump signal file descriptor
|
||||||
|
to child processes spawned with `ELECTRON_RUN_AS_NODE` which is used
|
||||||
|
by the crashpad client to connect with the handler process.
|
||||||
|
|
||||||
|
diff --git a/lib/child_process.js b/lib/child_process.js
|
||||||
|
index 73c1dc769542865bdf7a2a03c16cef141d3f4b05..76151c75ef7ad420d2642c1cd11c8624b7d7e2a0 100644
|
||||||
|
--- a/lib/child_process.js
|
||||||
|
+++ b/lib/child_process.js
|
||||||
|
@@ -59,6 +59,7 @@ let debug = require('internal/util/debuglog').debuglog(
|
||||||
|
);
|
||||||
|
const { Buffer } = require('buffer');
|
||||||
|
const { Pipe, constants: PipeConstants } = internalBinding('pipe_wrap');
|
||||||
|
+const { getCrashdumpSignalFD, getCrashpadHandlerPID } = process._linkedBinding('electron_common_crashpad_support');
|
||||||
|
|
||||||
|
const {
|
||||||
|
AbortError,
|
||||||
|
@@ -159,7 +160,6 @@ function fork(modulePath, args = [], options) {
|
||||||
|
ArrayPrototypeSplice(execArgv, index - 1, 2);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
-
|
||||||
|
args = [...execArgv, modulePath, ...args];
|
||||||
|
|
||||||
|
if (typeof options.stdio === 'string') {
|
||||||
|
@@ -613,6 +613,21 @@ function normalizeSpawnArguments(file, args, options) {
|
||||||
|
'options.windowsVerbatimArguments');
|
||||||
|
}
|
||||||
|
|
||||||
|
+ if (process.platform === 'linux') {
|
||||||
|
+ if (ObjectPrototypeHasOwnProperty(options.env || process.env, 'ELECTRON_RUN_AS_NODE') &&
|
||||||
|
+ (file === process.execPath)) {
|
||||||
|
+ // On Linux, pass the file descriptor which crashpad handler process
|
||||||
|
+ // uses to monitor the child process and PID of the handler process.
|
||||||
|
+ // https://source.chromium.org/chromium/chromium/src/+/110.0.5415.0:components/crash/core/app/crashpad_linux.cc;l=199-206
|
||||||
|
+ const fd = getCrashdumpSignalFD();
|
||||||
|
+ const pid = getCrashpadHandlerPID();
|
||||||
|
+ if (fd !== -1 && pid !== -1) {
|
||||||
|
+ options.env.CRASHDUMP_SIGNAL_FD = fd;
|
||||||
|
+ options.env.CRASHPAD_HANDLER_PID = pid;
|
||||||
|
+ }
|
||||||
|
+ }
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
if (options.shell) {
|
||||||
|
const command = ArrayPrototypeJoin([file, ...args], ' ');
|
||||||
|
// Set the shell, switches, and commands.
|
|
@ -34,6 +34,14 @@
|
||||||
#include "chrome/child/v8_crashpad_support_win.h"
|
#include "chrome/child/v8_crashpad_support_win.h"
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
|
#if BUILDFLAG(IS_LINUX)
|
||||||
|
#include "base/environment.h"
|
||||||
|
#include "base/posix/global_descriptors.h"
|
||||||
|
#include "base/strings/string_number_conversions.h"
|
||||||
|
#include "components/crash/core/app/crash_switches.h" // nogncheck
|
||||||
|
#include "content/public/common/content_descriptors.h"
|
||||||
|
#endif
|
||||||
|
|
||||||
#if !IS_MAS_BUILD()
|
#if !IS_MAS_BUILD()
|
||||||
#include "components/crash/core/app/crashpad.h" // nogncheck
|
#include "components/crash/core/app/crashpad.h" // nogncheck
|
||||||
#include "shell/app/electron_crash_reporter_client.h"
|
#include "shell/app/electron_crash_reporter_client.h"
|
||||||
|
@ -110,15 +118,20 @@ int NodeMain(int argc, char* argv[]) {
|
||||||
v8_crashpad_support::SetUp();
|
v8_crashpad_support::SetUp();
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
// TODO(deepak1556): Enable crashpad support on linux for
|
#if BUILDFLAG(IS_LINUX)
|
||||||
// ELECTRON_RUN_AS_NODE processes.
|
auto os_env = base::Environment::Create();
|
||||||
// Refs https://github.com/electron/electron/issues/36030
|
std::string fd_string, pid_string;
|
||||||
#if BUILDFLAG(IS_WIN) || (BUILDFLAG(IS_MAC) && !IS_MAS_BUILD())
|
if (os_env->GetVar("CRASHDUMP_SIGNAL_FD", &fd_string) &&
|
||||||
ElectronCrashReporterClient::Create();
|
os_env->GetVar("CRASHPAD_HANDLER_PID", &pid_string)) {
|
||||||
crash_reporter::InitializeCrashpad(false, "node");
|
int fd = -1, pid = -1;
|
||||||
crash_keys::SetCrashKeysFromCommandLine(
|
DCHECK(base::StringToInt(fd_string, &fd));
|
||||||
*base::CommandLine::ForCurrentProcess());
|
DCHECK(base::StringToInt(pid_string, &pid));
|
||||||
crash_keys::SetPlatformCrashKey();
|
base::GlobalDescriptors::GetInstance()->Set(kCrashDumpSignal, fd);
|
||||||
|
// Following API is unsafe in multi-threaded scenario, but at this point
|
||||||
|
// we are still single threaded.
|
||||||
|
os_env->UnSetVar("CRASHDUMP_SIGNAL_FD");
|
||||||
|
os_env->UnSetVar("CRASHPAD_HANDLER_PID");
|
||||||
|
}
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
int exit_code = 1;
|
int exit_code = 1;
|
||||||
|
@ -158,6 +171,29 @@ int NodeMain(int argc, char* argv[]) {
|
||||||
return result->exit_code();
|
return result->exit_code();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#if BUILDFLAG(IS_LINUX)
|
||||||
|
// On Linux, initialize crashpad after Nodejs init phase so that
|
||||||
|
// crash and termination signal handlers can be set by the crashpad client.
|
||||||
|
if (!pid_string.empty()) {
|
||||||
|
auto* command_line = base::CommandLine::ForCurrentProcess();
|
||||||
|
command_line->AppendSwitchASCII(
|
||||||
|
crash_reporter::switches::kCrashpadHandlerPid, pid_string);
|
||||||
|
ElectronCrashReporterClient::Create();
|
||||||
|
crash_reporter::InitializeCrashpad(false, "node");
|
||||||
|
crash_keys::SetCrashKeysFromCommandLine(
|
||||||
|
*base::CommandLine::ForCurrentProcess());
|
||||||
|
crash_keys::SetPlatformCrashKey();
|
||||||
|
// Ensure the flags and env variable does not propagate to userland.
|
||||||
|
command_line->RemoveSwitch(crash_reporter::switches::kCrashpadHandlerPid);
|
||||||
|
}
|
||||||
|
#elif BUILDFLAG(IS_WIN) || (BUILDFLAG(IS_MAC) && !IS_MAS_BUILD())
|
||||||
|
ElectronCrashReporterClient::Create();
|
||||||
|
crash_reporter::InitializeCrashpad(false, "node");
|
||||||
|
crash_keys::SetCrashKeysFromCommandLine(
|
||||||
|
*base::CommandLine::ForCurrentProcess());
|
||||||
|
crash_keys::SetPlatformCrashKey();
|
||||||
|
#endif
|
||||||
|
|
||||||
gin::V8Initializer::LoadV8Snapshot(
|
gin::V8Initializer::LoadV8Snapshot(
|
||||||
gin::V8SnapshotFileType::kWithAdditionalContext);
|
gin::V8SnapshotFileType::kWithAdditionalContext);
|
||||||
|
|
||||||
|
|
39
shell/common/api/crashpad_support.cc
Normal file
39
shell/common/api/crashpad_support.cc
Normal file
|
@ -0,0 +1,39 @@
|
||||||
|
// Copyright (c) 2022 GitHub, Inc.
|
||||||
|
// Use of this source code is governed by the MIT license that can be
|
||||||
|
// found in the LICENSE file.
|
||||||
|
|
||||||
|
#include "shell/common/gin_helper/dictionary.h"
|
||||||
|
#include "shell/common/node_includes.h"
|
||||||
|
|
||||||
|
#if BUILDFLAG(IS_LINUX)
|
||||||
|
#include "components/crash/core/app/crashpad.h" // nogncheck
|
||||||
|
#endif
|
||||||
|
|
||||||
|
namespace {
|
||||||
|
|
||||||
|
#if BUILDFLAG(IS_LINUX)
|
||||||
|
int GetCrashdumpSignalFD() {
|
||||||
|
int fd;
|
||||||
|
return crash_reporter::GetHandlerSocket(&fd, nullptr) ? fd : -1;
|
||||||
|
}
|
||||||
|
|
||||||
|
int GetCrashpadHandlerPID() {
|
||||||
|
int pid;
|
||||||
|
return crash_reporter::GetHandlerSocket(nullptr, &pid) ? pid : -1;
|
||||||
|
}
|
||||||
|
#endif
|
||||||
|
|
||||||
|
void Initialize(v8::Local<v8::Object> exports,
|
||||||
|
v8::Local<v8::Value> unused,
|
||||||
|
v8::Local<v8::Context> context,
|
||||||
|
void* priv) {
|
||||||
|
gin_helper::Dictionary dict(context->GetIsolate(), exports);
|
||||||
|
#if BUILDFLAG(IS_LINUX)
|
||||||
|
dict.SetMethod("getCrashdumpSignalFD", &GetCrashdumpSignalFD);
|
||||||
|
dict.SetMethod("getCrashpadHandlerPID", &GetCrashpadHandlerPID);
|
||||||
|
#endif
|
||||||
|
}
|
||||||
|
|
||||||
|
} // namespace
|
||||||
|
|
||||||
|
NODE_LINKED_MODULE_CONTEXT_AWARE(electron_common_crashpad_support, Initialize)
|
|
@ -80,6 +80,7 @@
|
||||||
V(electron_common_asar) \
|
V(electron_common_asar) \
|
||||||
V(electron_common_clipboard) \
|
V(electron_common_clipboard) \
|
||||||
V(electron_common_command_line) \
|
V(electron_common_command_line) \
|
||||||
|
V(electron_common_crashpad_support) \
|
||||||
V(electron_common_environment) \
|
V(electron_common_environment) \
|
||||||
V(electron_common_features) \
|
V(electron_common_features) \
|
||||||
V(electron_common_native_image) \
|
V(electron_common_native_image) \
|
||||||
|
|
|
@ -166,9 +166,7 @@ ifdescribe(!isLinuxOnArm && !process.mas && !process.env.DISABLE_CRASH_REPORTER_
|
||||||
expect(crash.mainProcessSpecific).to.equal('mps');
|
expect(crash.mainProcessSpecific).to.equal('mps');
|
||||||
});
|
});
|
||||||
|
|
||||||
// TODO(deepak1556): Re-enable this test once
|
ifit(!isLinuxOnArm)('when a node process crashes', async () => {
|
||||||
// https://github.com/electron/electron/issues/36030 is resolved.
|
|
||||||
ifit(process.platform !== 'linux')('when a node process crashes', async () => {
|
|
||||||
const { port, waitForCrash } = await startServer();
|
const { port, waitForCrash } = await startServer();
|
||||||
runCrashApp('node', port);
|
runCrashApp('node', port);
|
||||||
const crash = await waitForCrash();
|
const crash = await waitForCrash();
|
||||||
|
@ -177,6 +175,33 @@ ifdescribe(!isLinuxOnArm && !process.mas && !process.env.DISABLE_CRASH_REPORTER_
|
||||||
expect(crash.rendererSpecific).to.be.undefined();
|
expect(crash.rendererSpecific).to.be.undefined();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
ifit(!isLinuxOnArm)('when a node process inside a node process crashes', async () => {
|
||||||
|
const { port, waitForCrash } = await startServer();
|
||||||
|
runCrashApp('node-fork', port);
|
||||||
|
const crash = await waitForCrash();
|
||||||
|
checkCrash('node', crash);
|
||||||
|
expect(crash.mainProcessSpecific).to.be.undefined();
|
||||||
|
expect(crash.rendererSpecific).to.be.undefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
// Ensures that passing in crashpadHandlerPID flag for Linx child processes
|
||||||
|
// does not affect child proocess args.
|
||||||
|
ifit(process.platform === 'linux')('ensure linux child process args are not modified', async () => {
|
||||||
|
const { port, waitForCrash } = await startServer();
|
||||||
|
let exitCode: number | null = null;
|
||||||
|
const appPath = path.join(__dirname, 'fixtures', 'apps', 'crash');
|
||||||
|
const crashType = 'node-extra-args';
|
||||||
|
const crashProcess = childProcess.spawn(process.execPath, [appPath,
|
||||||
|
`--crash-type=${crashType}`,
|
||||||
|
`--crash-reporter-url=http://127.0.0.1:${port}`
|
||||||
|
], { stdio: 'inherit' });
|
||||||
|
crashProcess.once('close', (code) => {
|
||||||
|
exitCode = code;
|
||||||
|
});
|
||||||
|
await waitForCrash();
|
||||||
|
expect(exitCode).to.equal(0);
|
||||||
|
});
|
||||||
|
|
||||||
describe('with guid', () => {
|
describe('with guid', () => {
|
||||||
for (const processType of ['main', 'renderer', 'sandboxed-renderer']) {
|
for (const processType of ['main', 'renderer', 'sandboxed-renderer']) {
|
||||||
it(`when ${processType} crashes`, async () => {
|
it(`when ${processType} crashes`, async () => {
|
||||||
|
|
6
spec/fixtures/apps/crash/fork.js
vendored
Normal file
6
spec/fixtures/apps/crash/fork.js
vendored
Normal file
|
@ -0,0 +1,6 @@
|
||||||
|
const path = require('path');
|
||||||
|
const childProcess = require('child_process');
|
||||||
|
|
||||||
|
const crashPath = path.join(__dirname, 'node-crash.js');
|
||||||
|
const child = childProcess.fork(crashPath, { silent: true });
|
||||||
|
child.on('exit', () => process.exit(0));
|
13
spec/fixtures/apps/crash/main.js
vendored
13
spec/fixtures/apps/crash/main.js
vendored
|
@ -54,6 +54,19 @@ app.whenReady().then(() => {
|
||||||
const crashPath = path.join(__dirname, 'node-crash.js');
|
const crashPath = path.join(__dirname, 'node-crash.js');
|
||||||
const child = childProcess.fork(crashPath, { silent: true });
|
const child = childProcess.fork(crashPath, { silent: true });
|
||||||
child.on('exit', () => process.exit(0));
|
child.on('exit', () => process.exit(0));
|
||||||
|
} else if (crashType === 'node-fork') {
|
||||||
|
const scriptPath = path.join(__dirname, 'fork.js');
|
||||||
|
const child = childProcess.fork(scriptPath, { silent: true });
|
||||||
|
child.on('exit', () => process.exit(0));
|
||||||
|
} else if (crashType === 'node-extra-args') {
|
||||||
|
let exitcode = -1;
|
||||||
|
const crashPath = path.join(__dirname, 'node-extra-args.js');
|
||||||
|
const child = childProcess.fork(crashPath, ['--enable-logging'], { silent: true });
|
||||||
|
child.send('message');
|
||||||
|
child.on('message', (forkedArgs) => {
|
||||||
|
if (JSON.stringify(forkedArgs) !== JSON.stringify(child.spawnargs)) { exitcode = 1; } else { exitcode = 0; }
|
||||||
|
process.exit(exitcode);
|
||||||
|
});
|
||||||
} else {
|
} else {
|
||||||
console.error(`Unrecognized crash type: '${crashType}'`);
|
console.error(`Unrecognized crash type: '${crashType}'`);
|
||||||
process.exit(1);
|
process.exit(1);
|
||||||
|
|
9
spec/fixtures/apps/crash/node-extra-args.js
vendored
Normal file
9
spec/fixtures/apps/crash/node-extra-args.js
vendored
Normal file
|
@ -0,0 +1,9 @@
|
||||||
|
const path = require('path');
|
||||||
|
const childProcess = require('child_process');
|
||||||
|
|
||||||
|
process.on('message', function () {
|
||||||
|
process.send(process.argv);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Allow time to send args, then crash the app.
|
||||||
|
setTimeout(() => process.nextTick(() => process.crash()), 10000);
|
Loading…
Reference in a new issue