Remove the Mac-specific implementation of LaunchApp, and share the Linux version.
Fixes a race condition with file descriptors, and gives the Mac access to the environment-alterning version of LaunchApp
BUG=11174
TEST=Launching render/plugin/utility processes should still work on the Mac
Review URL: http://codereview.chromium.org/165067
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@22649 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/base/process_util.h b/base/process_util.h
index 31d8de7..450dbc2 100644
--- a/base/process_util.h
+++ b/base/process_util.h
@@ -142,7 +142,7 @@
bool LaunchApp(const std::vector<std::string>& argv,
const file_handle_mapping_vector& fds_to_remap,
bool wait, ProcessHandle* process_handle);
-#if defined(OS_LINUX)
+
// Similar to above, but also (un)set environment variables in child process
// through |environ|.
typedef std::vector<std::pair<const char*, const char*> > environment_vector;
@@ -150,7 +150,6 @@
const environment_vector& environ,
const file_handle_mapping_vector& fds_to_remap,
bool wait, ProcessHandle* process_handle);
-#endif // defined(OS_LINUX)
#endif // defined(OS_POSIX)
// Executes the application specified by cl. This function delegates to one
diff --git a/base/process_util_linux.cc b/base/process_util_linux.cc
index 50eb4a2..a05e9bb 100644
--- a/base/process_util_linux.cc
+++ b/base/process_util_linux.cc
@@ -13,7 +13,6 @@
#include <string>
-#include "base/eintr_wrapper.h"
#include "base/file_util.h"
#include "base/logging.h"
#include "base/string_tokenizer.h"
@@ -86,82 +85,6 @@
return FilePath(std::string(exename, len));
}
-bool LaunchApp(const std::vector<std::string>& argv,
- const environment_vector& environ,
- const file_handle_mapping_vector& fds_to_remap,
- bool wait, ProcessHandle* process_handle) {
- pid_t pid = fork();
- if (pid < 0)
- return false;
-
- if (pid == 0) {
- // Child process
- InjectiveMultimap fd_shuffle;
- for (file_handle_mapping_vector::const_iterator
- it = fds_to_remap.begin(); it != fds_to_remap.end(); ++it) {
- fd_shuffle.push_back(InjectionArc(it->first, it->second, false));
- }
-
- for (environment_vector::const_iterator it = environ.begin();
- it != environ.end(); ++it) {
- if (it->first) {
- if (it->second) {
- setenv(it->first, it->second, 1);
- } else {
- unsetenv(it->first);
- }
- }
- }
-
- // Obscure fork() rule: in the child, if you don't end up doing exec*(),
- // you call _exit() instead of exit(). This is because _exit() does not
- // call any previously-registered (in the parent) exit handlers, which
- // might do things like block waiting for threads that don't even exist
- // in the child.
- if (!ShuffleFileDescriptors(fd_shuffle))
- _exit(127);
-
- // If we are using the SUID sandbox, it sets a magic environment variable
- // ("SBX_D"), so we remove that variable from the environment here on the
- // off chance that it's already set.
- unsetenv("SBX_D");
-
- CloseSuperfluousFds(fd_shuffle);
-
- scoped_array<char*> argv_cstr(new char*[argv.size() + 1]);
- for (size_t i = 0; i < argv.size(); i++)
- argv_cstr[i] = const_cast<char*>(argv[i].c_str());
- argv_cstr[argv.size()] = NULL;
- execvp(argv_cstr[0], argv_cstr.get());
- LOG(ERROR) << "LaunchApp: exec failed!, argv_cstr[0] " << argv_cstr[0]
- << ", errno " << errno;
- _exit(127);
- } else {
- // Parent process
- if (wait)
- HANDLE_EINTR(waitpid(pid, 0, 0));
-
- if (process_handle)
- *process_handle = pid;
- }
-
- return true;
-}
-
-bool LaunchApp(const std::vector<std::string>& argv,
- const file_handle_mapping_vector& fds_to_remap,
- bool wait, ProcessHandle* process_handle) {
- base::environment_vector no_env;
- return LaunchApp(argv, no_env, fds_to_remap, wait, process_handle);
-}
-
-bool LaunchApp(const CommandLine& cl,
- bool wait, bool start_hidden,
- ProcessHandle* process_handle) {
- file_handle_mapping_vector no_files;
- return LaunchApp(cl.argv(), no_files, wait, process_handle);
-}
-
NamedProcessIterator::NamedProcessIterator(const std::wstring& executable_name,
const ProcessFilter* filter)
: executable_name_(executable_name), filter_(filter) {
diff --git a/base/process_util_mac.mm b/base/process_util_mac.mm
index 9653ada..de9b3f1 100644
--- a/base/process_util_mac.mm
+++ b/base/process_util_mac.mm
@@ -21,78 +21,6 @@
namespace base {
-bool LaunchApp(const std::vector<std::string>& argv,
- const file_handle_mapping_vector& fds_to_remap,
- bool wait, 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();
-
- 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 (file_handle_mapping_vector::const_iterator it = fds_to_remap.begin();
- it != fds_to_remap.end();
- ++it) {
- int src_fd = it->first;
- int dest_fd = it->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;
- }
- }
- }
-
- int pid = 0;
- int spawn_succeeded = (posix_spawnp(&pid,
- argv_copy[0],
- &file_actions,
- NULL,
- argv_copy,
- *_NSGetEnviron()) == 0);
-
- posix_spawn_file_actions_destroy(&file_actions);
-
- bool process_handle_valid = pid > 0;
- if (!spawn_succeeded || !process_handle_valid) {
- retval = false;
- } else {
- if (wait)
- HANDLE_EINTR(waitpid(pid, 0, 0));
-
- if (process_handle)
- *process_handle = pid;
- }
-
- return retval;
-}
-
-bool LaunchApp(const CommandLine& cl,
- bool wait, bool start_hidden, ProcessHandle* process_handle) {
- // TODO(playmobil): Do we need to respect the start_hidden flag?
- file_handle_mapping_vector no_files;
- return LaunchApp(cl.argv(), no_files, wait, process_handle);
-}
-
NamedProcessIterator::NamedProcessIterator(const std::wstring& executable_name,
const ProcessFilter* filter)
: executable_name_(executable_name),
diff --git a/base/process_util_posix.cc b/base/process_util_posix.cc
index 6ff660f..ecb4937 100644
--- a/base/process_util_posix.cc
+++ b/base/process_util_posix.cc
@@ -220,6 +220,82 @@
}
}
+bool LaunchApp(const std::vector<std::string>& argv,
+ const environment_vector& environ,
+ const file_handle_mapping_vector& fds_to_remap,
+ bool wait, ProcessHandle* process_handle) {
+ pid_t pid = fork();
+ if (pid < 0)
+ return false;
+
+ if (pid == 0) {
+ // Child process
+ InjectiveMultimap fd_shuffle;
+ for (file_handle_mapping_vector::const_iterator
+ it = fds_to_remap.begin(); it != fds_to_remap.end(); ++it) {
+ fd_shuffle.push_back(InjectionArc(it->first, it->second, false));
+ }
+
+ for (environment_vector::const_iterator it = environ.begin();
+ it != environ.end(); ++it) {
+ if (it->first) {
+ if (it->second) {
+ setenv(it->first, it->second, 1);
+ } else {
+ unsetenv(it->first);
+ }
+ }
+ }
+
+ // Obscure fork() rule: in the child, if you don't end up doing exec*(),
+ // you call _exit() instead of exit(). This is because _exit() does not
+ // call any previously-registered (in the parent) exit handlers, which
+ // might do things like block waiting for threads that don't even exist
+ // in the child.
+ if (!ShuffleFileDescriptors(fd_shuffle))
+ _exit(127);
+
+ // If we are using the SUID sandbox, it sets a magic environment variable
+ // ("SBX_D"), so we remove that variable from the environment here on the
+ // off chance that it's already set.
+ unsetenv("SBX_D");
+
+ CloseSuperfluousFds(fd_shuffle);
+
+ scoped_array<char*> argv_cstr(new char*[argv.size() + 1]);
+ for (size_t i = 0; i < argv.size(); i++)
+ argv_cstr[i] = const_cast<char*>(argv[i].c_str());
+ argv_cstr[argv.size()] = NULL;
+ execvp(argv_cstr[0], argv_cstr.get());
+ LOG(ERROR) << "LaunchApp: exec failed!, argv_cstr[0] " << argv_cstr[0]
+ << ", errno " << errno;
+ _exit(127);
+ } else {
+ // Parent process
+ if (wait)
+ HANDLE_EINTR(waitpid(pid, 0, 0));
+
+ if (process_handle)
+ *process_handle = pid;
+ }
+
+ return true;
+}
+
+bool LaunchApp(const std::vector<std::string>& argv,
+ const file_handle_mapping_vector& fds_to_remap,
+ bool wait, ProcessHandle* process_handle) {
+ base::environment_vector no_env;
+ return LaunchApp(argv, no_env, fds_to_remap, wait, process_handle);
+}
+
+bool LaunchApp(const CommandLine& cl,
+ bool wait, bool start_hidden,
+ ProcessHandle* process_handle) {
+ file_handle_mapping_vector no_files;
+ return LaunchApp(cl.argv(), no_files, wait, process_handle);
+}
+
ProcessMetrics::ProcessMetrics(ProcessHandle process) : process_(process),
last_time_(0),
last_system_time_(0) {