fix: enable crashpad for ELECTRON_RUN_AS_NODE processes (#36483)

* fix: enable crashpad for ELECTRON_RUN_AS_NODE processes

* 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 <[email protected]>
Co-authored-by: Jeremy Rose <[email protected]>
Co-authored-by: VerteDinde <[email protected]>

* chore: update patches

Co-authored-by: Jeremy Rose <[email protected]>
Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>

filenames.gni

@@ -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",

patches/node/.patches

@@ -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

patches/node/enable_crashpad_linux_node_processes.patch

@@ -0,0 +1,51 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: VerteDinde <[email protected]>
+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 2a91c4820bebf55068c4d54a2e1133176de77a6d..a9eb36881acb8eba50f05758995ebe327de86a2d 100644
+--- a/lib/child_process.js
++++ b/lib/child_process.js
+@@ -58,6 +58,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,
+@@ -146,7 +147,6 @@ function fork(modulePath, args = [], options) {
+       ArrayPrototypeSplice(execArgv, index - 1, 2);
+     }
+   }
+-
+   args = [...execArgv, modulePath, ...args];
+ 
+   if (typeof options.stdio === 'string') {
+@@ -592,6 +592,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.

shell/app/node_main.cc

@@ -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"
@@ -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;
@@ -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);
 

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)

shell/common/node_bindings.cc

@@ -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)        \

spec/api-crash-reporter-spec.ts

@@ -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();
@@ -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 () => {

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));

spec/fixtures/apps/crash/main.js

@@ -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);

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);