Bug 1400051 - IPC: use process_util_linux on BSD and remove process_util_bsd. r?glandium
This change is mainly to avoid the use of SetAllFDsToCloseOnExec, which
has an unavoidable race condition that can leak file descriptors into
child processes. The "Linux" implementation of child process creation,
now that B2G support has been removed, is essentially a generic Unix
fork+dup2+execve which works on BSD (and is already used on Solaris).
MozReview-Commit-ID: KUpP2RYIYNp
--- a/ipc/chromium/moz.build
+++ b/ipc/chromium/moz.build
@@ -90,26 +90,19 @@ if os_macosx:
# This file cannot be built in unified mode because of the redefinition
# of NoOp.
'src/base/platform_thread_mac.mm',
]
if os_bsd:
SOURCES += [
'src/base/atomicops_internals_x86_gcc.cc',
+ 'src/base/process_util_linux.cc',
'src/base/time_posix.cc',
]
- if CONFIG['OS_ARCH'] == 'GNU_kFreeBSD':
- SOURCES += [
- 'src/base/process_util_linux.cc'
- ]
- else:
- SOURCES += [
- 'src/base/process_util_bsd.cc'
- ]
if os_linux:
SOURCES += [
'src/base/atomicops_internals_x86_gcc.cc',
'src/base/process_util_linux.cc',
'src/base/time_posix.cc',
]
if CONFIG['OS_TARGET'] == 'Android':
--- a/ipc/chromium/src/base/process_util.h
+++ b/ipc/chromium/src/base/process_util.h
@@ -116,17 +116,17 @@ struct LaunchOptions {
#if defined(OS_POSIX)
environment_map env_map;
// A mapping of (src fd -> dest fd) to propagate into the child
// process. All other fds will be closed, except std{in,out,err}.
file_handle_mapping_vector fds_to_remap;
#endif
-#if defined(OS_LINUX) || defined(OS_SOLARIS)
+#if defined(OS_LINUX)
struct ForkDelegate {
virtual ~ForkDelegate() { }
virtual pid_t Fork() = 0;
};
// If non-null, the fork delegate will be called instead of fork().
mozilla::UniquePtr<ForkDelegate> fork_delegate = nullptr;
#endif
deleted file mode 100644
--- a/ipc/chromium/src/base/process_util_bsd.cc
+++ /dev/null
@@ -1,101 +0,0 @@
-/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* vim: set ts=8 sts=2 et sw=2 tw=80: */
-// Copyright (c) 2008 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-// derived from process_util_mac.cc
-
-#include "base/process_util.h"
-
-#include <fcntl.h>
-#include <spawn.h>
-#include <sys/wait.h>
-
-#include <string>
-
-#include "base/eintr_wrapper.h"
-
-namespace {
-
-static mozilla::EnvironmentLog gProcessLog("MOZ_PROCESS_LOG");
-
-} // namespace
-
-namespace base {
-
-
-bool LaunchApp(const std::vector<std::string>& argv,
- const LaunchOptions& options,
- ProcessHandle* process_handle)
-{
- bool retval = true;
-
- char* argv_copy[argv.size() + 1];
- for (size_t i = 0; i < argv.size(); i++) {
- argv_copy[i] = const_cast<char*>(argv[i].c_str());
- }
- argv_copy[argv.size()] = NULL;
-
- // Make sure we don't leak any FDs to the child process by marking all FDs
- // as close-on-exec.
- SetAllFDsToCloseOnExec();
-
- EnvironmentArray vars = BuildEnvironmentArray(options.env_map);
-
- posix_spawn_file_actions_t file_actions;
- if (posix_spawn_file_actions_init(&file_actions) != 0) {
- return false;
- }
-
- // Turn fds_to_remap array into a set of dup2 calls.
- for (const auto& fd_map : options.fds_to_remap) {
- int src_fd = fd_map.first;
- int dest_fd = fd_map.second;
-
- if (src_fd == dest_fd) {
- int flags = fcntl(src_fd, F_GETFD);
- if (flags != -1) {
- fcntl(src_fd, F_SETFD, flags & ~FD_CLOEXEC);
- }
- } else {
- if (posix_spawn_file_actions_adddup2(&file_actions, src_fd, dest_fd) != 0) {
- posix_spawn_file_actions_destroy(&file_actions);
- return false;
- }
- }
- }
-
- pid_t pid = 0;
- int spawn_succeeded = (posix_spawnp(&pid,
- argv_copy[0],
- &file_actions,
- NULL,
- argv_copy,
- vars.get()) == 0);
-
- posix_spawn_file_actions_destroy(&file_actions);
-
- bool process_handle_valid = pid > 0;
- if (!spawn_succeeded || !process_handle_valid) {
- retval = false;
- } else {
- gProcessLog.print("==> process %d launched child process %d\n",
- GetCurrentProcId(), pid);
- if (options.wait)
- HANDLE_EINTR(waitpid(pid, 0, 0));
-
- if (process_handle)
- *process_handle = pid;
- }
-
- return retval;
-}
-
-bool LaunchApp(const CommandLine& cl,
- const LaunchOptions& options,
- ProcessHandle* process_handle) {
- return LaunchApp(cl.argv(), options, process_handle);
-}
-
-} // namespace base
--- a/ipc/chromium/src/base/process_util_linux.cc
+++ b/ipc/chromium/src/base/process_util_linux.cc
@@ -11,16 +11,19 @@
#include <sys/wait.h>
#include <unistd.h>
#include "base/eintr_wrapper.h"
#include "base/logging.h"
#include "mozilla/ipc/FileDescriptorShuffle.h"
#include "mozilla/UniquePtr.h"
+// WARNING: despite the name, this file is also used on the BSDs and
+// Solaris (basically, Unixes that aren't Mac OS), not just Linux.
+
namespace {
static mozilla::EnvironmentLog gProcessLog("MOZ_PROCESS_LOG");
} // namespace
namespace base {
@@ -31,17 +34,22 @@ bool LaunchApp(const std::vector<std::st
mozilla::UniquePtr<char*[]> argv_cstr(new char*[argv.size() + 1]);
EnvironmentArray envp = BuildEnvironmentArray(options.env_map);
mozilla::ipc::FileDescriptorShuffle shuffle;
if (!shuffle.Init(options.fds_to_remap)) {
return false;
}
+#ifdef OS_LINUX
pid_t pid = options.fork_delegate ? options.fork_delegate->Fork() : fork();
+#else
+ pid_t pid = fork();
+#endif
+
if (pid < 0)
return false;
if (pid == 0) {
// In the child:
for (const auto& fds : shuffle.Dup2Sequence()) {
if (HANDLE_EINTR(dup2(fds.first, fds.second)) != fds.second) {
// This shouldn't happen, but check for it. And see below