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

fix: enable crashpad for ELECTRON_RUN_AS_NODE processes #36483

Merged
merged 2 commits into from Nov 29, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions filenames.gni
Expand Up @@ -524,6 +524,7 @@ filenames = {
"shell/browser/window_list_observer.h",
"shell/browser/zoom_level_delegate.cc",
"shell/browser/zoom_level_delegate.h",
"shell/common/api/crashpad_support.cc",
"shell/common/api/electron_api_asar.cc",
"shell/common/api/electron_api_clipboard.cc",
"shell/common/api/electron_api_clipboard.h",
Expand Down
1 change: 1 addition & 0 deletions patches/node/.patches
Expand Up @@ -52,3 +52,4 @@ drop_deserializerequest_move_constructor_for_c_20_compat.patch
fix_parallel_test-v8-stats.patch
fix_expose_the_built-in_electron_module_via_the_esm_loader.patch
chore_enable_c_17_for_native_modules.patch
enable_crashpad_linux_node_processes.patch
51 changes: 51 additions & 0 deletions patches/node/enable_crashpad_linux_node_processes.patch
@@ -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.
54 changes: 45 additions & 9 deletions shell/app/node_main.cc
Expand Up @@ -34,6 +34,14 @@
#include "chrome/child/v8_crashpad_support_win.h"
#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 !defined(MAS_BUILD)
#include "components/crash/core/app/crashpad.h" // nogncheck
#include "shell/app/electron_crash_reporter_client.h"
Expand Down Expand Up @@ -110,15 +118,20 @@ int NodeMain(int argc, char* argv[]) {
v8_crashpad_support::SetUp();
#endif

// TODO(deepak1556): Enable crashpad support on linux for
// ELECTRON_RUN_AS_NODE processes.
// Refs https://github.com/electron/electron/issues/36030
#if BUILDFLAG(IS_WIN) || (BUILDFLAG(IS_MAC) && !defined(MAS_BUILD))
ElectronCrashReporterClient::Create();
crash_reporter::InitializeCrashpad(false, "node");
crash_keys::SetCrashKeysFromCommandLine(
*base::CommandLine::ForCurrentProcess());
crash_keys::SetPlatformCrashKey();
#if BUILDFLAG(IS_LINUX)
auto os_env = base::Environment::Create();
std::string fd_string, pid_string;
if (os_env->GetVar("CRASHDUMP_SIGNAL_FD", &fd_string) &&
os_env->GetVar("CRASHPAD_HANDLER_PID", &pid_string)) {
int fd = -1, pid = -1;
DCHECK(base::StringToInt(fd_string, &fd));
DCHECK(base::StringToInt(pid_string, &pid));
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

int exit_code = 1;
Expand Down Expand Up @@ -148,6 +161,29 @@ int NodeMain(int argc, char* argv[]) {
if (result.early_return)
exit(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) && !defined(MAS_BUILD))
ElectronCrashReporterClient::Create();
crash_reporter::InitializeCrashpad(false, "node");
crash_keys::SetCrashKeysFromCommandLine(
*base::CommandLine::ForCurrentProcess());
crash_keys::SetPlatformCrashKey();
#endif

gin::V8Initializer::LoadV8Snapshot(
gin::V8SnapshotFileType::kWithAdditionalContext);

Expand Down
39 changes: 39 additions & 0 deletions shell/common/api/crashpad_support.cc
@@ -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)
1 change: 1 addition & 0 deletions shell/common/node_bindings.cc
Expand Up @@ -80,6 +80,7 @@
V(electron_common_asar) \
V(electron_common_clipboard) \
V(electron_common_command_line) \
V(electron_common_crashpad_support) \
V(electron_common_environment) \
V(electron_common_features) \
V(electron_common_native_image) \
Expand Down
31 changes: 28 additions & 3 deletions spec/api-crash-reporter-spec.ts
Expand Up @@ -166,9 +166,7 @@ ifdescribe(!isLinuxOnArm && !process.mas && !process.env.DISABLE_CRASH_REPORTER_
expect(crash.mainProcessSpecific).to.equal('mps');
});

// TODO(deepak1556): Re-enable this test once
// https://github.com/electron/electron/issues/36030 is resolved.
ifit(process.platform !== 'linux')('when a node process crashes', async () => {
ifit(!isLinuxOnArm)('when a node process crashes', async () => {
const { port, waitForCrash } = await startServer();
runCrashApp('node', port);
const crash = await waitForCrash();
Expand All @@ -177,6 +175,33 @@ ifdescribe(!isLinuxOnArm && !process.mas && !process.env.DISABLE_CRASH_REPORTER_
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', () => {
for (const processType of ['main', 'renderer', 'sandboxed-renderer']) {
it(`when ${processType} crashes`, async () => {
Expand Down
6 changes: 6 additions & 0 deletions spec/fixtures/apps/crash/fork.js
@@ -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 changes: 13 additions & 0 deletions spec/fixtures/apps/crash/main.js
Expand Up @@ -54,6 +54,19 @@ app.whenReady().then(() => {
const crashPath = path.join(__dirname, 'node-crash.js');
const child = childProcess.fork(crashPath, { silent: true });
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 {
console.error(`Unrecognized crash type: '${crashType}'`);
process.exit(1);
Expand Down
9 changes: 9 additions & 0 deletions spec/fixtures/apps/crash/node-extra-args.js
@@ -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);