3party/crashpad
0
0
Fork 0
mirror of https://github.com/chromium/crashpad.git synced 2025-03-09 22:16:13 +00:00
Code Issues Packages Projects Releases Wiki Activity

Reland "posix: Replace DoubleForkAndExec() with ForkAndSpawn()"

Browse Source 
This is a reland of 460943dd9a71dc76f68182a8ede766d5543e5341

Original change's description:
> The DoubleForkAndExec() function was taking over 622 milliseconds to run
> on macOS 11 (BigSur) on Intel i5-1038NG7. I did some debugging by adding
> some custom traces and found that the fork() syscall is the bottleneck
> here, i.e., the first fork() takes around 359 milliseconds and the
> nested fork() takes around 263 milliseconds. Replacing the nested fork()
> and exec() with posix_spawn() reduces the time consumption to 257
> milliseconds!
>
> See https://github.com/libuv/libuv/pull/3064 to know why fork() is so
> slow on macOS and why posix_spawn() is a better replacement.
>
> Another point to note is that even base::LaunchProcess() from Chromium
> calls posix_spawnp() on macOS -
> 8f8d82dea0:base/process/launch_mac.cc;l=295-296

The reland isolates the change to non-Android POSIX systems because
posix_spawn and posix_spawnp are available in Android NDK 28, but
Chromium is building with version 23.

Change-Id: If44629f5445bb0e3d0a1d3698b85f047d1cbf04f
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3721655
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
This commit is contained in:
Darshan Sen 2022-06-24 19:20:54 +05:30 committed by Crashpad LUCI CQ
parent 6e946c4af8
commit 1c37daa5ac
8 changed files with 303 additions and 213 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
AUTHORS
View File

@ -12,3 +12,4 @@ Opera Software ASA
Vewd Software AS
LG Electronics, Inc.
MIPS Technologies, Inc.
Darshan Sen <raisinten@gmail.com>

10
client/crashpad_client_linux.cc
View File

@ -45,9 +45,9 @@
#include "util/linux/socket.h"
#include "util/misc/address_sanitizer.h"
#include "util/misc/from_pointer_cast.h"
#include "util/posix/double_fork_and_exec.h"
#include "util/posix/scoped_mmap.h"
#include "util/posix/signals.h"
#include "util/posix/spawn_subprocess.h"
namespace crashpad {
@ -459,7 +459,7 @@ bool CrashpadClient::StartHandler(
argv.push_back(FormatArgumentInt("initial-client-fd", handler_sock.get()));
argv.push_back("--shared-client-connection");
if (!DoubleForkAndExec(argv, nullptr, handler_sock.get(), false, nullptr)) {
if (!SpawnSubprocess(argv, nullptr, handler_sock.get(), false, nullptr)) {
return false;
}
@ -614,7 +614,7 @@ bool CrashpadClient::StartJavaHandlerForClient(
int socket) {
std::vector<std::string> argv = BuildAppProcessArgs(
class_name, database, metrics_dir, url, annotations, arguments, socket);
return DoubleForkAndExec(argv, env, socket, false, nullptr);
return SpawnSubprocess(argv, env, socket, false, nullptr);
}
bool CrashpadClient::StartHandlerWithLinkerAtCrash(
@ -663,7 +663,7 @@ bool CrashpadClient::StartHandlerWithLinkerForClient(
annotations,
arguments,
socket);
return DoubleForkAndExec(argv, env, socket, false, nullptr);
return SpawnSubprocess(argv, env, socket, false, nullptr);
}
#endif
@ -697,7 +697,7 @@ bool CrashpadClient::StartHandlerForClient(
argv.push_back(FormatArgumentInt("initial-client-fd", socket));
return DoubleForkAndExec(argv, nullptr, socket, true, nullptr);
return SpawnSubprocess(argv, nullptr, socket, true, nullptr);
}
// static

4
client/crashpad_client_mac.cc
View File

@ -36,7 +36,7 @@
#include "util/mach/notify_server.h"
#include "util/misc/clock.h"
#include "util/misc/implicit_cast.h"
#include "util/posix/double_fork_and_exec.h"
#include "util/posix/spawn_subprocess.h"
namespace crashpad {
@ -343,7 +343,7 @@ class HandlerStarter final : public NotifyServer::DefaultInterface {
// this parent process, which was probably using the exception server now
// being restarted. The handler can’t monitor itself for its own crashes via
// this interface.
if (!DoubleForkAndExec(
if (!SpawnSubprocess(
argv,
nullptr,
server_write_fd.get(),

13
handler/linux/cros_crash_report_exception_handler.cc
View File

@ -29,7 +29,7 @@
#include "util/linux/ptrace_client.h"
#include "util/misc/metrics.h"
#include "util/misc/uuid.h"
#include "util/posix/double_fork_and_exec.h"
#include "util/posix/spawn_subprocess.h"
namespace crashpad {
@ -266,12 +266,11 @@ bool CrosCrashReportExceptionHandler::HandleExceptionWithConnection(
argv.push_back("--always_allow_feedback");
}
if (!DoubleForkAndExec(argv,
nullptr /* envp */,
file_writer.fd() /* preserve_fd */,
false /* use_path */,
nullptr /* child_function */)) {
LOG(ERROR) << "DoubleForkAndExec failed";
if (!SpawnSubprocess(argv,
nullptr /* envp */,
file_writer.fd() /* preserve_fd */,
false /* use_path */,
nullptr /* child_function */)) {
Metrics::ExceptionCaptureResult(
Metrics::CaptureResult::kFinishedWritingCrashReportFailed);
return false;

4
util/BUILD.gn
View File

@ -288,11 +288,11 @@ crashpad_static_library("util") {
sources += [
"posix/close_multiple.cc",
"posix/close_multiple.h",
"posix/double_fork_and_exec.cc",
"posix/double_fork_and_exec.h",
"posix/drop_privileges.cc",
"posix/drop_privileges.h",
"posix/process_info.h",
"posix/spawn_subprocess.cc",
"posix/spawn_subprocess.h",
# These map signals to and from strings. While Fuchsia defines some of
# the common SIGx defines, signals are never raised on Fuchsia, so

166
util/posix/double_fork_and_exec.cc
View File

@ -1,166 +0,0 @@
// Copyright 2017 The Crashpad Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#include "util/posix/double_fork_and_exec.h"
#include <stdlib.h>
#include <string.h>
#include <sys/wait.h>
#include <unistd.h>
#include "base/check_op.h"
#include "base/logging.h"
#include "base/posix/eintr_wrapper.h"
#include "base/strings/stringprintf.h"
#include "util/posix/close_multiple.h"
namespace crashpad {
bool DoubleForkAndExec(const std::vector<std::string>& argv,
const std::vector<std::string>* envp,
int preserve_fd,
bool use_path,
void (*child_function)()) {
DCHECK(!envp || !use_path);
// argv_c contains const char* pointers and is terminated by nullptr. This is
// suitable for passing to execv(). Although argv_c is not used in the parent
// process, it must be built in the parent process because it’s unsafe to do
// so in the child or grandchild process.
std::vector<const char*> argv_c;
argv_c.reserve(argv.size() + 1);
for (const std::string& argument : argv) {
argv_c.push_back(argument.c_str());
}
argv_c.push_back(nullptr);
std::vector<const char*> envp_c;
if (envp) {
envp_c.reserve(envp->size() + 1);
for (const std::string& variable : *envp) {
envp_c.push_back(variable.c_str());
}
envp_c.push_back(nullptr);
}
// Double-fork(). The three processes involved are parent, child, and
// grandchild. The grandchild will call execv(). The child exits immediately
// after spawning the grandchild, so the grandchild becomes an orphan and its
// parent process ID becomes 1. This relieves the parent and child of the
// responsibility to reap the grandchild with waitpid() or similar. The
// grandchild is expected to outlive the parent process, so the parent
// shouldn’t be concerned with reaping it. This approach means that accidental
// early termination of the handler process will not result in a zombie
// process.
pid_t pid = fork();
if (pid < 0) {
PLOG(ERROR) << "fork";
return false;
}
if (pid == 0) {
// Child process.
if (child_function) {
child_function();
}
// Call setsid(), creating a new process group and a new session, both led
// by this process. The new process group has no controlling terminal. This
// disconnects it from signals generated by the parent process’ terminal.
//
// setsid() is done in the child instead of the grandchild so that the
// grandchild will not be a session leader. If it were a session leader, an
// accidental open() of a terminal device without O_NOCTTY would make that
// terminal the controlling terminal.
//
// It’s not desirable for the grandchild to have a controlling terminal. The
// grandchild manages its own lifetime, such as by monitoring clients on its
// own and exiting when it loses all clients and when it deems it
// appropraite to do so. It may serve clients in different process groups or
// sessions than its original client, and receiving signals intended for its
// original client’s process group could be harmful in that case.
PCHECK(setsid() != -1) << "setsid";
pid = fork();
if (pid < 0) {
PLOG(FATAL) << "fork";
}
if (pid > 0) {
// Child process.
// _exit() instead of exit(), because fork() was called.
_exit(EXIT_SUCCESS);
}
// Grandchild process.
CloseMultipleNowOrOnExec(STDERR_FILENO + 1, preserve_fd);
// &argv_c[0] is a pointer to a pointer to const char data, but because of
// how C (not C++) works, execvp() wants a pointer to a const pointer to
// char data. It modifies neither the data nor the pointers, so the
// const_cast is safe.
char* const* argv_for_execv = const_cast<char* const*>(&argv_c[0]);
if (envp) {
// This cast is safe for the same reason that the argv_for_execv cast is.
char* const* envp_for_execv = const_cast<char* const*>(&envp_c[0]);
execve(argv_for_execv[0], argv_for_execv, envp_for_execv);
PLOG(FATAL) << "execve " << argv_for_execv[0];
}
if (use_path) {
execvp(argv_for_execv[0], argv_for_execv);
PLOG(FATAL) << "execvp " << argv_for_execv[0];
}
execv(argv_for_execv[0], argv_for_execv);
PLOG(FATAL) << "execv " << argv_for_execv[0];
}
// waitpid() for the child, so that it does not become a zombie process. The
// child normally exits quickly.
//
// Failures from this point on may result in the accumulation of a zombie, but
// should not be considered fatal. Log only warnings, but don’t treat these
// failures as a failure of the function overall.
int status;
pid_t wait_pid = HANDLE_EINTR(waitpid(pid, &status, 0));
if (wait_pid == -1) {
PLOG(WARNING) << "waitpid";
return true;
}
DCHECK_EQ(wait_pid, pid);
if (WIFSIGNALED(status)) {
int sig = WTERMSIG(status);
LOG(WARNING) << base::StringPrintf(
"intermediate process terminated by signal %d (%s)%s",
sig,
strsignal(sig),
WCOREDUMP(status) ? " (core dumped)" : "");
} else if (!WIFEXITED(status)) {
LOG(WARNING) << base::StringPrintf(
"intermediate process: unknown termination 0x%x", status);
} else if (WEXITSTATUS(status) != EXIT_SUCCESS) {
LOG(WARNING) << "intermediate process exited with code "
<< WEXITSTATUS(status);
}
return true;
}
} // namespace crashpad

261
util/posix/spawn_subprocess.cc Normal file
View File

@ -0,0 +1,261 @@
// Copyright 2017 The Crashpad Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#include "util/posix/spawn_subprocess.h"
#include <errno.h>
#include <spawn.h>
#include <stdlib.h>
#include <string.h>
#include <sys/wait.h>
#include <unistd.h>
#include "base/check.h"
#include "base/check_op.h"
#include "base/logging.h"
#include "base/posix/eintr_wrapper.h"
#include "base/strings/stringprintf.h"
#include "build/build_config.h"
#include "util/posix/close_multiple.h"
#if BUILDFLAG(IS_ANDROID)
#include <android/api-level.h>
#endif
extern char** environ;
namespace crashpad {
namespace {
#if BUILDFLAG(IS_APPLE)
class PosixSpawnAttr {
public:
PosixSpawnAttr() {
PCHECK((errno = posix_spawnattr_init(&attr_)) == 0)
<< "posix_spawnattr_init";
}
PosixSpawnAttr(const PosixSpawnAttr&) = delete;
PosixSpawnAttr& operator=(const PosixSpawnAttr&) = delete;
~PosixSpawnAttr() {
PCHECK((errno = posix_spawnattr_destroy(&attr_)) == 0)
<< "posix_spawnattr_destroy";
}
void SetFlags(short flags) {
PCHECK((errno = posix_spawnattr_setflags(&attr_, flags)) == 0)
<< "posix_spawnattr_setflags";
}
const posix_spawnattr_t* Get() const { return &attr_; }
private:
posix_spawnattr_t attr_;
};
class PosixSpawnFileActions {
public:
PosixSpawnFileActions() {
PCHECK((errno = posix_spawn_file_actions_init(&file_actions_)) == 0)
<< "posix_spawn_file_actions_init";
}
PosixSpawnFileActions(const PosixSpawnFileActions&) = delete;
PosixSpawnFileActions& operator=(const PosixSpawnFileActions&) = delete;
~PosixSpawnFileActions() {
PCHECK((errno = posix_spawn_file_actions_destroy(&file_actions_)) == 0)
<< "posix_spawn_file_actions_destroy";
}
void AddInheritedFileDescriptor(int fd) {
PCHECK((errno = posix_spawn_file_actions_addinherit_np(&file_actions_,
fd)) == 0)
<< "posix_spawn_file_actions_addinherit_np";
}
const posix_spawn_file_actions_t* Get() const { return &file_actions_; }
private:
posix_spawn_file_actions_t file_actions_;
};
#endif
} // namespace
bool SpawnSubprocess(const std::vector<std::string>& argv,
const std::vector<std::string>* envp,
int preserve_fd,
bool use_path,
void (*child_function)()) {
// argv_c contains const char* pointers and is terminated by nullptr. This is
// suitable for passing to posix_spawn*() and execv*(). Although argv_c is not
// used in the parent process, it must be built in the parent process because
// it’s unsafe to do so in the child or grandchild process.
std::vector<const char*> argv_c;
argv_c.reserve(argv.size() + 1);
for (const std::string& argument : argv) {
argv_c.push_back(argument.c_str());
}
argv_c.push_back(nullptr);
std::vector<const char*> envp_c;
if (envp) {
envp_c.reserve(envp->size() + 1);
for (const std::string& variable : *envp) {
envp_c.push_back(variable.c_str());
}
envp_c.push_back(nullptr);
}
// The three processes involved are parent, child, and grandchild. The child
// exits immediately after spawning the grandchild, so the grandchild becomes
// an orphan and its parent process ID becomes 1. This relieves the parent and
// child of the responsibility to reap the grandchild with waitpid() or
// similar. The grandchild is expected to outlive the parent process, so the
// parent shouldn’t be concerned with reaping it. This approach means that
// accidental early termination of the handler process will not result in a
// zombie process.
pid_t pid = fork();
if (pid < 0) {
PLOG(ERROR) << "fork";
return false;
}
if (pid == 0) {
// Child process.
if (child_function) {
child_function();
}
// Call setsid(), creating a new process group and a new session, both led
// by this process. The new process group has no controlling terminal. This
// disconnects it from signals generated by the parent process’ terminal.
//
// setsid() is done in the child instead of the grandchild so that the
// grandchild will not be a session leader. If it were a session leader, an
// accidental open() of a terminal device without O_NOCTTY would make that
// terminal the controlling terminal.
//
// It’s not desirable for the grandchild to have a controlling terminal. The
// grandchild manages its own lifetime, such as by monitoring clients on its
// own and exiting when it loses all clients and when it deems it
// appropraite to do so. It may serve clients in different process groups or
// sessions than its original client, and receiving signals intended for its
// original client’s process group could be harmful in that case.
PCHECK(setsid() != -1) << "setsid";
// &argv_c[0] is a pointer to a pointer to const char data, but because of
// how C (not C++) works, posix_spawn*() and execv*() want a pointer to
// a const pointer to char data. They modify neither the data nor the
// pointers, so the const_cast is safe.
char* const* argv_for_spawn = const_cast<char* const*>(argv_c.data());
// This cast is safe for the same reason that the argv_for_spawn cast is.
char* const* envp_for_spawn =
envp ? const_cast<char* const*>(envp_c.data()) : environ;
#if BUILDFLAG(IS_ANDROID) && __ANDROID_API__ < 28
pid = fork();
if (pid < 0) {
PLOG(FATAL) << "fork";
}
if (pid > 0) {
// Child process.
// _exit() instead of exit(), because fork() was called.
_exit(EXIT_SUCCESS);
}
// Grandchild process.
CloseMultipleNowOrOnExec(STDERR_FILENO + 1, preserve_fd);
auto execve_fp = use_path ? execvpe : execve;
execve_fp(argv_for_spawn[0], argv_for_spawn, envp_for_spawn);
PLOG(FATAL) << (use_path ? "execvpe" : "execve");
#else
#if BUILDFLAG(IS_APPLE)
PosixSpawnAttr attr;
attr.SetFlags(POSIX_SPAWN_CLOEXEC_DEFAULT);
PosixSpawnFileActions file_actions;
for (int fd = 0; fd <= STDERR_FILENO; ++fd) {
file_actions.AddInheritedFileDescriptor(fd);
}
file_actions.AddInheritedFileDescriptor(preserve_fd);
const posix_spawnattr_t* attr_p = attr.Get();
const posix_spawn_file_actions_t* file_actions_p = file_actions.Get();
#else
CloseMultipleNowOrOnExec(STDERR_FILENO + 1, preserve_fd);
const posix_spawnattr_t* attr_p = nullptr;
const posix_spawn_file_actions_t* file_actions_p = nullptr;
#endif
auto posix_spawn_fp = use_path ? posix_spawnp : posix_spawn;
if ((errno = posix_spawn_fp(nullptr,
argv_for_spawn[0],
file_actions_p,
attr_p,
argv_for_spawn,
envp_for_spawn)) != 0) {
PLOG(FATAL) << (use_path ? "posix_spawnp" : "posix_spawn");
}
// _exit() instead of exit(), because fork() was called.
_exit(EXIT_SUCCESS);
#endif
}
// waitpid() for the child, so that it does not become a zombie process. The
// child normally exits quickly.
//
// Failures from this point on may result in the accumulation of a zombie, but
// should not be considered fatal. Log only warnings, but don’t treat these
// failures as a failure of the function overall.
int status;
pid_t wait_pid = HANDLE_EINTR(waitpid(pid, &status, 0));
if (wait_pid == -1) {
PLOG(WARNING) << "waitpid";
return true;
}
DCHECK_EQ(wait_pid, pid);
if (WIFSIGNALED(status)) {
int sig = WTERMSIG(status);
LOG(WARNING) << base::StringPrintf(
"intermediate process terminated by signal %d (%s)%s",
sig,
strsignal(sig),
WCOREDUMP(status) ? " (core dumped)" : "");
} else if (!WIFEXITED(status)) {
LOG(WARNING) << base::StringPrintf(
"intermediate process: unknown termination 0x%x", status);
} else if (WEXITSTATUS(status) != EXIT_SUCCESS) {
LOG(WARNING) << "intermediate process exited with code "
<< WEXITSTATUS(status);
}
return true;
}
} // namespace crashpad

57
util/posix/double_fork_and_exec.h → util/posix/spawn_subprocess.h
View File

@ -12,48 +12,43 @@
// See the License for the specific language governing permissions and
// limitations under the License.
#ifndef CRASHPAD_UTIL_POSIX_DOUBLE_FORK_AND_EXEC_H_
#define CRASHPAD_UTIL_POSIX_DOUBLE_FORK_AND_EXEC_H_
#ifndef CRASHPAD_UTIL_POSIX_SPAWN_SUBPROCESS_H_
#define CRASHPAD_UTIL_POSIX_SPAWN_SUBPROCESS_H_
#include <string>
#include <vector>
namespace crashpad {
//! \brief Executes a (grand-)child process.
//! \brief Spawns a subprocess.
//!
//! The grandchild process will be started through the
//! double-`fork()`-and-`execv()` pattern. This allows the grandchild to fully
//! disassociate from the parent. The grandchild will not be a member of the
//! parent’s process group or session and will not have a controlling terminal,
//! providing isolation from signals not intended for it. The grandchild’s
//! parent process, in terms of the process tree hierarchy, will be the process
//! with process ID 1, relieving any other process of the responsibility to reap
//! it via `waitpid()`. Aside from the three file descriptors associated with
//! the standard input/output streams and any file descriptor passed in \a
//! preserve_fd, the grandchild will not inherit any file descriptors from the
//! parent process.
//! A grandchild process will be started through the
//! `fork()`-and-`posix_spawn()` pattern where supported, and
//! double-`fork()`-and-`execv()` pattern elsewhere. This allows the grandchild
//! to fully disassociate from the parent. The grandchild will not be a member
//! of the parent’s process group or session and will not have a controlling
//! terminal, providing isolation from signals not intended for it. The
//! grandchild’s parent process, in terms of the process tree hierarchy, will be
//! the process with process ID 1, relieving any other process of the
//! responsibility to reap it via `waitpid()`. Aside from the three file
//! descriptors associated with the standard input/output streams and any file
//! descriptor passed in \a preserve_fd, the grandchild will not inherit any
//! file descriptors from the parent process.
//!
//! \param[in] argv The argument vector to start the grandchild process with.
//! `argv[0]` is used as the path to the executable.
//! \param[in] envp A vector of environment variables of the form `var=value` to
//! be passed to `execve()`. If this value is `nullptr`, the current
//! environment is used.
//! be passed to the spawned process. If this value is `nullptr`, the
//! current environment is used.
//! \param[in] preserve_fd A file descriptor to be inherited by the grandchild
//! process. This file descriptor is inherited in addition to the three file
//! descriptors associated with the standard input/output streams. Use `-1`
//! if no additional file descriptors are to be inherited.
//! \param[in] use_path Whether to consult the `PATH` environment variable when
//! requested to start an executable at a non-absolute path. If `false`,
//! `execv()`, which does not consult `PATH`, will be used. If `true`,
//! `execvp()`, which does consult `PATH`, will be used.
//! requested to start an executable at a non-absolute path.
//! \param[in] child_function If not `nullptr`, this function will be called in
//! the intermediate child process, prior to the second `fork()`. Take note
//! that this function will run in the context of a forked process, and must
//! be safe for that purpose.
//!
//! Setting both \a envp to a value other than `nullptr` and \a use_path to
//! `true` is not currently supported.
//! the intermediate child process. Take note that this function will run in
//! the context of a forked process, and must be safe for that purpose.
//!
//! \return `true` on success, and `false` on failure with a message logged.
//! Only failures that occur in the parent process that indicate a definite
@ -63,12 +58,12 @@ namespace crashpad {
//! terminating. The caller assumes the responsibility for detecting such
//! failures, for example, by observing a failure to perform a successful
//! handshake with the grandchild process.
bool DoubleForkAndExec(const std::vector<std::string>& argv,
const std::vector<std::string>* envp,
int preserve_fd,
bool use_path,
void (*child_function)());
bool SpawnSubprocess(const std::vector<std::string>& argv,
const std::vector<std::string>* envp,
int preserve_fd,
bool use_path,
void (*child_function)());
} // namespace crashpad
#endif // CRASHPAD_UTIL_POSIX_DOUBLE_FORK_AND_EXEC_H_
#endif // CRASHPAD_UTIL_POSIX_SPAWN_SUBPROCESS_H_